From d31bb5c08e51dcf72dd981a059a056e2cb3c194b Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 14 Dec 2022 16:26:38 +0100 Subject: [PATCH] Fix FocusTrap escape due to strange tabindex values (#2093) * sort DOM nodes using tabIndex first It will still keep the same DOM order if tabIndex matches, thanks to stable sorts! * refactor `focusIn` API All the arguments resulted in usage like `focusIn(container, Focus.First, true, null)`, and to make things worse, we need to add something else to this list in the future. Instead, let's keep the `container` and the type of `Focus` as known params, all the other things can sit in an options object. * fix FocusTrap escape due to strange tabindex values This code will now ensure that we can't escape the FocusTrap if you use `` and you happen to tab to an element outside of the FocusTrap because the next item in line happens to be outside of the FocusTrap and we never hit any of the focus guard elements. How it works is as follows: 1. The `onBlur` is implemented on the `FocusTrap` itself, this will give us some information in the event itself. - `e.target` is the element that is being blurred (think of it as `from`) - `e.currentTarget` is the element with the event listener (the dialog) - `e.relatedTarget` is the element we are going to (think of it as `to`) 2. If the blur happened due to a `` or `+`, then we will move focus back inside the FocusTrap, and go from the `e.target` to the next or previous value. 3. If the blur happened programmatically (so no tab keys are involved, aka no direction is known), then the focus is restored to the `e.target` value. Fixes: #1656 * update changelog --- packages/@headlessui-react/CHANGELOG.md | 1 + .../src/components/dialog/dialog.test.tsx | 6 +- .../components/focus-trap/focus-trap.test.tsx | 197 ++++++++++++------ .../src/components/focus-trap/focus-trap.tsx | 72 +++++-- .../src/components/popover/popover.tsx | 2 +- .../src/utils/focus-management.ts | 26 ++- packages/@headlessui-vue/CHANGELOG.md | 1 + .../src/components/dialog/dialog.test.ts | 8 +- .../components/focus-trap/focus-trap.test.ts | 121 ++++++----- .../src/components/focus-trap/focus-trap.ts | 67 ++++-- .../src/components/popover/popover.ts | 2 +- .../src/utils/focus-management.ts | 26 ++- 12 files changed, 347 insertions(+), 182 deletions(-) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 813d61b29e..f96c0f13af 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix regression where `displayValue` crashes ([#2087](https://github.com/tailwindlabs/headlessui/pull/2087)) - Fix `displayValue` syncing when `Combobox.Input` is unmounted and re-mounted in different trees ([#2090](https://github.com/tailwindlabs/headlessui/pull/2090)) +- Fix FocusTrap escape due to strange tabindex values ([#2093](https://github.com/tailwindlabs/headlessui/pull/2093)) ## [1.7.5] - 2022-12-08 diff --git a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx index cc6d15874f..1f3efc93b1 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx @@ -218,7 +218,6 @@ describe('Rendering', () => { }) it('should be possible to use a different render strategy for the Dialog', async () => { - let focusCounter = jest.fn() function Example() { let [isOpen, setIsOpen] = useState(false) @@ -228,7 +227,7 @@ describe('Rendering', () => { Trigger - + ) @@ -239,17 +238,14 @@ describe('Rendering', () => { await nextFrame() assertDialog({ state: DialogState.InvisibleHidden }) - expect(focusCounter).toHaveBeenCalledTimes(0) // Let's open the Dialog, to see if it is not hidden anymore await click(document.getElementById('trigger')) - expect(focusCounter).toHaveBeenCalledTimes(1) assertDialog({ state: DialogState.Visible }) // Let's close the Dialog await press(Keys.Escape) - expect(focusCounter).toHaveBeenCalledTimes(1) assertDialog({ state: DialogState.InvisibleHidden }) }) diff --git a/packages/@headlessui-react/src/components/focus-trap/focus-trap.test.tsx b/packages/@headlessui-react/src/components/focus-trap/focus-trap.test.tsx index 23a5a4e777..7f43d5ccd9 100644 --- a/packages/@headlessui-react/src/components/focus-trap/focus-trap.test.tsx +++ b/packages/@headlessui-react/src/components/focus-trap/focus-trap.test.tsx @@ -1,4 +1,4 @@ -import React, { useState, useRef, FocusEvent } from 'react' +import React, { useState, useRef } from 'react' import { render, screen } from '@testing-library/react' import { FocusTrap } from './focus-trap' @@ -6,6 +6,13 @@ import { assertActiveElement } from '../../test-utils/accessibility-assertions' import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs' import { click, press, shift, Keys } from '../../test-utils/interactions' +beforeAll(() => { + jest.spyOn(window, 'requestAnimationFrame').mockImplementation(setImmediate as any) + jest.spyOn(window, 'cancelAnimationFrame').mockImplementation(clearImmediate as any) +}) + +afterAll(() => jest.restoreAllMocks()) + function nextFrame() { return new Promise((resolve) => { requestAnimationFrame(() => { @@ -365,76 +372,134 @@ it('should be possible skip disabled elements within the focus trap', async () = assertActiveElement(document.getElementById('item-a')) }) -it('should try to focus all focusable items (and fail)', async () => { - let spy = jest.spyOn(console, 'warn').mockImplementation(jest.fn()) - let focusHandler = jest.fn() - function handleFocus(e: FocusEvent) { - let target = e.target as HTMLElement - focusHandler(target.id) - screen.getByText('After')?.focus() - } +it( + 'should not be possible to programmatically escape the focus trap', + suppressConsoleLogs(async () => { + function Example() { + return ( + <> + - render( - <> - - - - - - - - - - ) + + + + + + + ) + } - await nextFrame() + render() - expect(focusHandler.mock.calls).toEqual([['item-a'], ['item-b'], ['item-c'], ['item-d']]) - expect(spy).toHaveBeenCalledWith('There are no focusable elements inside the ') - spy.mockReset() -}) + await nextFrame() -it('should end up at the last focusable element', async () => { - let spy = jest.spyOn(console, 'warn').mockImplementation(jest.fn()) + let [a, b, c, d] = Array.from(document.querySelectorAll('input')) - let focusHandler = jest.fn() - function handleFocus(e: FocusEvent) { - let target = e.target as HTMLElement - focusHandler(target.id) - screen.getByText('After')?.focus() - } + // Ensure that input-b is the active element + assertActiveElement(b) - render( - <> - - - - - - - - - - ) + // Tab to the next item + await press(Keys.Tab) - await nextFrame() + // Ensure that input-c is the active element + assertActiveElement(c) - expect(focusHandler.mock.calls).toEqual([['item-a'], ['item-b'], ['item-c']]) - assertActiveElement(screen.getByText('Item D')) - expect(spy).not.toHaveBeenCalled() - spy.mockReset() -}) + // Try to move focus + a?.focus() + + // Ensure that input-c is still the active element + assertActiveElement(c) + + // Click on an element within the FocusTrap + await click(b) + + // Ensure that input-b is the active element + assertActiveElement(b) + + // Try to move focus again + a?.focus() + + // Ensure that input-b is still the active element + assertActiveElement(b) + + // Focus on an element within the FocusTrap + d?.focus() + + // Ensure that input-d is the active element + assertActiveElement(d) + + // Try to move focus again + a?.focus() + + // Ensure that input-d is still the active element + assertActiveElement(d) + }) +) + +it( + 'should not be possible to escape the FocusTrap due to strange tabIndex usage', + suppressConsoleLogs(async () => { + function Example() { + return ( + <> +
+ + +
+ + + + + + + ) + } + + render() + + await nextFrame() + + let [_a, _b, c, d] = Array.from(document.querySelectorAll('input')) + + // First item in the FocusTrap should be the active one + assertActiveElement(c) + + // Tab to the next item + await press(Keys.Tab) + + // Ensure that input-d is the active element + assertActiveElement(d) + + // Tab to the next item + await press(Keys.Tab) + + // Ensure that input-c is the active element + assertActiveElement(c) + + // Tab to the next item + await press(Keys.Tab) + + // Ensure that input-d is the active element + assertActiveElement(d) + + // Let's go the other way + + // Tab to the previous item + await press(shift(Keys.Tab)) + + // Ensure that input-c is the active element + assertActiveElement(c) + + // Tab to the previous item + await press(shift(Keys.Tab)) + + // Ensure that input-d is the active element + assertActiveElement(d) + + // Tab to the previous item + await press(shift(Keys.Tab)) + + // Ensure that input-c is the active element + assertActiveElement(c) + }) +) diff --git a/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx b/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx index 2db6dc6679..a0a60339f8 100644 --- a/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx +++ b/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx @@ -6,6 +6,7 @@ import React, { ElementType, MutableRefObject, Ref, + FocusEvent as ReactFocusEvent, } from 'react' import { Props } from '../../types' @@ -22,6 +23,7 @@ import { useOwnerDocument } from '../../hooks/use-owner' import { useEventListener } from '../../hooks/use-event-listener' import { microTask } from '../../utils/micro-task' import { useWatch } from '../../hooks/use-watch' +import { useDisposables } from '../../hooks/use-disposables' let DEFAULT_FOCUS_TRAP_TAG = 'div' as const @@ -75,27 +77,69 @@ export let FocusTrap = Object.assign( ) let direction = useTabDirection() - let handleFocus = useEvent(() => { + let handleFocus = useEvent((e: ReactFocusEvent) => { let el = container.current as HTMLElement if (!el) return // TODO: Cleanup once we are using real browser tests - if (process.env.NODE_ENV === 'test') { - microTask(() => { - match(direction.current, { - [TabDirection.Forwards]: () => focusIn(el, Focus.First), - [TabDirection.Backwards]: () => focusIn(el, Focus.Last), - }) - }) - } else { + let wrapper = process.env.NODE_ENV === 'test' ? microTask : (cb: Function) => cb() + wrapper(() => { match(direction.current, { - [TabDirection.Forwards]: () => focusIn(el, Focus.First), - [TabDirection.Backwards]: () => focusIn(el, Focus.Last), + [TabDirection.Forwards]: () => + focusIn(el, Focus.First, { skipElements: [e.relatedTarget as HTMLElement] }), + [TabDirection.Backwards]: () => + focusIn(el, Focus.Last, { skipElements: [e.relatedTarget as HTMLElement] }), }) - } + }) }) - let ourProps = { ref: focusTrapRef } + let d = useDisposables() + let recentlyUsedTabKey = useRef(false) + let ourProps = { + ref: focusTrapRef, + onKeyDown(e: KeyboardEvent) { + if (e.key == 'Tab') { + recentlyUsedTabKey.current = true + d.requestAnimationFrame(() => { + recentlyUsedTabKey.current = false + }) + } + }, + onBlur(e: ReactFocusEvent) { + let allContainers = new Set(containers?.current) + allContainers.add(container) + + let relatedTarget = e.relatedTarget as HTMLElement | null + if (!relatedTarget) return + + // Known guards, leave them alone! + if (relatedTarget.dataset.headlessuiFocusGuard === 'true') { + return + } + + // Blur is triggered due to focus on relatedTarget, and the relatedTarget is not inside any + // of the dialog containers. In other words, let's move focus back in! + if (!contains(allContainers, relatedTarget)) { + // Was the blur invoke via the keyboard? Redirect to the next in line. + if (recentlyUsedTabKey.current) { + focusIn( + container.current as HTMLElement, + match(direction.current, { + [TabDirection.Forwards]: () => Focus.Next, + [TabDirection.Backwards]: () => Focus.Previous, + }) | Focus.WrapAround, + { relativeTo: e.target as HTMLElement } + ) + } + + // It was invoke via something else (e.g.: click, programmatically, ...). Redirect to the + // previous active item in the FocusTrap + else if (e.target instanceof HTMLElement) { + focusElement(e.target) + } + } + }, + } return ( <> @@ -103,6 +147,7 @@ export let FocusTrap = Object.assign( @@ -117,6 +162,7 @@ export let FocusTrap = Object.assign( diff --git a/packages/@headlessui-react/src/components/popover/popover.tsx b/packages/@headlessui-react/src/components/popover/popover.tsx index 17f74bb61b..8b393c97e1 100644 --- a/packages/@headlessui-react/src/components/popover/popover.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.tsx @@ -792,7 +792,7 @@ let Panel = forwardRefWithAs(function Panel focusIn(el, Focus.Last), }) diff --git a/packages/@headlessui-react/src/utils/focus-management.ts b/packages/@headlessui-react/src/utils/focus-management.ts index 0800fbde3e..ab8971ae38 100644 --- a/packages/@headlessui-react/src/utils/focus-management.ts +++ b/packages/@headlessui-react/src/utils/focus-management.ts @@ -66,7 +66,11 @@ enum Direction { export function getFocusableElements(container: HTMLElement | null = document.body) { if (container == null) return [] - return Array.from(container.querySelectorAll(focusableSelector)) + return Array.from(container.querySelectorAll(focusableSelector)).sort( + // We want to move `tabIndex={0}` to the end of the list, this is what the browser does as well. + (a, z) => + Math.sign((a.tabIndex || Number.MAX_SAFE_INTEGER) - (z.tabIndex || Number.MAX_SAFE_INTEGER)) + ) } export enum FocusableMode { @@ -143,14 +147,17 @@ export function sortByDomNode( } export function focusFrom(current: HTMLElement | null, focus: Focus) { - return focusIn(getFocusableElements(), focus, true, current) + return focusIn(getFocusableElements(), focus, { relativeTo: current }) } export function focusIn( container: HTMLElement | HTMLElement[], focus: Focus, - sorted = true, - active: HTMLElement | null = null + { + sorted = true, + relativeTo = null, + skipElements = [], + }: Partial<{ sorted: boolean; relativeTo: HTMLElement | null; skipElements: HTMLElement[] }> = {} ) { let ownerDocument = Array.isArray(container) ? container.length > 0 @@ -163,7 +170,12 @@ export function focusIn( ? sortByDomNode(container) : container : getFocusableElements(container) - active = active ?? (ownerDocument.activeElement as HTMLElement) + + if (skipElements.length > 0) { + elements = elements.filter((x) => !skipElements.includes(x)) + } + + relativeTo = relativeTo ?? (ownerDocument.activeElement as HTMLElement) let direction = (() => { if (focus & (Focus.First | Focus.Next)) return Direction.Next @@ -174,8 +186,8 @@ export function focusIn( let startIndex = (() => { if (focus & Focus.First) return 0 - if (focus & Focus.Previous) return Math.max(0, elements.indexOf(active)) - 1 - if (focus & Focus.Next) return Math.max(0, elements.indexOf(active)) + 1 + if (focus & Focus.Previous) return Math.max(0, elements.indexOf(relativeTo)) - 1 + if (focus & Focus.Next) return Math.max(0, elements.indexOf(relativeTo)) + 1 if (focus & Focus.Last) return elements.length - 1 throw new Error('Missing Focus.First, Focus.Previous, Focus.Next or Focus.Last') diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 52c1c24616..558ddf10fb 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix regression where `displayValue` crashes ([#2087](https://github.com/tailwindlabs/headlessui/pull/2087)) - Fix `displayValue` syncing when `Combobox.Input` is unmounted and re-mounted in different trees ([#2090](https://github.com/tailwindlabs/headlessui/pull/2090)) +- Fix FocusTrap escape due to strange tabindex values ([#2093](https://github.com/tailwindlabs/headlessui/pull/2093)) ## [1.7.5] - 2022-12-08 diff --git a/packages/@headlessui-vue/src/components/dialog/dialog.test.ts b/packages/@headlessui-vue/src/components/dialog/dialog.test.ts index 22358c55b0..c54ff3f796 100644 --- a/packages/@headlessui-vue/src/components/dialog/dialog.test.ts +++ b/packages/@headlessui-vue/src/components/dialog/dialog.test.ts @@ -312,20 +312,18 @@ describe('Rendering', () => { }) it('should be possible to use a different render strategy for the Dialog', async () => { - let focusCounter = jest.fn() renderTemplate({ template: `
- +
`, setup() { let isOpen = ref(false) return { - focusCounter, isOpen, setIsOpen(value: boolean) { isOpen.value = value @@ -337,19 +335,15 @@ describe('Rendering', () => { await nextFrame() assertDialog({ state: DialogState.InvisibleHidden }) - expect(focusCounter).toHaveBeenCalledTimes(0) // Let's open the Dialog, to see if it is not hidden anymore await click(document.getElementById('trigger')) - expect(focusCounter).toHaveBeenCalledTimes(1) assertDialog({ state: DialogState.Visible }) // Let's close the Dialog await press(Keys.Escape) - expect(focusCounter).toHaveBeenCalledTimes(1) - assertDialog({ state: DialogState.InvisibleHidden }) }) diff --git a/packages/@headlessui-vue/src/components/focus-trap/focus-trap.test.ts b/packages/@headlessui-vue/src/components/focus-trap/focus-trap.test.ts index 8ae1d202a1..c7996c7ee3 100644 --- a/packages/@headlessui-vue/src/components/focus-trap/focus-trap.test.ts +++ b/packages/@headlessui-vue/src/components/focus-trap/focus-trap.test.ts @@ -387,73 +387,68 @@ it('should be possible skip disabled elements within the focus trap', async () = assertActiveElement(document.getElementById('item-a')) }) -it('should try to focus all focusable items in order (and fail)', async () => { - let spy = jest.spyOn(console, 'warn').mockImplementation(jest.fn()) - let focusHandler = jest.fn() +it( + 'should not be possible to escape the FocusTrap due to strange tabIndex usage', + suppressConsoleLogs(async () => { + renderTemplate( + html` +
+
+ + +
- renderTemplate({ - template: html` -
- - - - - - - - -
- `, - setup() { - return { - handleFocus(e: Event) { - let target = e.target as HTMLElement - focusHandler(target.id) - getByText('After')?.focus() - }, - } - }, - }) + + + + +
+ ` + ) - await nextFrame() + await nextFrame() - expect(focusHandler.mock.calls).toEqual([['item-a'], ['item-b'], ['item-c'], ['item-d']]) - expect(spy).toHaveBeenCalledWith('There are no focusable elements inside the ') - spy.mockReset() -}) + let [_a, _b, c, d] = Array.from(document.querySelectorAll('input')) -it('should end up at the last focusable element', async () => { - let spy = jest.spyOn(console, 'warn').mockImplementation(jest.fn()) - let focusHandler = jest.fn() + // First item in the FocusTrap should be the active one + assertActiveElement(c) - renderTemplate({ - template: html` -
- - - - - - - - -
- `, - setup() { - return { - handleFocus(e: Event) { - let target = e.target as HTMLElement - focusHandler(target.id) - getByText('After')?.focus() - }, - } - }, - }) + // Tab to the next item + await press(Keys.Tab) - await nextFrame() + // Ensure that input-d is the active element + assertActiveElement(d) - expect(focusHandler.mock.calls).toEqual([['item-a'], ['item-b'], ['item-c']]) - assertActiveElement(getByText('Item D')) - expect(spy).not.toHaveBeenCalled() - spy.mockReset() -}) + // Tab to the next item + await press(Keys.Tab) + + // Ensure that input-c is the active element + assertActiveElement(c) + + // Tab to the next item + await press(Keys.Tab) + + // Ensure that input-d is the active element + assertActiveElement(d) + + // Let's go the other way + + // Tab to the previous item + await press(shift(Keys.Tab)) + + // Ensure that input-c is the active element + assertActiveElement(c) + + // Tab to the previous item + await press(shift(Keys.Tab)) + + // Ensure that input-d is the active element + assertActiveElement(d) + + // Tab to the previous item + await press(shift(Keys.Tab)) + + // Ensure that input-c is the active element + assertActiveElement(c) + }) +) diff --git a/packages/@headlessui-vue/src/components/focus-trap/focus-trap.ts b/packages/@headlessui-vue/src/components/focus-trap/focus-trap.ts index 8b6b421481..faa6501426 100644 --- a/packages/@headlessui-vue/src/components/focus-trap/focus-trap.ts +++ b/packages/@headlessui-vue/src/components/focus-trap/focus-trap.ts @@ -81,29 +81,70 @@ export let FocusTrap = Object.assign( ) let direction = useTabDirection() - function handleFocus() { + function handleFocus(e: FocusEvent) { let el = dom(container) as HTMLElement if (!el) return // TODO: Cleanup once we are using real browser tests - if (process.env.NODE_ENV === 'test') { - microTask(() => { - match(direction.value, { - [TabDirection.Forwards]: () => focusIn(el, Focus.First), - [TabDirection.Backwards]: () => focusIn(el, Focus.Last), - }) - }) - } else { + let wrapper = process.env.NODE_ENV === 'test' ? microTask : (cb: Function) => cb() + wrapper(() => { match(direction.value, { - [TabDirection.Forwards]: () => focusIn(el, Focus.First), - [TabDirection.Backwards]: () => focusIn(el, Focus.Last), + [TabDirection.Forwards]: () => + focusIn(el, Focus.First, { skipElements: [e.relatedTarget as HTMLElement] }), + [TabDirection.Backwards]: () => + focusIn(el, Focus.Last, { skipElements: [e.relatedTarget as HTMLElement] }), + }) + }) + } + + let recentlyUsedTabKey = ref(false) + function handleKeyDown(e: KeyboardEvent) { + if (e.key === 'Tab') { + recentlyUsedTabKey.value = true + requestAnimationFrame(() => { + recentlyUsedTabKey.value = false }) } } + function handleBlur(e: FocusEvent) { + let allContainers = new Set(props.containers?.value) + allContainers.add(container) + + let relatedTarget = e.relatedTarget as HTMLElement | null + if (!relatedTarget) return + + // Known guards, leave them alone! + if (relatedTarget.dataset.headlessuiFocusGuard === 'true') { + return + } + + // Blur is triggered due to focus on relatedTarget, and the relatedTarget is not inside any + // of the dialog containers. In other words, let's move focus back in! + if (!contains(allContainers, relatedTarget)) { + // Was the blur invoke via the keyboard? Redirect to the next in line. + if (recentlyUsedTabKey.value) { + focusIn( + dom(container) as HTMLElement, + match(direction.value, { + [TabDirection.Forwards]: () => Focus.Next, + [TabDirection.Backwards]: () => Focus.Previous, + }) | Focus.WrapAround, + { relativeTo: e.target as HTMLElement } + ) + } + + // It was invoke via something else (e.g.: click, programmatically, ...). Redirect to the + // previous active item in the FocusTrap + else if (e.target instanceof HTMLElement) { + focusElement(e.target) + } + } + } + return () => { let slot = {} - let ourProps = { ref: container } + let ourProps = { ref: container, onKeydown: handleKeyDown, onFocusout: handleBlur } let { features, initialFocus, containers: _containers, ...theirProps } = props return h(Fragment, [ @@ -111,6 +152,7 @@ export let FocusTrap = Object.assign( h(Hidden, { as: 'button', type: 'button', + 'data-headlessui-focus-guard': true, onFocus: handleFocus, features: HiddenFeatures.Focusable, }), @@ -126,6 +168,7 @@ export let FocusTrap = Object.assign( h(Hidden, { as: 'button', type: 'button', + 'data-headlessui-focus-guard': true, onFocus: handleFocus, features: HiddenFeatures.Focusable, }), diff --git a/packages/@headlessui-vue/src/components/popover/popover.ts b/packages/@headlessui-vue/src/components/popover/popover.ts index b5bb8ca3b4..f767ec89bc 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.ts @@ -632,7 +632,7 @@ export let PopoverPanel = defineComponent({ } } - focusIn(combined, Focus.First, false) + focusIn(combined, Focus.First, { sorted: false }) }, [TabDirection.Backwards]: () => focusIn(el, Focus.Previous), }) diff --git a/packages/@headlessui-vue/src/utils/focus-management.ts b/packages/@headlessui-vue/src/utils/focus-management.ts index c8abd5836d..9ec2cd5f72 100644 --- a/packages/@headlessui-vue/src/utils/focus-management.ts +++ b/packages/@headlessui-vue/src/utils/focus-management.ts @@ -59,7 +59,11 @@ enum Direction { export function getFocusableElements(container: HTMLElement | null = document.body) { if (container == null) return [] - return Array.from(container.querySelectorAll(focusableSelector)) + return Array.from(container.querySelectorAll(focusableSelector)).sort( + // We want to move `:tabindex="0"` to the end of the list, this is what the browser does as well. + (a, z) => + Math.sign((a.tabIndex || Number.MAX_SAFE_INTEGER) - (z.tabIndex || Number.MAX_SAFE_INTEGER)) + ) } export enum FocusableMode { @@ -136,14 +140,17 @@ export function sortByDomNode( } export function focusFrom(current: HTMLElement | null, focus: Focus) { - return focusIn(getFocusableElements(), focus, true, current) + return focusIn(getFocusableElements(), focus, { relativeTo: current }) } export function focusIn( container: HTMLElement | HTMLElement[], focus: Focus, - sorted = true, - active: HTMLElement | null = null + { + sorted = true, + relativeTo = null, + skipElements = [], + }: Partial<{ sorted: boolean; relativeTo: HTMLElement | null; skipElements: HTMLElement[] }> = {} ) { let ownerDocument = (Array.isArray(container) @@ -157,7 +164,12 @@ export function focusIn( ? sortByDomNode(container) : container : getFocusableElements(container) - active = active ?? (ownerDocument.activeElement as HTMLElement) + + if (skipElements.length > 0) { + elements = elements.filter((x) => !skipElements.includes(x)) + } + + relativeTo = relativeTo ?? (ownerDocument.activeElement as HTMLElement) let direction = (() => { if (focus & (Focus.First | Focus.Next)) return Direction.Next @@ -168,8 +180,8 @@ export function focusIn( let startIndex = (() => { if (focus & Focus.First) return 0 - if (focus & Focus.Previous) return Math.max(0, elements.indexOf(active)) - 1 - if (focus & Focus.Next) return Math.max(0, elements.indexOf(active)) + 1 + if (focus & Focus.Previous) return Math.max(0, elements.indexOf(relativeTo)) - 1 + if (focus & Focus.Next) return Math.max(0, elements.indexOf(relativeTo)) + 1 if (focus & Focus.Last) return elements.length - 1 throw new Error('Missing Focus.First, Focus.Previous, Focus.Next or Focus.Last')