Skip to content

Commit

Permalink
Make sure data-disabled is available on virtualized options (#3128)
Browse files Browse the repository at this point in the history
* Make sure data-disabled is available on virtualized options

* Add test

* Update changelog

* calculate `disabled` state once

This way we only have to calculate it once and we can re-use that
information throughout the component. If the `value` changes of the
option, then the component has to re-render anyway which will re-compute
the `disabled` state.

---------

Co-authored-by: Robin Malfait <malfait.robin@gmail.com>
  • Loading branch information
thecrypticace and RobinMalfait committed Apr 24, 2024
1 parent d2b7345 commit 166e862
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 13 deletions.
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Move focus to `ListboxOptions` and `MenuItems` when they are rendered later ([#3112](https://github.com/tailwindlabs/headlessui/pull/3112))
- Ensure anchored components are always rendered in a stacking context ([#3115](https://github.com/tailwindlabs/headlessui/pull/3115))
- Add optional `onClose` callback to `Combobox` component ([#3122](https://github.com/tailwindlabs/headlessui/pull/3122))
- Make sure `data-disabled` is available on virtualized options in the `Combobox` component ([#3128](https://github.com/tailwindlabs/headlessui/pull/3128))

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1769,6 +1769,101 @@ describe('Rendering', () => {
expect(handleChange).toHaveBeenNthCalledWith(2, 'bob')
})
})

describe.each([{ virtual: true }, { virtual: false }])('Data attributes', ({ virtual }) => {
let data = ['Option A', 'Option B', 'Option C']
function MyCombobox<T>({
options = data.slice() as T[],
useComboboxOptions = true,
comboboxProps = {},
inputProps = {},
buttonProps = {},
optionProps = {},
}: {
options?: T[]
useComboboxOptions?: boolean
comboboxProps?: Record<string, any>
inputProps?: Record<string, any>
buttonProps?: Record<string, any>
optionProps?: Record<string, any>
}) {
function isDisabled(option: T): boolean {
return typeof option === 'string'
? false
: typeof option === 'object' &&
option !== null &&
'disabled' in option &&
typeof option.disabled === 'boolean'
? option?.disabled ?? false
: false
}
if (virtual) {
return (
<Combobox
virtual={{
options,
disabled: isDisabled,
}}
value={'test' as T}
onChange={NOOP}
{...comboboxProps}
>
<Combobox.Input onChange={NOOP} {...inputProps} />
<Combobox.Button {...buttonProps}>Trigger</Combobox.Button>
{useComboboxOptions && (
<Combobox.Options>
{({ option }) => {
return <Combobox.Option {...optionProps} value={option} />
}}
</Combobox.Options>
)}
</Combobox>
)
}

return (
<Combobox value="test" onChange={NOOP} {...comboboxProps}>
<Combobox.Input onChange={NOOP} {...inputProps} />
<Combobox.Button {...buttonProps}>Trigger</Combobox.Button>
{useComboboxOptions && (
<Combobox.Options>
{options.map((option, idx) => {
return (
<Combobox.Option
key={idx}
disabled={isDisabled(option)}
{...optionProps}
value={option}
/>
)
})}
</Combobox.Options>
)}
</Combobox>
)
}

it('Disabled options should get a data-disabled attribute', async () => {
render(
<MyCombobox
options={[
{ name: 'Option A', disabled: false },
{ name: 'Option B', disabled: true },
{ name: 'Option C', disabled: false },
]}
/>
)

// Open the Combobox
await click(getByText('Trigger'))

let options = getComboboxOptions()

expect(options[0]).not.toHaveAttribute('data-disabled')
expect(options[1]).toHaveAttribute('data-disabled', '')
expect(options[2]).not.toHaveAttribute('data-disabled')
})
})
})

describe('Rendering composition', () => {
Expand Down
30 changes: 17 additions & 13 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1315,7 +1315,7 @@ function InputFn<
: data.virtual
? data.options.find(
(option) =>
!data.virtual?.disabled(option.dataRef.current.value) &&
!option.dataRef.current.disabled &&
data.compare(
option.dataRef.current.value,
data.virtual!.options[data.activeOptionIndex!]
Expand Down Expand Up @@ -1666,18 +1666,18 @@ function OptionFn<
// But today is not that day..
TType = Parameters<typeof ComboboxRoot>[0]['value'],
>(props: ComboboxOptionProps<TTag, TType>, ref: Ref<HTMLLIElement>) {
let data = useData('Combobox.Option')
let actions = useActions('Combobox.Option')

let internalId = useId()
let {
id = `headlessui-combobox-option-${internalId}`,
disabled = false,
value,
disabled = data.virtual?.disabled(value) ?? false,
order = null,
...theirProps
} = props

let data = useData('Combobox.Option')
let actions = useActions('Combobox.Option')

let refocusInput = useRefocusableInput(data.inputRef)

let active = data.virtual
Expand Down Expand Up @@ -1749,7 +1749,7 @@ function OptionFn<
return
}

if (disabled || data.virtual?.disabled(value)) return
if (disabled) return
select()

// We want to make sure that we don't accidentally trigger the virtual keyboard.
Expand All @@ -1774,7 +1774,7 @@ function OptionFn<
})

let handleFocus = useEvent(() => {
if (disabled || data.virtual?.disabled(value)) {
if (disabled) {
return actions.goToOption(Focus.Nothing)
}
let idx = data.calculateIndex(value)
Expand All @@ -1787,24 +1787,28 @@ function OptionFn<

let handleMove = useEvent((evt) => {
if (!pointer.wasMoved(evt)) return
if (disabled || data.virtual?.disabled(value)) return
if (disabled) return
if (active) return
let idx = data.calculateIndex(value)
actions.goToOption(Focus.Specific, idx, ActivationTrigger.Pointer)
})

let handleLeave = useEvent((evt) => {
if (!pointer.wasMoved(evt)) return
if (disabled || data.virtual?.disabled(value)) return
if (disabled) return
if (!active) return
if (data.optionsPropsRef.current.hold) return
actions.goToOption(Focus.Nothing)
})

let slot = useMemo(
() => ({ active, focus: active, selected, disabled }) satisfies OptionRenderPropArg,
[active, selected, disabled]
)
let slot = useMemo(() => {
return {
active,
focus: active,
selected,
disabled,
} satisfies OptionRenderPropArg
}, [active, selected, disabled])

let ourProps = {
id,
Expand Down

0 comments on commit 166e862

Please sign in to comment.