diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 9c2ad31..1a49a7b 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix SSR tab rendering on React 17 ([#2102](https://github.com/tailwindlabs/headlessui/pull/2102)) - Fix arrow key handling in `Tab` (after DOM order changes) ([#2145](https://github.com/tailwindlabs/headlessui/pull/2145)) - Fix false positive warning about using multiple `` components ([#2146](https://github.com/tailwindlabs/headlessui/pull/2146)) +- Fix `Tab` key with non focusable elements in `Popover.Panel` ([#2147](https://github.com/tailwindlabs/headlessui/pull/2147)) ## [1.7.7] - 2022-12-16 diff --git a/packages/@headlessui-react/src/components/popover/popover.test.tsx b/packages/@headlessui-react/src/components/popover/popover.test.tsx index 9ed734d..fb6d760 100644 --- a/packages/@headlessui-react/src/components/popover/popover.test.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.test.tsx @@ -1335,6 +1335,40 @@ describe('Keyboard interactions', () => { }) ) + it( + 'should close the Popover menu once we Tab out of a Popover without focusable elements', + suppressConsoleLogs(async () => { + render( + <> + Previous + + + Trigger 1 + No focusable elements here + + + Next + + ) + + // Focus the button of the Popover + await focus(getPopoverButton()) + + // Open popover + await click(getPopoverButton()) + + // Let's Tab out of the Popover + await press(Keys.Tab) + + // Verify the next link is now focused + assertActiveElement(getByText('Next')) + + // Verify the popover is closed + assertPopoverButton({ state: PopoverState.InvisibleUnmounted }) + assertPopoverPanel({ state: PopoverState.InvisibleUnmounted }) + }) + ) + it( 'should close the Popover when the Popover.Panel has a focus prop', suppressConsoleLogs(async () => { diff --git a/packages/@headlessui-react/src/components/popover/popover.tsx b/packages/@headlessui-react/src/components/popover/popover.tsx index 31c9d79..a6f7176 100644 --- a/packages/@headlessui-react/src/components/popover/popover.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.tsx @@ -33,6 +33,7 @@ import { focusIn, isFocusableElement, FocusableMode, + FocusResult, } from '../../utils/focus-management' import { OpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' import { useResolveButtonType } from '../../hooks/use-resolve-button-type' @@ -526,10 +527,21 @@ let Button = forwardRefWithAs(function Button focusIn(el, Focus.First), [TabDirection.Backwards]: () => focusIn(el, Focus.Last), }) + + if (result === FocusResult.Error) { + focusIn( + getFocusableElements().filter((el) => el.dataset.headlessuiFocusGuard !== 'true'), + match(direction.current, { + [TabDirection.Forwards]: Focus.Next, + [TabDirection.Backwards]: Focus.Previous, + }), + { relativeTo: state.button } + ) + } } // TODO: Cleanup once we are using real browser tests @@ -553,6 +565,7 @@ let Button = forwardRefWithAs(function Button { - focusIn(el, Focus.First) + // Try to focus the first thing in the panel. But if that fails (e.g.: there are no + // focusable elements, then we can move outside of the panel) + let result = focusIn(el, Focus.First) + if (result === FocusResult.Error) { + state.afterPanelSentinel.current?.focus() + } }, [TabDirection.Backwards]: () => { // Coming from the Popover.Panel (which is portalled to somewhere else). Let's redirect @@ -785,10 +803,7 @@ let Panel = forwardRefWithAs(function Panel focusIn(el, Focus.Last), + [TabDirection.Backwards]: () => { + // Try to focus the first thing in the panel. But if that fails (e.g.: there are no + // focusable elements, then we can move outside of the panel) + let result = focusIn(el, Focus.Previous) + if (result === FocusResult.Error) { + state.button?.focus() + } + }, }) } @@ -815,6 +837,7 @@ let Panel = forwardRefWithAs(function Panel { }) ) - fit( + it( 'should be possible to click elements inside the dialog when they reside inside a shadow boundary', suppressConsoleLogs(async () => { let fn = jest.fn() diff --git a/packages/@headlessui-vue/src/components/popover/popover.test.ts b/packages/@headlessui-vue/src/components/popover/popover.test.ts index 126cfb5..f048460 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.test.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.test.ts @@ -15,7 +15,7 @@ import { assertContainsActiveElement, getPopoverOverlay, } from '../../test-utils/accessibility-assertions' -import { click, press, Keys, MouseButton, shift } from '../../test-utils/interactions' +import { click, focus, press, Keys, MouseButton, shift } from '../../test-utils/interactions' import { html } from '../../test-utils/html' import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' @@ -1369,6 +1369,40 @@ describe('Keyboard interactions', () => { }) ) + it( + 'should close the Popover menu once we Tab out of a Popover without focusable elements', + suppressConsoleLogs(async () => { + renderTemplate( + html` +
+ + Trigger 1 + No focusable elements here + + + Next +
+ ` + ) + + // Focus the button of the Popover + await focus(getPopoverButton()) + + // Open popover + await click(getPopoverButton()) + + // Let's Tab out of the Popover + await press(Keys.Tab) + + // Verify the next link is now focused + assertActiveElement(getByText('Next')) + + // Verify the popover is closed + assertPopoverButton({ state: PopoverState.InvisibleUnmounted }) + assertPopoverPanel({ state: PopoverState.InvisibleUnmounted }) + }) + ) + it( 'should close the Popover when the PopoverPanel has a focus prop', suppressConsoleLogs(async () => { diff --git a/packages/@headlessui-vue/src/components/popover/popover.ts b/packages/@headlessui-vue/src/components/popover/popover.ts index f767ec8..c0dd983 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.ts @@ -26,6 +26,7 @@ import { focusIn, isFocusableElement, FocusableMode, + FocusResult, } from '../../utils/focus-management' import { dom } from '../../utils/dom' import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' @@ -406,10 +407,21 @@ export let PopoverButton = defineComponent({ if (!el) return function run() { - match(direction.value, { + let result = match(direction.value, { [TabDirection.Forwards]: () => focusIn(el, Focus.First), [TabDirection.Backwards]: () => focusIn(el, Focus.Last), }) + + if (result === FocusResult.Error) { + focusIn( + getFocusableElements().filter((el) => el.dataset.headlessuiFocusGuard !== 'true'), + match(direction.value, { + [TabDirection.Forwards]: Focus.Next, + [TabDirection.Backwards]: Focus.Previous, + }), + { relativeTo: dom(api.button) } + ) + } } // TODO: Cleanup once we are using real browser tests @@ -435,6 +447,7 @@ export let PopoverButton = defineComponent({ h(Hidden, { id: sentinelId, features: HiddenFeatures.Focusable, + 'data-headlessui-focus-guard': true, as: 'button', type: 'button', onFocus: handleFocus, @@ -584,7 +597,12 @@ export let PopoverPanel = defineComponent({ function run() { match(direction.value, { [TabDirection.Forwards]: () => { - focusIn(el, Focus.Next) + // Try to focus the first thing in the panel. But if that fails (e.g.: there are no + // focusable elements, then we can move outside of the panel) + let result = focusIn(el, Focus.First) + if (result === FocusResult.Error) { + dom(api.afterPanelSentinel)?.focus() + } }, [TabDirection.Backwards]: () => { // Coming from the Popover.Panel (which is portalled to somewhere else). Let's redirect @@ -623,10 +641,7 @@ export let PopoverPanel = defineComponent({ // Ignore sentinel buttons and items inside the panel for (let element of combined.slice()) { - if ( - element?.id?.startsWith?.('headlessui-focus-sentinel-') || - panel?.contains(element) - ) { + if (element.dataset.headlessuiFocusGuard === 'true' || panel?.contains(element)) { let idx = combined.indexOf(element) if (idx !== -1) combined.splice(idx, 1) } @@ -634,7 +649,14 @@ export let PopoverPanel = defineComponent({ focusIn(combined, Focus.First, { sorted: false }) }, - [TabDirection.Backwards]: () => focusIn(el, Focus.Previous), + [TabDirection.Backwards]: () => { + // Try to focus the first thing in the panel. But if that fails (e.g.: there are no + // focusable elements, then we can move outside of the panel) + let result = focusIn(el, Focus.Previous) + if (result === FocusResult.Error) { + dom(api.button)?.focus() + } + }, }) } @@ -676,6 +698,7 @@ export let PopoverPanel = defineComponent({ id: beforePanelSentinelId, ref: api.beforePanelSentinel, features: HiddenFeatures.Focusable, + 'data-headlessui-focus-guard': true, as: 'button', type: 'button', onFocus: handleBeforeFocus, @@ -687,6 +710,7 @@ export let PopoverPanel = defineComponent({ id: afterPanelSentinelId, ref: api.afterPanelSentinel, features: HiddenFeatures.Focusable, + 'data-headlessui-focus-guard': true, as: 'button', type: 'button', onFocus: handleAfterFocus,