Skip to content

Commit

Permalink
Allow Tab and Shift+Tab when Listbox is open (#3284)
Browse files Browse the repository at this point in the history
* allow `Tab` and `Shift+Tab` in `Listbox` component

This will make it consistent with the `Menu` and the ARIA Authoring
Practices Guide.

* update tests

* update changelog
  • Loading branch information
RobinMalfait authored Jun 11, 2024
1 parent 6b6c259 commit 1e9a3f1
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 31 deletions.
4 changes: 4 additions & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Ensure `ComboboxInput` does not sync with current value while typing ([#3259](https://github.com/tailwindlabs/headlessui/pull/3259))
- Cancel outside click behavior on touch devices when scrolling ([#3266](https://github.com/tailwindlabs/headlessui/pull/3266))

### Changed

- Allow using the `Tab` and `Shift+Tab` keys when the `Listbox` component is open ([#3284](https://github.com/tailwindlabs/headlessui/pull/3284))

## [2.0.4] - 2024-05-25

### Fixed
Expand Down
60 changes: 33 additions & 27 deletions packages/@headlessui-react/src/components/listbox/listbox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1956,17 +1956,20 @@ describe('Keyboard interactions', () => {

describe('`Tab` key', () => {
it(
'should focus trap when we use Tab',
'should not focus trap when we use Tab',
suppressConsoleLogs(async () => {
render(
<Listbox value={undefined} onChange={(x) => console.log(x)}>
<Listbox.Button>Trigger</Listbox.Button>
<Listbox.Options>
<Listbox.Option value="a">Option A</Listbox.Option>
<Listbox.Option value="b">Option B</Listbox.Option>
<Listbox.Option value="c">Option C</Listbox.Option>
</Listbox.Options>
</Listbox>
<>
<Listbox value={undefined} onChange={(x) => console.log(x)}>
<Listbox.Button>Trigger</Listbox.Button>
<Listbox.Options>
<Listbox.Option value="a">Option A</Listbox.Option>
<Listbox.Option value="b">Option B</Listbox.Option>
<Listbox.Option value="c">Option C</Listbox.Option>
</Listbox.Options>
</Listbox>
<a href="#">After</a>
</>
)

assertListboxButton({
Expand Down Expand Up @@ -1996,28 +1999,31 @@ describe('Keyboard interactions', () => {
options.forEach((option) => assertListboxOption(option))
assertActiveListboxOption(options[0])

// Try to tab
// Tab to the next element
await press(Keys.Tab)

// Verify it is still open
assertListboxButton({ state: ListboxState.Visible })
assertListbox({ state: ListboxState.Visible })
assertActiveElement(getListbox())
// Verify the listbox is closed
assertListboxButton({ state: ListboxState.InvisibleUnmounted })
assertListbox({ state: ListboxState.InvisibleUnmounted })
assertActiveElement(getByText('After'))
})
)

it(
'should focus trap when we use Shift+Tab',
'should not focus trap when we use Shift+Tab',
suppressConsoleLogs(async () => {
render(
<Listbox value={undefined} onChange={(x) => console.log(x)}>
<Listbox.Button>Trigger</Listbox.Button>
<Listbox.Options>
<Listbox.Option value="a">Option A</Listbox.Option>
<Listbox.Option value="b">Option B</Listbox.Option>
<Listbox.Option value="c">Option C</Listbox.Option>
</Listbox.Options>
</Listbox>
<>
<a href="#">Before</a>
<Listbox value={undefined} onChange={(x) => console.log(x)}>
<Listbox.Button>Trigger</Listbox.Button>
<Listbox.Options>
<Listbox.Option value="a">Option A</Listbox.Option>
<Listbox.Option value="b">Option B</Listbox.Option>
<Listbox.Option value="c">Option C</Listbox.Option>
</Listbox.Options>
</Listbox>
</>
)

assertListboxButton({
Expand Down Expand Up @@ -2050,10 +2056,10 @@ describe('Keyboard interactions', () => {
// Try to Shift+Tab
await press(shift(Keys.Tab))

// Verify it is still open
assertListboxButton({ state: ListboxState.Visible })
assertListbox({ state: ListboxState.Visible })
assertActiveElement(getListbox())
// Verify the listbox is closed
assertListboxButton({ state: ListboxState.InvisibleUnmounted })
assertListbox({ state: ListboxState.InvisibleUnmounted })
assertActiveElement(getByText('Before'))
})
)
})
Expand Down
18 changes: 16 additions & 2 deletions packages/@headlessui-react/src/components/listbox/listbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,13 @@ import type { EnsureArray, Props } from '../../types'
import { isDisabledReactIssue7711 } from '../../utils/bugs'
import { Focus, calculateActiveIndex } from '../../utils/calculate-active-index'
import { disposables } from '../../utils/disposables'
import { FocusableMode, isFocusableElement, sortByDomNode } from '../../utils/focus-management'
import {
Focus as FocusManagementFocus,
FocusableMode,
focusFrom,
isFocusableElement,
sortByDomNode,
} from '../../utils/focus-management'
import { attemptSubmit } from '../../utils/form'
import { match } from '../../utils/match'
import { getOwnerDocument } from '../../utils/owner'
Expand Down Expand Up @@ -1064,6 +1070,11 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
case Keys.Tab:
event.preventDefault()
event.stopPropagation()
flushSync(() => actions.closeListbox())
focusFrom(
data.buttonRef.current!,
event.shiftKey ? FocusManagementFocus.Previous : FocusManagementFocus.Next
)
break

default:
Expand Down Expand Up @@ -1093,7 +1104,10 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
'aria-orientation': data.orientation,
onKeyDown: handleKeyDown,
role: 'listbox',
tabIndex: 0,
// When the `Listbox` is closed, it should not be focusable. This allows us
// to skip focusing the `ListboxOptions` when pressing the tab key on an
// open `Listbox`, and go to the next focusable element.
tabIndex: data.listboxState === ListboxStates.Open ? 0 : undefined,
style: {
...theirProps.style,
...style,
Expand Down
7 changes: 5 additions & 2 deletions packages/@headlessui-react/src/components/menu/menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ function ItemsFn<TTag extends ElementType = typeof DEFAULT_ITEMS_TAG>(
open: state.menuState === MenuStates.Open,
...transitionData,
} satisfies ItemsRenderPropArg
}, [state, transitionData])
}, [state.menuState, transitionData])

let ourProps = mergeProps(anchor ? getFloatingPanelProps() : {}, {
'aria-activedescendant':
Expand All @@ -772,7 +772,10 @@ function ItemsFn<TTag extends ElementType = typeof DEFAULT_ITEMS_TAG>(
onKeyDown: handleKeyDown,
onKeyUp: handleKeyUp,
role: 'menu',
tabIndex: 0,
// When the `Menu` is closed, it should not be focusable. This allows us
// to skip focusing the `MenuItems` when pressing the tab key on an
// open `Menu`, and go to the next focusable element.
tabIndex: state.menuState === MenuStates.Open ? 0 : undefined,
ref: itemsRef,
style: {
...theirProps.style,
Expand Down

0 comments on commit 1e9a3f1

Please sign in to comment.