Make sure data-disabled is available on virtualized options (#3128)

* 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 <[email protected]>
tailwindlabs · Apr 24, 2024 · 166e862 · 166e862
1 parent d2b7345
commit 166e862
Show file tree

Hide file tree

Showing 3 changed files with 113 additions and 13 deletions.
diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md
@@ -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
 

diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx
@@ -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', () => {

diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx
@@ -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!]
@@ -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
@@ -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.
@@ -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)
@@ -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,