huanld/headlessui
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
* fix `Tab` key with non focusable elements in `Popover.Panel`

Fixes: #2112

* ensure all Dialog tests are running

* update changelog
This commit is contained in:
Robin Malfait
2023-01-04 19:08:47 +01:00
committed by GitHub
parent 69b953ae9d
commit 6fa6074cd5
7 changed files with 134 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/@headlessui-react/CHANGELOG.md
+1
View File
@@ -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
packages/@headlessui-react/src/components/popover/popover.test.tsx
+34
View File
@@ -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 () => {
packages/@headlessui-react/src/components/popover/popover.tsx
+31 -7
View File
@@ -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<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
@@ -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}
@@ -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
@@ -785,10 +803,7 @@ 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)
}
@@ -796,7 +811,14 @@ let Panel = forwardRefWithAs(function Panel<TTag extends ElementType = typeof DE
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()
}
},
})
}
@@ -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}
@@ -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}
packages/@headlessui-vue/CHANGELOG.md
+1
View File
@@ -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
packages/@headlessui-vue/src/components/dialog/dialog.test.ts
+1 -1
View File
@@ -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()
packages/@headlessui-vue/src/components/popover/popover.test.ts
+35 -1
View File
@@ -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`
<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 () => {
packages/@headlessui-vue/src/components/popover/popover.ts
+31 -7
View File
@@ -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,