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

Fix Tab key with non focusable elements in Popover.Panel #2147

Merged
merged 3 commits into from
Jan 4, 2023
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
- 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 `<Popover.Button>` 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

Expand Down
34 changes: 34 additions & 0 deletions packages/@headlessui-react/src/components/popover/popover.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<>
<a href="/">Previous</a>

<Popover>
<Popover.Button>Trigger 1</Popover.Button>
<Popover.Panel>No focusable elements here</Popover.Panel>
</Popover>

<a href="/">Next</a>
</>
)

// 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 () => {
Expand Down
38 changes: 31 additions & 7 deletions packages/@headlessui-react/src/components/popover/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -526,10 +527,21 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
if (!el) return

function run() {
match(direction.current, {
let result = match(direction.current, {
[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.current, {
[TabDirection.Forwards]: Focus.Next,
[TabDirection.Backwards]: Focus.Previous,
}),
{ relativeTo: state.button }
)
}
}

// TODO: Cleanup once we are using real browser tests
Expand All @@ -553,6 +565,7 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
<Hidden
id={sentinelId}
features={HiddenFeatures.Focusable}
data-headlessui-focus-guard
as="button"
type="button"
onFocus={handleFocus}
Expand Down Expand Up @@ -748,7 +761,12 @@ let Panel = forwardRefWithAs(function Panel<TTag extends ElementType = typeof DE
function run() {
match(direction.current, {
[TabDirection.Forwards]: () => {
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
Expand Down Expand Up @@ -785,18 +803,22 @@ let Panel = forwardRefWithAs(function Panel<TTag extends ElementType = typeof DE

// Ignore sentinel buttons and items inside the panel
for (let element of combined.slice()) {
if (
element?.id?.startsWith?.('headlessui-focus-sentinel-') ||
state.panel?.contains(element)
) {
if (element.dataset.headlessuiFocusGuard === 'true' || state.panel?.contains(element)) {
let idx = combined.indexOf(element)
if (idx !== -1) combined.splice(idx, 1)
}
}

focusIn(combined, Focus.First, { sorted: false })
},
[TabDirection.Backwards]: () => 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()
}
},
})
}

Expand All @@ -815,6 +837,7 @@ let Panel = forwardRefWithAs(function Panel<TTag extends ElementType = typeof DE
id={beforePanelSentinelId}
ref={state.beforePanelSentinel}
features={HiddenFeatures.Focusable}
data-headlessui-focus-guard
as="button"
type="button"
onFocus={handleBeforeFocus}
Expand All @@ -834,6 +857,7 @@ let Panel = forwardRefWithAs(function Panel<TTag extends ElementType = typeof DE
id={afterPanelSentinelId}
ref={state.afterPanelSentinel}
features={HiddenFeatures.Focusable}
data-headlessui-focus-guard
as="button"
type="button"
onFocus={handleAfterFocus}
Expand Down
1 change: 1 addition & 0 deletions packages/@headlessui-vue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Ensure `disabled="false"` is not incorrectly passed to the underlying DOM Node ([#2138](https://github.com/tailwindlabs/headlessui/pull/2138))
- Fix arrow key handling in `Tab` (after DOM order changes) ([#2145](https://github.com/tailwindlabs/headlessui/pull/2145))
- Fix `Tab` key with non focusable elements in `Popover.Panel` ([#2147](https://github.com/tailwindlabs/headlessui/pull/2147))

## [1.7.7] - 2022-12-16

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1475,7 +1475,7 @@ describe('Mouse interactions', () => {
})
)

fit(
it(
'should be possible to click elements inside the dialog when they reside inside a shadow boundary',
suppressConsoleLogs(async () => {
let fn = jest.fn()
Expand Down
36 changes: 35 additions & 1 deletion packages/@headlessui-vue/src/components/popover/popover.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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`
<div>
<Popover>
<PopoverButton>Trigger 1</PopoverButton>
<PopoverPanel>No focusable elements here</PopoverPanel>
</Popover>

<a href="/">Next</a>
</div>
`
)

// 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 () => {
Expand Down
38 changes: 31 additions & 7 deletions packages/@headlessui-vue/src/components/popover/popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -623,18 +641,22 @@ 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)
}
}

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()
}
},
})
}

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down