Skip to content

Commit 4f52d83

Browse files
RobinMalfaitRobinMalfait
authored
Fix Dialog cycling (#553)
* add tests to verify that tabbing around when using `initialFocus` works * add nesting example to `playground-vue` * fix nested dialog and initialFocus cycling * make React dialog consistent - Disable FocusLock on leaf Dialog's * update changelog
1 parent 27dece1 commit 4f52d83

File tree

11 files changed

+403
-174
lines changed

11 files changed

+403
-174
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3535
- Reset Combobox Input when the value gets reset ([#1181](https://github.com/tailwindlabs/headlessui/pull/1181))
3636
- Adjust active {item,option} index ([#1184](https://github.com/tailwindlabs/headlessui/pull/1184))
3737
- Fix re-focusing element after close ([#1186](https://github.com/tailwindlabs/headlessui/pull/1186))
38+
- Fix `Dialog` cycling ([#553](https://github.com/tailwindlabs/headlessui/pull/553))
3839

3940
## [@headlessui/react@v1.5.0] - 2022-02-17
4041

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

Lines changed: 63 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { createElement, useState } from 'react'
1+
import React, { createElement, useRef, useState } from 'react'
22
import { render } from '@testing-library/react'
33

44
import { Dialog } from './dialog'
@@ -515,6 +515,57 @@ describe('Keyboard interactions', () => {
515515
})
516516
)
517517
})
518+
519+
describe('`Tab` key', () => {
520+
it(
521+
'should be possible to tab around when using the initialFocus ref',
522+
suppressConsoleLogs(async () => {
523+
function Example() {
524+
let [isOpen, setIsOpen] = useState(false)
525+
let initialFocusRef = useRef(null)
526+
return (
527+
<>
528+
<button id="trigger" onClick={() => setIsOpen((v) => !v)}>
529+
Trigger
530+
</button>
531+
<Dialog open={isOpen} onClose={setIsOpen} initialFocus={initialFocusRef}>
532+
Contents
533+
<TabSentinel id="a" />
534+
<input type="text" id="b" ref={initialFocusRef} />
535+
</Dialog>
536+
</>
537+
)
538+
}
539+
render(<Example />)
540+
541+
assertDialog({ state: DialogState.InvisibleUnmounted })
542+
543+
// Open dialog
544+
await click(document.getElementById('trigger'))
545+
546+
// Verify it is open
547+
assertDialog({
548+
state: DialogState.Visible,
549+
attributes: { id: 'headlessui-dialog-1' },
550+
})
551+
552+
// Verify that the input field is focused
553+
assertActiveElement(document.getElementById('b'))
554+
555+
// Verify that we can tab around
556+
await press(Keys.Tab)
557+
assertActiveElement(document.getElementById('a'))
558+
559+
// Verify that we can tab around
560+
await press(Keys.Tab)
561+
assertActiveElement(document.getElementById('b'))
562+
563+
// Verify that we can tab around
564+
await press(Keys.Tab)
565+
assertActiveElement(document.getElementById('a'))
566+
})
567+
)
568+
})
518569
})
519570

520571
describe('Mouse interactions', () => {
@@ -762,19 +813,17 @@ describe('Nesting', () => {
762813
let [showChild, setShowChild] = useState(false)
763814

764815
return (
765-
<>
766-
<Dialog open={true} onClose={onClose}>
767-
<Dialog.Overlay />
768-
769-
<div>
770-
<p>Level: {level}</p>
771-
<button onClick={() => setShowChild(true)}>Open {level + 1} a</button>
772-
<button onClick={() => setShowChild(true)}>Open {level + 1} b</button>
773-
<button onClick={() => setShowChild(true)}>Open {level + 1} c</button>
774-
</div>
775-
{showChild && <Nested onClose={setShowChild} level={level + 1} />}
776-
</Dialog>
777-
</>
816+
<Dialog open={true} onClose={onClose}>
817+
<Dialog.Overlay />
818+
819+
<div>
820+
<p>Level: {level}</p>
821+
<button onClick={() => setShowChild(true)}>Open {level + 1} a</button>
822+
<button onClick={() => setShowChild(true)}>Open {level + 1} b</button>
823+
<button onClick={() => setShowChild(true)}>Open {level + 1} c</button>
824+
</div>
825+
{showChild && <Nested onClose={setShowChild} level={level + 1} />}
826+
</Dialog>
778827
)
779828
}
780829

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ let DialogRoot = forwardRefWithAs(function Dialog<
165165
`You provided an \`onClose\` prop to the \`Dialog\`, but the value is not a function. Received: ${onClose}`
166166
)
167167
}
168+
168169
let dialogState = open ? DialogStates.Open : DialogStates.Closed
169170
let visible = (() => {
170171
if (usesOpenClosedState !== null) {
@@ -200,7 +201,7 @@ let DialogRoot = forwardRefWithAs(function Dialog<
200201
enabled
201202
? match(position, {
202203
parent: FocusTrapFeatures.RestoreFocus,
203-
leaf: FocusTrapFeatures.All,
204+
leaf: FocusTrapFeatures.All & ~FocusTrapFeatures.FocusLock,
204205
})
205206
: FocusTrapFeatures.None,
206207
{ initialFocus, containers }

‎packages/@headlessui-react/src/hooks/use-focus-trap.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,7 @@ export function useFocusTrap(
4141
containers?: MutableRefObject<Set<MutableRefObject<HTMLElement | null>>>
4242
} = {}
4343
) {
44-
let restoreElement = useRef<HTMLElement | null>(
45-
typeof window !== 'undefined' ? (document.activeElement as HTMLElement) : null
46-
)
44+
let restoreElement = useRef<HTMLElement | null>(null)
4745
let previousActiveElement = useRef<HTMLElement | null>(null)
4846
let mounted = useIsMounted()
4947

@@ -54,7 +52,9 @@ export function useFocusTrap(
5452
useEffect(() => {
5553
if (!featuresRestoreFocus) return
5654

57-
restoreElement.current = document.activeElement as HTMLElement
55+
if (!restoreElement.current) {
56+
restoreElement.current = document.activeElement as HTMLElement
57+
}
5858
}, [featuresRestoreFocus])
5959

6060
// Restore the focus when we unmount the component.
@@ -70,7 +70,8 @@ export function useFocusTrap(
7070
// Handle initial focus
7171
useEffect(() => {
7272
if (!featuresInitialFocus) return
73-
if (!container.current) return
73+
let containerElement = container.current
74+
if (!containerElement) return
7475

7576
let activeElement = document.activeElement as HTMLElement
7677

@@ -79,7 +80,7 @@ export function useFocusTrap(
7980
previousActiveElement.current = activeElement
8081
return // Initial focus ref is already the active element
8182
}
82-
} else if (container.current.contains(activeElement)) {
83+
} else if (containerElement.contains(activeElement)) {
8384
previousActiveElement.current = activeElement
8485
return // Already focused within Dialog
8586
}
@@ -88,7 +89,7 @@ export function useFocusTrap(
8889
if (initialFocus?.current) {
8990
focusElement(initialFocus.current)
9091
} else {
91-
if (focusIn(container.current, Focus.First) === FocusResult.Error) {
92+
if (focusIn(containerElement, Focus.First) === FocusResult.Error) {
9293
console.warn('There are no focusable elements inside the <FocusTrap />')
9394
}
9495
}

‎packages/@headlessui-vue/src/components/dialog/dialog.test.ts‎

Lines changed: 66 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,68 @@ describe('Keyboard interactions', () => {
637637
})
638638
)
639639
})
640+
641+
describe('`Tab` key', () => {
642+
it(
643+
'should be possible to tab around when using the initialFocus ref',
644+
suppressConsoleLogs(async () => {
645+
renderTemplate({
646+
template: `
647+
<div>
648+
<button id="trigger" @click="toggleOpen">
649+
Trigger
650+
</button>
651+
<Dialog :open="isOpen" @close="setIsOpen" :initialFocus="initialFocusRef">
652+
Contents
653+
<TabSentinel id="a" />
654+
<input type="text" id="b" ref="initialFocusRef" />
655+
</Dialog>
656+
</div>
657+
`,
658+
setup() {
659+
let isOpen = ref(false)
660+
let initialFocusRef = ref(null)
661+
return {
662+
isOpen,
663+
initialFocusRef,
664+
setIsOpen(value: boolean) {
665+
isOpen.value = value
666+
},
667+
toggleOpen() {
668+
isOpen.value = !isOpen.value
669+
},
670+
}
671+
},
672+
})
673+
674+
assertDialog({ state: DialogState.InvisibleUnmounted })
675+
676+
// Open dialog
677+
await click(document.getElementById('trigger'))
678+
679+
// Verify it is open
680+
assertDialog({
681+
state: DialogState.Visible,
682+
attributes: { id: 'headlessui-dialog-1' },
683+
})
684+
685+
// Verify that the input field is focused
686+
assertActiveElement(document.getElementById('b'))
687+
688+
// Verify that we can tab around
689+
await press(Keys.Tab)
690+
assertActiveElement(document.getElementById('a'))
691+
692+
// Verify that we can tab around
693+
await press(Keys.Tab)
694+
assertActiveElement(document.getElementById('b'))
695+
696+
// Verify that we can tab around
697+
await press(Keys.Tab)
698+
assertActiveElement(document.getElementById('a'))
699+
})
700+
)
701+
})
640702
})
641703

642704
describe('Mouse interactions', () => {
@@ -950,31 +1012,13 @@ describe('Nesting', () => {
9501012

9511013
return () => {
9521014
let level = props.level ?? 1
953-
return h(Dialog, { open: true, onClose: onClose }, () => [
1015+
return h(Dialog, { open: true, onClose }, () => [
9541016
h(DialogOverlay),
9551017
h('div', [
9561018
h('p', `Level: ${level}`),
957-
h(
958-
'button',
959-
{
960-
onClick: () => (showChild.value = true),
961-
},
962-
`Open ${level + 1} a`
963-
),
964-
h(
965-
'button',
966-
{
967-
onClick: () => (showChild.value = true),
968-
},
969-
`Open ${level + 1} b`
970-
),
971-
h(
972-
'button',
973-
{
974-
onClick: () => (showChild.value = true),
975-
},
976-
`Open ${level + 1} c`
977-
),
1019+
h('button', { onClick: () => (showChild.value = true) }, `Open ${level + 1} a`),
1020+
h('button', { onClick: () => (showChild.value = true) }, `Open ${level + 1} b`),
1021+
h('button', { onClick: () => (showChild.value = true) }, `Open ${level + 1} c`),
9781022
]),
9791023
showChild.value &&
9801024
h(Nested, {

0 commit comments

Comments
 (0)