Improve accessibility when announcing Listbox.Option and Combobox.Option components (#1812)

* ensure that `aria-selected` is explicitly set to `false`

The WAI-ARIA Best Practices don't recommend this and prefer
`aria-selected: true` or undefined (aka not existing when it is
"false"). However in practice, both MacOS VoiceOver and NVDA experience
strange issues if you don't do this (e.g.: everything before the
selected item is also selected)

* update tests to ensure we are checking for `aria-selected=false`

* update changelog
This commit is contained in:
Robin Malfait
2022-09-02 16:31:46 +02:00
committed by GitHub
parent ed4d80e442
commit 4878092712
12 changed files with 50 additions and 84 deletions
+1
View File
@@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Don't scroll when wrapping around in focus trap ([#1789](https://github.com/tailwindlabs/headlessui/pull/1789))
- Fix `Transition` component's incorrect cleanup and order of events ([#1803](https://github.com/tailwindlabs/headlessui/pull/1803))
- Ensure enter transitions work when using `unmount={false}` ([#1811](https://github.com/tailwindlabs/headlessui/pull/1811))
- Improve accessibility when announcing `Listbox.Option` and `Combobox.Option` components ([#1812](https://github.com/tailwindlabs/headlessui/pull/1812))
## [1.6.6] - 2022-07-07
@@ -295,23 +295,23 @@ describe('Rendering', () => {
await click(getComboboxButton())
let [alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'false')
await click(getComboboxOptions()[2])
await click(getComboboxButton())
;[alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'false')
expect(charlie).toHaveAttribute('aria-selected', 'true')
await click(getComboboxOptions()[1])
await click(getComboboxButton())
;[alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'false')
})
)
@@ -339,15 +339,15 @@ describe('Rendering', () => {
await click(getComboboxOptions()[2])
let [alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).toHaveAttribute('aria-selected', 'true')
await click(getComboboxOptions()[2])
;[alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'false')
})
)
})
@@ -1155,7 +1155,7 @@ let Option = forwardRefWithAs(function Option<
// 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 === true ? true : undefined,
'aria-selected': selected,
disabled: undefined, // Never forward the `disabled` prop
onClick: handleClick,
onFocus: handleFocus,
@@ -286,23 +286,23 @@ describe('Rendering', () => {
await click(getListboxButton())
let [alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'false')
await click(getListboxOptions()[2])
await click(getListboxButton())
;[alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'false')
expect(charlie).toHaveAttribute('aria-selected', 'true')
await click(getListboxOptions()[1])
await click(getListboxButton())
;[alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'false')
})
)
@@ -330,15 +330,15 @@ describe('Rendering', () => {
await click(getListboxOptions()[2])
let [alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).toHaveAttribute('aria-selected', 'true')
await click(getListboxOptions()[2])
;[alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'false')
})
)
})
@@ -875,7 +875,7 @@ let Option = forwardRefWithAs(function Option<
// 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 === true ? true : undefined,
'aria-selected': selected,
disabled: undefined, // Never forward the `disabled` prop
onClick: handleClick,
onFocus: handleFocus,
@@ -618,7 +618,7 @@ export function assertNoActiveComboboxOption(combobox = getComboboxInput()) {
export function assertNoSelectedComboboxOption(items = getComboboxOptions()) {
try {
for (let item of items) expect(item).not.toHaveAttribute('aria-selected')
for (let item of items) expect(item).toHaveAttribute('aria-selected', 'false')
} catch (err) {
if (err instanceof Error) Error.captureStackTrace(err, assertNoSelectedComboboxOption)
throw err
@@ -656,16 +656,7 @@ export function assertComboboxOption(
}
if (options.selected != null) {
switch (options.selected) {
case true:
return expect(item).toHaveAttribute('aria-selected', 'true')
case false:
return expect(item).not.toHaveAttribute('aria-selected')
default:
assertNever(options.selected)
}
return expect(item).toHaveAttribute('aria-selected', options.selected ? 'true' : 'false')
}
} catch (err) {
if (err instanceof Error) Error.captureStackTrace(err, assertComboboxOption)
@@ -948,7 +939,7 @@ export function assertNoActiveListboxOption(listbox = getListbox()) {
export function assertNoSelectedListboxOption(items = getListboxOptions()) {
try {
for (let item of items) expect(item).not.toHaveAttribute('aria-selected')
for (let item of items) expect(item).toHaveAttribute('aria-selected', 'false')
} catch (err) {
if (err instanceof Error) Error.captureStackTrace(err, assertNoSelectedListboxOption)
throw err
@@ -986,16 +977,7 @@ export function assertListboxOption(
}
if (options.selected != null) {
switch (options.selected) {
case true:
return expect(item).toHaveAttribute('aria-selected', 'true')
case false:
return expect(item).not.toHaveAttribute('aria-selected')
default:
assertNever(options.selected)
}
return expect(item).toHaveAttribute('aria-selected', options.selected ? 'true' : 'false')
}
} catch (err) {
if (err instanceof Error) Error.captureStackTrace(err, assertListboxOption)
+1
View File
@@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Only select the active option when using "singular" mode when pressing `<tab>` in the `Combobox` component ([#1750](https://github.com/tailwindlabs/headlessui/pull/1750))
- Only restore focus to the `MenuButton` if necessary when activating a `MenuOption` ([#1782](https://github.com/tailwindlabs/headlessui/pull/1782))
- Don't scroll when wrapping around in focus trap ([#1789](https://github.com/tailwindlabs/headlessui/pull/1789))
- Improve accessibility when announcing `ListboxOption` and `ComboboxOption` components ([#1812](https://github.com/tailwindlabs/headlessui/pull/1812))
## [1.6.7] - 2022-07-12
@@ -331,23 +331,23 @@ describe('Rendering', () => {
await click(getComboboxButton())
let [alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'false')
await click(getComboboxOptions()[2])
await click(getComboboxButton())
;[alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'false')
expect(charlie).toHaveAttribute('aria-selected', 'true')
await click(getComboboxOptions()[1])
await click(getComboboxButton())
;[alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'false')
})
)
@@ -375,15 +375,15 @@ describe('Rendering', () => {
await click(getComboboxOptions()[2])
let [alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).toHaveAttribute('aria-selected', 'true')
await click(getComboboxOptions()[2])
;[alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'false')
})
)
})
@@ -943,7 +943,7 @@ export let ComboboxOption = defineComponent({
// 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.value === true ? selected.value : undefined,
'aria-selected': selected.value,
disabled: undefined, // Never forward the `disabled` prop
onClick: handleClick,
onFocus: handleFocus,
@@ -311,23 +311,23 @@ describe('Rendering', () => {
await click(getListboxButton())
let [alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'false')
await click(getListboxOptions()[2])
await click(getListboxButton())
;[alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'false')
expect(charlie).toHaveAttribute('aria-selected', 'true')
await click(getListboxOptions()[1])
await click(getListboxButton())
;[alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'false')
})
)
@@ -355,15 +355,15 @@ describe('Rendering', () => {
await click(getListboxOptions()[2])
let [alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).toHaveAttribute('aria-selected', 'true')
await click(getListboxOptions()[2])
;[alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(alice).toHaveAttribute('aria-selected', 'false')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'false')
})
)
})
@@ -757,7 +757,7 @@ export let ListboxOption = defineComponent({
// 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.value === true ? selected.value : undefined,
'aria-selected': selected.value,
disabled: undefined, // Never forward the `disabled` prop
onClick: handleClick,
onFocus: handleFocus,
@@ -618,7 +618,7 @@ export function assertNoActiveComboboxOption(combobox = getComboboxInput()) {
export function assertNoSelectedComboboxOption(items = getComboboxOptions()) {
try {
for (let item of items) expect(item).not.toHaveAttribute('aria-selected')
for (let item of items) expect(item).toHaveAttribute('aria-selected', 'false')
} catch (err) {
if (err instanceof Error) Error.captureStackTrace(err, assertNoSelectedComboboxOption)
throw err
@@ -656,16 +656,7 @@ export function assertComboboxOption(
}
if (options.selected != null) {
switch (options.selected) {
case true:
return expect(item).toHaveAttribute('aria-selected', 'true')
case false:
return expect(item).not.toHaveAttribute('aria-selected')
default:
assertNever(options.selected)
}
return expect(item).toHaveAttribute('aria-selected', options.selected ? 'true' : 'false')
}
} catch (err) {
if (err instanceof Error) Error.captureStackTrace(err, assertComboboxOption)
@@ -948,7 +939,7 @@ export function assertNoActiveListboxOption(listbox = getListbox()) {
export function assertNoSelectedListboxOption(items = getListboxOptions()) {
try {
for (let item of items) expect(item).not.toHaveAttribute('aria-selected')
for (let item of items) expect(item).toHaveAttribute('aria-selected', 'false')
} catch (err) {
if (err instanceof Error) Error.captureStackTrace(err, assertNoSelectedListboxOption)
throw err
@@ -986,16 +977,7 @@ export function assertListboxOption(
}
if (options.selected != null) {
switch (options.selected) {
case true:
return expect(item).toHaveAttribute('aria-selected', 'true')
case false:
return expect(item).not.toHaveAttribute('aria-selected')
default:
assertNever(options.selected)
}
return expect(item).toHaveAttribute('aria-selected', options.selected ? 'true' : 'false')
}
} catch (err) {
if (err instanceof Error) Error.captureStackTrace(err, assertListboxOption)