Skip to content

Commit dbcfb23

Browse files
RobinMalfaitRobinMalfait
authored
Fix FocusTrap in Dialog when there is only 1 focusable element (#2172)
* add tests to guarantee `FocusTrap` with a single element works as expected * it should keep the focus in the Dialog Even if there is only 1 element. We were skipping the current active element so the container didn't have any elements anymore and just continued to the next focusable element in line. This will prevent that and ensure that we can only skip elements if there are multiple ones. * update changelog
1 parent 6f205f0 commit dbcfb23

File tree

10 files changed

+386
-12
lines changed

10 files changed

+386
-12
lines changed

‎packages/@headlessui-react/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1818
- Don’t overwrite classes during SSR when rendering fragments ([#2173](https://github.com/tailwindlabs/headlessui/pull/2173))
1919
- Improve `Combobox` accessibility ([#2153](https://github.com/tailwindlabs/headlessui/pull/2153))
2020
- Fix crash when reading `headlessuiFocusGuard` of `relatedTarget` in the `FocusTrap` component ([#2203](https://github.com/tailwindlabs/headlessui/pull/2203))
21+
- Fix `FocusTrap` in `Dialog` when there is only 1 focusable element ([#2172](https://github.com/tailwindlabs/headlessui/pull/2172))
2122

2223
## [1.7.7] - 2022-12-16
2324

‎packages/@headlessui-react/src/components/dialog/dialog.test.tsx

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
getDialogs,
2222
getDialogOverlays,
2323
} from '../../test-utils/accessibility-assertions'
24-
import { click, mouseDrag, press, Keys } from '../../test-utils/interactions'
24+
import { click, mouseDrag, press, Keys, shift } from '../../test-utils/interactions'
2525
import { PropsOf } from '../../types'
2626
import { Transition } from '../transitions/transition'
2727
import { createPortal } from 'react-dom'
@@ -797,6 +797,106 @@ describe('Keyboard interactions', () => {
797797
assertActiveElement(document.getElementById('a'))
798798
})
799799
)
800+
801+
it(
802+
'should not escape the FocusTrap when there is only 1 focusable element (going forwards)',
803+
suppressConsoleLogs(async () => {
804+
function Example() {
805+
let [isOpen, setIsOpen] = useState(false)
806+
return (
807+
<>
808+
<button id="trigger" onClick={() => setIsOpen((v) => !v)}>
809+
Trigger
810+
</button>
811+
<button>Before</button>
812+
<Dialog open={isOpen} onClose={setIsOpen}>
813+
<Dialog.Panel>
814+
<input type="text" id="a" />
815+
</Dialog.Panel>
816+
</Dialog>
817+
<button>After</button>
818+
</>
819+
)
820+
}
821+
render(<Example />)
822+
823+
assertDialog({ state: DialogState.InvisibleUnmounted })
824+
825+
// Open dialog
826+
await click(document.getElementById('trigger'))
827+
828+
// Verify it is open
829+
assertDialog({
830+
state: DialogState.Visible,
831+
attributes: { id: 'headlessui-dialog-1' },
832+
})
833+
834+
// Verify that the input field is focused
835+
assertActiveElement(document.getElementById('a'))
836+
837+
// Verify that we stay within the Dialog
838+
await press(Keys.Tab)
839+
assertActiveElement(document.getElementById('a'))
840+
841+
// Verify that we stay within the Dialog
842+
await press(Keys.Tab)
843+
assertActiveElement(document.getElementById('a'))
844+
845+
// Verify that we stay within the Dialog
846+
await press(Keys.Tab)
847+
assertActiveElement(document.getElementById('a'))
848+
})
849+
)
850+
851+
it(
852+
'should not escape the FocusTrap when there is only 1 focusable element (going backwards)',
853+
suppressConsoleLogs(async () => {
854+
function Example() {
855+
let [isOpen, setIsOpen] = useState(false)
856+
return (
857+
<>
858+
<button id="trigger" onClick={() => setIsOpen((v) => !v)}>
859+
Trigger
860+
</button>
861+
<button>Before</button>
862+
<Dialog open={isOpen} onClose={setIsOpen}>
863+
<Dialog.Panel>
864+
<input type="text" id="a" />
865+
</Dialog.Panel>
866+
</Dialog>
867+
<button>After</button>
868+
</>
869+
)
870+
}
871+
render(<Example />)
872+
873+
assertDialog({ state: DialogState.InvisibleUnmounted })
874+
875+
// Open dialog
876+
await click(document.getElementById('trigger'))
877+
878+
// Verify it is open
879+
assertDialog({
880+
state: DialogState.Visible,
881+
attributes: { id: 'headlessui-dialog-1' },
882+
})
883+
884+
// Verify that the input field is focused
885+
assertActiveElement(document.getElementById('a'))
886+
887+
// Verify that we stay within the Dialog
888+
await press(shift(Keys.Tab))
889+
assertActiveElement(document.getElementById('a'))
890+
891+
// Verify that we stay within the Dialog
892+
await press(shift(Keys.Tab))
893+
assertActiveElement(document.getElementById('a'))
894+
895+
// Verify that we stay within the Dialog
896+
await press(shift(Keys.Tab))
897+
assertActiveElement(document.getElementById('a'))
898+
})
899+
)
800900
})
801901
})
802902

‎packages/@headlessui-react/src/components/focus-trap/focus-trap.test.tsx

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,50 @@ it('should warn when there is no focusable element inside the FocusTrap', async
108108
spy.mockReset()
109109
})
110110

111+
it(
112+
'should not be possible to programmatically escape the focus trap (if there is only 1 focusable element)',
113+
suppressConsoleLogs(async () => {
114+
function Example() {
115+
return (
116+
<>
117+
<input id="a" autoFocus />
118+
119+
<FocusTrap>
120+
<input id="b" />
121+
</FocusTrap>
122+
</>
123+
)
124+
}
125+
126+
render(<Example />)
127+
128+
await nextFrame()
129+
130+
let [a, b] = Array.from(document.querySelectorAll('input'))
131+
132+
// Ensure that input-b is the active element
133+
assertActiveElement(b)
134+
135+
// Tab to the next item
136+
await press(Keys.Tab)
137+
138+
// Ensure that input-b is still the active element
139+
assertActiveElement(b)
140+
141+
// Try to move focus
142+
a?.focus()
143+
144+
// Ensure that input-b is still the active element
145+
assertActiveElement(b)
146+
147+
// Click on an element within the FocusTrap
148+
await click(b)
149+
150+
// Ensure that input-b is the active element
151+
assertActiveElement(b)
152+
})
153+
)
154+
111155
it(
112156
'should not be possible to programmatically escape the focus trap',
113157
suppressConsoleLogs(async () => {
@@ -214,6 +258,56 @@ it('should restore the previously focused element, before entering the FocusTrap
214258
assertActiveElement(document.getElementById('item-2'))
215259
})
216260

261+
it('should stay in the FocusTrap when using `tab`, if there is only 1 focusable element', async () => {
262+
render(
263+
<>
264+
<button>Before</button>
265+
<FocusTrap>
266+
<button id="item-a">Item A</button>
267+
</FocusTrap>
268+
<button>After</button>
269+
</>
270+
)
271+
272+
await nextFrame()
273+
274+
// Item A should be focused because the FocusTrap will focus the first item
275+
assertActiveElement(document.getElementById('item-a'))
276+
277+
// Next
278+
await press(Keys.Tab)
279+
assertActiveElement(document.getElementById('item-a'))
280+
281+
// Next
282+
await press(Keys.Tab)
283+
assertActiveElement(document.getElementById('item-a'))
284+
})
285+
286+
it('should stay in the FocusTrap when using `shift+tab`, if there is only 1 focusable element', async () => {
287+
render(
288+
<>
289+
<button>Before</button>
290+
<FocusTrap>
291+
<button id="item-a">Item A</button>
292+
</FocusTrap>
293+
<button>After</button>
294+
</>
295+
)
296+
297+
await nextFrame()
298+
299+
// Item A should be focused because the FocusTrap will focus the first item
300+
assertActiveElement(document.getElementById('item-a'))
301+
302+
// Previous (loop around!)
303+
await press(shift(Keys.Tab))
304+
assertActiveElement(document.getElementById('item-a'))
305+
306+
// Previous
307+
await press(shift(Keys.Tab))
308+
assertActiveElement(document.getElementById('item-a'))
309+
})
310+
217311
it('should be possible tab to the next focusable element within the focus trap', async () => {
218312
render(
219313
<>

‎packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,12 @@ export let FocusTrap = Object.assign(
8585
let wrapper = process.env.NODE_ENV === 'test' ? microTask : (cb: Function) => cb()
8686
wrapper(() => {
8787
match(direction.current, {
88-
[TabDirection.Forwards]: () =>
89-
focusIn(el, Focus.First, { skipElements: [e.relatedTarget as HTMLElement] }),
90-
[TabDirection.Backwards]: () =>
91-
focusIn(el, Focus.Last, { skipElements: [e.relatedTarget as HTMLElement] }),
88+
[TabDirection.Forwards]: () => {
89+
focusIn(el, Focus.First, { skipElements: [e.relatedTarget as HTMLElement] })
90+
},
91+
[TabDirection.Backwards]: () => {
92+
focusIn(el, Focus.Last, { skipElements: [e.relatedTarget as HTMLElement] })
93+
},
9294
})
9395
})
9496
})

‎packages/@headlessui-react/src/utils/focus-management.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ export function focusIn(
171171
: container
172172
: getFocusableElements(container)
173173

174-
if (skipElements.length > 0) {
174+
if (skipElements.length > 0 && elements.length > 1) {
175175
elements = elements.filter((x) => !skipElements.includes(x))
176176
}
177177

‎packages/@headlessui-vue/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1919
- Don’t overwrite classes during SSR when rendering fragments ([#2173](https://github.com/tailwindlabs/headlessui/pull/2173))
2020
- Improve `Combobox` accessibility ([#2153](https://github.com/tailwindlabs/headlessui/pull/2153))
2121
- Fix crash when reading `headlessuiFocusGuard` of `relatedTarget` in the `FocusTrap` component ([#2203](https://github.com/tailwindlabs/headlessui/pull/2203))
22+
- Fix `FocusTrap` in `Dialog` when there is only 1 focusable element ([#2172](https://github.com/tailwindlabs/headlessui/pull/2172))
2223

2324
## [1.7.7] - 2022-12-16
2425

0 commit comments

Comments
 (0)