improve Combobox accessibility

This PR improves the Combobox because there are some differences
compared to the Listbox that we didn't properly account for.

Big thanks to @afercia for providing us with a wonderful issue report
that talks about the differences.

This commit fixes a few issues:

- The `aria-selected` value was always based on the `selected` value
  (which is correct for the `Listbox`) but it should be based on the
  visually selected value (in our case, this is the `active` value).
- We dropped an incorrect `aria-activedescendant` attribute from the
  `ComboboxOptions`, it should only exist on the `ComboboxInput`.
- We added a missing `aria-autocomplete` to the `ComboboxInput`, with a
  value of `list` so that assistive technology better understands what
  kind of Combobox we are dealing with.
- This also includes a VoiceOver bug fix where the announcements aren't
  properly happening unless a change to the `ComboboxInput` is made.
  Now we will trigger a change to the input when we open the `Combobox`,
  we will set the value to `''` and re-set to whatever value was
  present. We also make sure that the cursor position / selected range
  is reset so that the end user doesn't feel any differences (except for
  a better working VoiceOver on macOS).

Fixes: #2129
Co-authored-by: Andrea Fercia <a.fercia@gmail.com>

packages/@headlessui-react/src/components/combobox/combobox.tsx

@@ -662,6 +662,7 @@ type InputPropsWeControl =
   | 'aria-labelledby'
   | 'aria-expanded'
   | 'aria-activedescendant'
+  | 'aria-autocomplete'
   | 'onKeyDown'
   | 'onChange'
   | 'displayValue'
@@ -741,6 +742,37 @@ let Input = forwardRefWithAs(function Input<
     [currentDisplayValue, data.comboboxState]
   )
 
+  // Trick VoiceOver in behaving a little bit better. Manually "resetting" the input makes VoiceOver
+  // a bit more happy and doesn't require some changes manually first before announcing items
+  // correctly. This is a bit of a hacks, but it is a workaround for a VoiceOver bug.
+  //
+  // TODO: VoiceOver is still relatively buggy if you start VoiceOver while the Combobox is already
+  // in an open state.
+  useWatch(
+    ([newState], [oldState]) => {
+      if (newState === ComboboxState.Open && oldState === ComboboxState.Closed) {
+        let input = data.inputRef.current
+        if (!input) return
+
+        // Capture current state
+        let currentValue = input.value
+        let { selectionStart, selectionEnd, selectionDirection } = input
+
+        // Trick VoiceOver into announcing the value
+        input.value = ''
+
+        // Rollback to original state
+        input.value = currentValue
+        if (selectionDirection !== null) {
+          input.setSelectionRange(selectionStart, selectionEnd, selectionDirection)
+        } else {
+          input.setSelectionRange(selectionStart, selectionEnd)
+        }
+      }
+    },
+    [data.comboboxState]
+  )
+
   let isComposing = useRef(false)
   let handleCompositionStart = useEvent(() => {
     isComposing.current = true
@@ -905,6 +937,7 @@ let Input = forwardRefWithAs(function Input<
       data.activeOptionIndex === null ? undefined : data.options[data.activeOptionIndex]?.id,
     'aria-multiselectable': data.mode === ValueMode.Multi ? true : undefined,
     'aria-labelledby': labelledby,
+    'aria-autocomplete': 'list',
     defaultValue:
       props.defaultValue ??
       (data.defaultValue !== undefined
@@ -1090,13 +1123,7 @@ let DEFAULT_OPTIONS_TAG = 'ul' as const
 interface OptionsRenderPropArg {
   open: boolean
 }
-type OptionsPropsWeControl =
-  | 'aria-activedescendant'
-  | 'aria-labelledby'
-  | 'hold'
-  | 'onKeyDown'
-  | 'role'
-  | 'tabIndex'
+type OptionsPropsWeControl = 'aria-labelledby' | 'hold' | 'onKeyDown' | 'role' | 'tabIndex'
 
 let OptionsRenderFeatures = Features.RenderStrategy | Features.Static
 
@@ -1154,8 +1181,6 @@ let Options = forwardRefWithAs(function Options<
     [data]
   )
   let ourProps = {
-    'aria-activedescendant':
-      data.activeOptionIndex === null ? undefined : data.options[data.activeOptionIndex]?.id,
     'aria-labelledby': labelledby,
     role: 'listbox',
     id,
@@ -1286,10 +1311,10 @@ let Option = forwardRefWithAs(function Option<
     role: 'option',
     tabIndex: disabled === true ? undefined : -1,
     'aria-disabled': disabled === true ? true : undefined,
-    // According to the WAI-ARIA best practices, we should use aria-checked for
-    // multi-select,but Voice-Over disagrees. So we use aria-checked instead for
-    // both single and multi-select.
-    'aria-selected': selected,
+    // A combobox behaves a little bit different compared to a listbox. In a listbox the
+    // `aria-selected` value should be set based on the `selected` value, but in a combobox it
+    // should be based on the visually `active` value.
+    'aria-selected': active ? true : undefined,
     disabled: undefined, // Never forward the `disabled` prop
     onClick: handleClick,
     onFocus: handleFocus,

packages/@headlessui-react/src/test-utils/accessibility-assertions.ts

@@ -346,11 +346,13 @@ export function assertComboboxInput(
     switch (options.state) {
       case ComboboxState.Visible:
         expect(input).toHaveAttribute('aria-controls')
+        expect(input).toHaveAttribute('aria-autocomplete', 'list')
         expect(input).toHaveAttribute('aria-expanded', 'true')
         break
 
       case ComboboxState.InvisibleHidden:
         expect(input).toHaveAttribute('aria-controls')
+        expect(input).toHaveAttribute('aria-autocomplete', 'list')
         if (input.hasAttribute('disabled')) {
           expect(input).not.toHaveAttribute('aria-expanded')
         } else {
@@ -618,7 +620,7 @@ export function assertNoActiveComboboxOption(combobox = getComboboxInput()) {
 
 export function assertNoSelectedComboboxOption(items = getComboboxOptions()) {
   try {
-    for (let item of items) expect(item).toHaveAttribute('aria-selected', 'false')
+    for (let item of items) expect(item).not.toHaveAttribute('aria-selected')
   } catch (err) {
     if (err instanceof Error) Error.captureStackTrace(err, assertNoSelectedComboboxOption)
     throw err
@@ -656,7 +658,11 @@ export function assertComboboxOption(
     }
 
     if (options.selected != null) {
-      return expect(item).toHaveAttribute('aria-selected', options.selected ? 'true' : 'false')
+      if (options.selected) {
+        expect(item).toHaveAttribute('aria-selected', 'true')
+      } else {
+        expect(item).not.toHaveAttribute('aria-selected')
+      }
     }
   } catch (err) {
     if (err instanceof Error) Error.captureStackTrace(err, assertComboboxOption)

packages/@headlessui-vue/src/components/combobox/combobox.ts

@@ -712,6 +712,35 @@ export let ComboboxInput = defineComponent({
         },
         { immediate: true }
       )
+
+      // Trick VoiceOver in behaving a little bit better. Manually "resetting" the input makes
+      // VoiceOver a bit more happy and doesn't require some changes manually first before
+      // announcing items correctly. This is a bit of a hacks, but it is a workaround for a
+      // VoiceOver bug.
+      //
+      // TODO: VoiceOver is still relatively buggy if you start VoiceOver while the Combobox is
+      // already in an open state.
+      watch([api.comboboxState], ([newState], [oldState]) => {
+        if (newState === ComboboxStates.Open && oldState === ComboboxStates.Closed) {
+          let input = dom(api.inputRef)
+          if (!input) return
+
+          // Capture current state
+          let currentValue = input.value
+          let { selectionStart, selectionEnd, selectionDirection } = input
+
+          // Trick VoiceOver into announcing the value
+          input.value = ''
+
+          // Rollback to original state
+          input.value = currentValue
+          if (selectionDirection !== null) {
+            input.setSelectionRange(selectionStart, selectionEnd, selectionDirection)
+          } else {
+            input.setSelectionRange(selectionStart, selectionEnd)
+          }
+        }
+      })
     })
 
     let isComposing = ref(false)
@@ -880,6 +909,7 @@ export let ComboboxInput = defineComponent({
             : api.options.value[api.activeOptionIndex.value]?.id,
         'aria-multiselectable': api.mode.value === ValueMode.Multi ? true : undefined,
         'aria-labelledby': dom(api.labelRef)?.id ?? dom(api.buttonRef)?.id,
+        'aria-autocomplete': 'list',
         id,
         onCompositionstart: handleCompositionstart,
         onCompositionend: handleCompositionend,
@@ -956,10 +986,6 @@ export let ComboboxOptions = defineComponent({
     return () => {
       let slot = { open: api.comboboxState.value === ComboboxStates.Open }
       let ourProps = {
-        'aria-activedescendant':
-          api.activeOptionIndex.value === null
-            ? undefined
-            : api.options.value[api.activeOptionIndex.value]?.id,
         'aria-labelledby': dom(api.labelRef)?.id ?? dom(api.buttonRef)?.id,
         id,
         ref: api.optionsRef,
@@ -1074,10 +1100,10 @@ export let ComboboxOption = defineComponent({
         role: 'option',
         tabIndex: disabled === true ? undefined : -1,
         'aria-disabled': disabled === true ? true : undefined,
-        // According to the WAI-ARIA best practices, we should use aria-checked for
-        // multi-select,but Voice-Over disagrees. So we use aria-selected instead for
-        // both single and multi-select.
-        'aria-selected': selected.value,
+        // A combobox behaves a little bit different compared to a listbox. In a listbox the
+        // `aria-selected` value should be set based on the `selected` value, but in a combobox it
+        // should be based on the visually `active` value.
+        'aria-selected': active.value ? true : undefined,
         disabled: undefined, // Never forward the `disabled` prop
         onClick: handleClick,
         onFocus: handleFocus,