Repository: tailwindlabs/headlessui
Pull request #3128 (2024-04-24 11:44:12)

Make sure data-disabled is available on virtualized options
When using a combobox disabled options get a `data-disabled` attribute that can be used for styling. However, when the combobox was virtualized we were not. This happened because we were only looking at the disabled prop on the option itself but a virtual combobox uses a disabled function in the `virtual` options to determine whether or not an option is enabled.

This fixes that 👍 

@RobinMalfait you have an idea on how this should be tested?

vercel[bot]: [vc]: #HNIvzbphBIeDUzfUEnHTIxf+RnO8lXrGQvj9xyVpjw0=:eyJpc01vbm9yZXBvIjp0cnVlLCJ0eXBlIjoiZ2l0aHViIiwicHJvamVjdHMiOlt7Im5hbWUiOiJoZWFkbGVzc3VpLXJlYWN0Iiwicm9vdERpcmVjdG9yeSI6InBsYXlncm91bmRzL3JlYWN0IiwiaW5zcGVjdG9yVXJsIjoiaHR0cHM6Ly92ZXJjZWwuY29tL3RhaWx3aW5kbGFicy9oZWFkbGVzc3VpLXJlYWN0LzdrVzZqRVJ6dmFvM3BVM1ZUcUJhaUZLSG5Ccm4iLCJwcmV2aWV3VXJsIjoiaGVhZGxlc3N1aS1yZWFjdC1naXQtZml4LWRhdGEtZGlzYWJsZWQtb24tdi0wNTQ5OWItdGFpbHdpbmRsYWJzLnZlcmNlbC5hcHAiLCJuZXh0Q29tbWl0U3RhdHVzIjoiREVQTE9ZRUQifSx7Im5hbWUiOiJoZWFkbGVzc3VpLXZ1ZSIsInJvb3REaXJlY3RvcnkiOiJwbGF5Z3JvdW5kcy92dWUiLCJpbnNwZWN0b3JVcmwiOiJodHRwczovL3ZlcmNlbC5jb20vdGFpbHdpbmRsYWJzL2hlYWRsZXNzdWktdnVlLzRwQlA2UlFYZ3RDRjRFUWQ0ZEpSMzFNaDlqYTMiLCJwcmV2aWV3VXJsIjoiaGVhZGxlc3N1aS12dWUtZ2l0LWZpeC1kYXRhLWRpc2FibGVkLW9uLXZpci03ZDI3YTUtdGFpbHdpbmRsYWJzLnZlcmNlbC5hcHAiLCJuZXh0Q29tbWl0U3RhdHVzIjoiREVQTE9ZRUQifV19
**The latest updates on your projects**. Learn more about [Vercel for Git ↗︎](https://vercel.link/github-learn-more)

| Name | Status | Preview | Updated (UTC) |
| :--- | :----- | :------ | :------ |
| **headlessui-react** | ✅ Ready ([Inspect](https://vercel.com/tailwindlabs/headlessui-react/7kW6jERzvao3pU3VTqBaiFKHnBrn)) | [Visit Preview](https://headlessui-react-git-fix-data-disabled-on-v-05499b-tailwindlabs.vercel.app) | Apr 24, 2024 11:44am |
| **headlessui-vue** | ✅ Ready ([Inspect](https://vercel.com/tailwindlabs/headlessui-vue/4pBP6RQXgtCF4EQd4dJR31Mh9ja3)) | [Visit Preview](https://headlessui-vue-git-fix-data-disabled-on-vir-7d27a5-tailwindlabs.vercel.app) | Apr 24, 2024 11:44am |



RobinMalfait: @thecrypticace if I'm not mistaken we run all tests for the Combobox against a virtual and non-virtual version. So if you add a test where you explicitly test that data-disabled is present then that should cover this. Undoing your change should then result in a failing test.
thecrypticace: We run all the "interaction" tests but not rendering tests. But this is simple enough to add a test for — will do!
thecrypticace: Test added

Patch:
diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md
index 2d620e5..00fb3ff 100644
--- 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.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx
index 0e515da..15f13b2 100644
--- 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,7 +1787,7 @@ 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)
@@ -1795,16 +1795,20 @@ function OptionFn<
 
   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,


Test patch:
diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx
index fed87bd..912964d 100644
--- 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', () => {