From 1e9a3f1640712c58a86e70fe38b33e426e77812f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 12 Jun 2024 01:02:22 +0200 Subject: [PATCH] Allow `Tab` and `Shift+Tab` when `Listbox` is open (#3284) * 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 --- packages/@headlessui-react/CHANGELOG.md | 4 ++ .../src/components/listbox/listbox.test.tsx | 60 ++++++++++--------- .../src/components/listbox/listbox.tsx | 18 +++++- .../src/components/menu/menu.tsx | 7 ++- 4 files changed, 58 insertions(+), 31 deletions(-) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index e912d7076c..bcfe314bd5 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -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 diff --git a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx index a062a821c5..f603344b52 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx @@ -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( - console.log(x)}> - Trigger - - Option A - Option B - Option C - - + <> + console.log(x)}> + Trigger + + Option A + Option B + Option C + + + After + ) assertListboxButton({ @@ -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( - console.log(x)}> - Trigger - - Option A - Option B - Option C - - + <> + Before + console.log(x)}> + Trigger + + Option A + Option B + Option C + + + ) assertListboxButton({ @@ -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')) }) ) }) diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index e0c45daffa..8a66aec1e4 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -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' @@ -1064,6 +1070,11 @@ function OptionsFn( case Keys.Tab: event.preventDefault() event.stopPropagation() + flushSync(() => actions.closeListbox()) + focusFrom( + data.buttonRef.current!, + event.shiftKey ? FocusManagementFocus.Previous : FocusManagementFocus.Next + ) break default: @@ -1093,7 +1104,10 @@ function OptionsFn( '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, diff --git a/packages/@headlessui-react/src/components/menu/menu.tsx b/packages/@headlessui-react/src/components/menu/menu.tsx index 3619aac8ed..2b40a8c49f 100644 --- a/packages/@headlessui-react/src/components/menu/menu.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.tsx @@ -762,7 +762,7 @@ function ItemsFn( open: state.menuState === MenuStates.Open, ...transitionData, } satisfies ItemsRenderPropArg - }, [state, transitionData]) + }, [state.menuState, transitionData]) let ourProps = mergeProps(anchor ? getFloatingPanelProps() : {}, { 'aria-activedescendant': @@ -772,7 +772,10 @@ function ItemsFn( 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,