Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure tabbing to a portalled <PopoverPanel> component moves focus inside (without using <PortalGroup>) #3239

Merged
merged 3 commits into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [internal] Don’t set a focus fallback for Dialog’s in demo mode ([#3194](https://github.com/tailwindlabs/headlessui/pull/3194))
- Ensure page doesn't scroll down when pressing `Escape` to close the `Dialog` component ([#3218](https://github.com/tailwindlabs/headlessui/pull/3218))
- Fix crash when toggling between `virtual` and non-virtual mode in `Combobox` component ([#3236](https://github.com/tailwindlabs/headlessui/pull/3236))
- Ensure tabbing to a portalled `<PopoverPanel>` component moves focus inside (without using `<PortalGroup>`) ([#3239](https://github.com/tailwindlabs/headlessui/pull/3239))

### Deprecated

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { Keys, MouseButton, click, focus, press, shift } from '../../test-utils/
import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs'
import { Portal } from '../portal/portal'
import { Transition } from '../transition/transition'
import { Popover } from './popover'
import { Popover, PopoverButton, PopoverPanel } from './popover'

jest.mock('../../hooks/use-id')

Expand Down Expand Up @@ -1303,6 +1303,45 @@ describe('Keyboard interactions', () => {
})

describe('`Tab` key', () => {
it(
'should be possible to Tab through the panel contents and end up in the Button again (without PopoverGroup)',
suppressConsoleLogs(async () => {
render(
<Popover>
<PopoverButton>Trigger</PopoverButton>
<PopoverPanel portal>
<a href="/">Link 1</a>
<a href="/">Link 2</a>
</PopoverPanel>
</Popover>
)

// Focus the button of the first Popover
getByText('Trigger')?.focus()

// Open popover
await click(getByText('Trigger'))

// Verify we are focused on the first link
await press(Keys.Tab)
assertActiveElement(getByText('Link 1'))

// Verify we are focused on the second link
await press(Keys.Tab)
assertActiveElement(getByText('Link 2'))

// Let's Tab again
await press(Keys.Tab)

// Verify that the first Popover is still open
assertPopoverButton({ state: PopoverState.Visible })
assertPopoverPanel({ state: PopoverState.Visible })

// Verify that the button is focused again
assertActiveElement(getByText('Trigger'))
})
)

it(
'should be possible to Tab through the panel contents onto the next Popover.Button',
suppressConsoleLogs(async () => {
Expand Down
15 changes: 14 additions & 1 deletion packages/@headlessui-react/src/components/popover/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ interface StateDefinition {

beforePanelSentinel: MutableRefObject<HTMLButtonElement | null>
afterPanelSentinel: MutableRefObject<HTMLButtonElement | null>
afterButtonSentinel: MutableRefObject<HTMLButtonElement | null>

__demoMode: boolean
}
Expand Down Expand Up @@ -256,9 +257,19 @@ function PopoverFn<TTag extends ElementType = typeof DEFAULT_POPOVER_TAG>(
panelId: null,
beforePanelSentinel: createRef(),
afterPanelSentinel: createRef(),
afterButtonSentinel: createRef(),
} as StateDefinition)
let [
{ popoverState, button, buttonId, panel, panelId, beforePanelSentinel, afterPanelSentinel },
{
popoverState,
button,
buttonId,
panel,
panelId,
beforePanelSentinel,
afterPanelSentinel,
afterButtonSentinel,
},
dispatch,
] = reducerBag

Expand Down Expand Up @@ -346,6 +357,7 @@ function PopoverFn<TTag extends ElementType = typeof DEFAULT_POPOVER_TAG>(
if (root.contains(event.target)) return
if (beforePanelSentinel.current?.contains?.(event.target)) return
if (afterPanelSentinel.current?.contains?.(event.target)) return
if (afterButtonSentinel.current?.contains?.(event.target)) return

dispatch({ type: ActionTypes.ClosePopover })
},
Expand Down Expand Up @@ -700,6 +712,7 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
{visible && !isWithinPanel && isPortalled && (
<Hidden
id={sentinelId}
ref={state.afterButtonSentinel}
features={HiddenFeatures.Focusable}
data-headlessui-focus-guard
as="button"
Expand Down
Loading