Skip to content

Commit 948ae73

Browse files
RobinMalfaitRobinMalfait
authored
Allow root containers from the Dialog component in the FocusTrap component (#2322)
* add (failing) test to verify moving focus to 3rd party containers work * pass `resolveRootContainers` to `FocusTrap` * handle lazy containers in `FocusTrap` * update changelog
1 parent de7ddf9 commit 948ae73

File tree

9 files changed

+201
-51
lines changed

9 files changed

+201
-51
lines changed

‎packages/@headlessui-react/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- Ensure `Transition` component completes if nothing is transitioning ([#2318](https://github.com/tailwindlabs/headlessui/pull/2318))
1313
- Enable native label behavior for `<Switch>` where possible ([#2265](https://github.com/tailwindlabs/headlessui/pull/2265))
14+
- Allow root containers from the `Dialog` component in the `FocusTrap` component ([#2322](https://github.com/tailwindlabs/headlessui/pull/2322))
1415

1516
## [1.7.12] - 2023-02-24
1617

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

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
getDialogs,
2323
getDialogOverlays,
2424
} from '../../test-utils/accessibility-assertions'
25-
import { click, mouseDrag, press, Keys, shift } from '../../test-utils/interactions'
25+
import { click, mouseDrag, press, Keys, shift, focus } from '../../test-utils/interactions'
2626
import { PropsOf } from '../../types'
2727
import { Transition } from '../transitions/transition'
2828
import { OpenClosedProvider, State } from '../../internal/open-closed'
@@ -1357,6 +1357,62 @@ describe('Mouse interactions', () => {
13571357
})
13581358
)
13591359

1360+
it(
1361+
'should be possible to focus elements created by third party libraries',
1362+
suppressConsoleLogs(async () => {
1363+
let fn = jest.fn()
1364+
let handleFocus = jest.fn()
1365+
1366+
function ThirdPartyLibrary() {
1367+
return createPortal(
1368+
<>
1369+
<button data-lib onClick={fn}>
1370+
3rd party button
1371+
</button>
1372+
</>,
1373+
document.body
1374+
)
1375+
}
1376+
1377+
function Example() {
1378+
let [isOpen, setIsOpen] = useState(true)
1379+
1380+
return (
1381+
<div>
1382+
<span>Main app</span>
1383+
<Dialog open={isOpen} onClose={setIsOpen}>
1384+
<div>
1385+
Contents
1386+
<TabSentinel onFocus={handleFocus} />
1387+
</div>
1388+
</Dialog>
1389+
<ThirdPartyLibrary />
1390+
</div>
1391+
)
1392+
}
1393+
1394+
render(<Example />)
1395+
1396+
await nextFrame()
1397+
1398+
// Verify it is open
1399+
assertDialog({ state: DialogState.Visible })
1400+
1401+
// Click the button inside the 3rd party library
1402+
await focus(document.querySelector('[data-lib]'))
1403+
1404+
// Verify that the focus is on the 3rd party button, and that we are not redirecting focus to
1405+
// the dialog again.
1406+
assertActiveElement(document.querySelector('[data-lib]'))
1407+
1408+
// This should only have been called once (when opening the Dialog)
1409+
expect(handleFocus).toHaveBeenCalledTimes(1)
1410+
1411+
// Verify the dialog is still open
1412+
assertDialog({ state: DialogState.Visible })
1413+
})
1414+
)
1415+
13601416
it(
13611417
'should be possible to click elements inside the dialog when they reside inside a shadow boundary',
13621418
suppressConsoleLogs(async () => {

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

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,6 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
155155
open = (usesOpenClosedState & State.Open) === State.Open
156156
}
157157

158-
let containers = useRef<Set<MutableRefObject<HTMLElement | null>>>(new Set())
159158
let internalDialogRef = useRef<HTMLDivElement | null>(null)
160159
let dialogRef = useSyncRefs(internalDialogRef, ref)
161160

@@ -256,7 +255,7 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
256255
}, [mainTreeNode])
257256
useInert(resolveRootOfParentDialog, inertParentDialogs)
258257

259-
let resolveContainers = useEvent(() => {
258+
let resolveRootContainers = useEvent(() => {
260259
// Third party roots
261260
let rootContainers = Array.from(
262261
ownerDocument?.querySelectorAll('html > *, body > *, [data-headlessui-portal]') ?? []
@@ -278,7 +277,7 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
278277
if (hasNestedDialogs) return false
279278
return true
280279
})()
281-
useOutsideClick(() => resolveContainers(), close, outsideClickEnabled)
280+
useOutsideClick(() => resolveRootContainers(), close, outsideClickEnabled)
282281

283282
// Handle `Escape` to close
284283
let escapeToCloseEnabled = (() => {
@@ -302,7 +301,7 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
302301
if (hasParentDialog) return false
303302
return true
304303
})()
305-
useScrollLock(ownerDocument, scrollLockEnabled, resolveContainers)
304+
useScrollLock(ownerDocument, scrollLockEnabled, resolveRootContainers)
306305

307306
// Trigger close when the FocusTrap gets hidden
308307
useEffect(() => {
@@ -349,18 +348,12 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
349348
type="Dialog"
350349
enabled={dialogState === DialogStates.Open}
351350
element={internalDialogRef}
352-
onUpdate={useEvent((message, type, element) => {
351+
onUpdate={useEvent((message, type) => {
353352
if (type !== 'Dialog') return
354353

355354
match(message, {
356-
[StackMessage.Add]() {
357-
containers.current.add(element)
358-
setNestedDialogCount((count) => count + 1)
359-
},
360-
[StackMessage.Remove]() {
361-
containers.current.add(element)
362-
setNestedDialogCount((count) => count - 1)
363-
},
355+
[StackMessage.Add]: () => setNestedDialogCount((count) => count + 1),
356+
[StackMessage.Remove]: () => setNestedDialogCount((count) => count - 1),
364357
})
365358
})}
366359
>
@@ -372,7 +365,7 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
372365
<DescriptionProvider slot={slot} name="Dialog.Description">
373366
<FocusTrap
374367
initialFocus={initialFocus}
375-
containers={containers}
368+
containers={resolveRootContainers}
376369
features={
377370
enabled
378371
? match(position, {

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

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,26 @@ import { microTask } from '../../utils/micro-task'
2525
import { useWatch } from '../../hooks/use-watch'
2626
import { useDisposables } from '../../hooks/use-disposables'
2727

28+
type Containers =
29+
// Lazy resolved containers
30+
| (() => Iterable<HTMLElement>)
31+
32+
// List of containers
33+
| MutableRefObject<Set<MutableRefObject<HTMLElement | null>>>
34+
35+
function resolveContainers(containers?: Containers): Set<HTMLElement> {
36+
if (!containers) return new Set<HTMLElement>()
37+
if (typeof containers === 'function') return new Set(containers())
38+
39+
let all = new Set<HTMLElement>()
40+
for (let container of containers.current) {
41+
if (container.current instanceof HTMLElement) {
42+
all.add(container.current)
43+
}
44+
}
45+
return all
46+
}
47+
2848
let DEFAULT_FOCUS_TRAP_TAG = 'div' as const
2949

3050
enum Features {
@@ -50,7 +70,7 @@ enum Features {
5070
export type FocusTrapProps<TTag extends ElementType> = Props<TTag> & {
5171
initialFocus?: MutableRefObject<HTMLElement | null>
5272
features?: Features
53-
containers?: MutableRefObject<Set<MutableRefObject<HTMLElement | null>>>
73+
containers?: Containers
5474
}
5575

5676
function FocusTrapFn<TTag extends ElementType = typeof DEFAULT_FOCUS_TRAP_TAG>(
@@ -109,8 +129,8 @@ function FocusTrapFn<TTag extends ElementType = typeof DEFAULT_FOCUS_TRAP_TAG>(
109129
}
110130
},
111131
onBlur(e: ReactFocusEvent) {
112-
let allContainers = new Set(containers?.current)
113-
allContainers.add(container)
132+
let allContainers = resolveContainers(containers)
133+
if (container.current instanceof HTMLElement) allContainers.add(container.current)
114134

115135
let relatedTarget = e.relatedTarget
116136
if (!(relatedTarget instanceof HTMLElement)) return
@@ -123,7 +143,7 @@ function FocusTrapFn<TTag extends ElementType = typeof DEFAULT_FOCUS_TRAP_TAG>(
123143
// Blur is triggered due to focus on relatedTarget, and the relatedTarget is not inside any
124144
// of the dialog containers. In other words, let's move focus back in!
125145
if (!contains(allContainers, relatedTarget)) {
126-
// Was the blur invoke via the keyboard? Redirect to the next in line.
146+
// Was the blur invoked via the keyboard? Redirect to the next in line.
127147
if (recentlyUsedTabKey.current) {
128148
focusIn(
129149
container.current as HTMLElement,
@@ -135,7 +155,7 @@ function FocusTrapFn<TTag extends ElementType = typeof DEFAULT_FOCUS_TRAP_TAG>(
135155
)
136156
}
137157

138-
// It was invoke via something else (e.g.: click, programmatically, ...). Redirect to the
158+
// It was invoked via something else (e.g.: click, programmatically, ...). Redirect to the
139159
// previous active item in the FocusTrap
140160
else if (e.target instanceof HTMLElement) {
141161
focusElement(e.target)
@@ -308,7 +328,7 @@ function useFocusLock(
308328
}: {
309329
ownerDocument: Document | null
310330
container: MutableRefObject<HTMLElement | null>
311-
containers?: MutableRefObject<Set<MutableRefObject<HTMLElement | null>>>
331+
containers?: Containers
312332
previousActiveElement: MutableRefObject<HTMLElement | null>
313333
},
314334
enabled: boolean
@@ -323,8 +343,8 @@ function useFocusLock(
323343
if (!enabled) return
324344
if (!mounted.current) return
325345

326-
let allContainers = new Set(containers?.current)
327-
allContainers.add(container)
346+
let allContainers = resolveContainers(containers)
347+
if (container.current instanceof HTMLElement) allContainers.add(container.current)
328348

329349
let previous = previousActiveElement.current
330350
if (!previous) return
@@ -348,9 +368,9 @@ function useFocusLock(
348368
)
349369
}
350370

351-
function contains(containers: Set<MutableRefObject<HTMLElement | null>>, element: HTMLElement) {
371+
function contains(containers: Set<HTMLElement>, element: HTMLElement) {
352372
for (let container of containers) {
353-
if (container.current?.contains(element)) return true
373+
if (container.contains(element)) return true
354374
}
355375

356376
return false

‎packages/@headlessui-vue/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Fixed
1111

1212
- Enable native label behavior for `<Switch>` where possible ([#2265](https://github.com/tailwindlabs/headlessui/pull/2265))
13+
- Allow root containers from the `Dialog` component in the `FocusTrap` component ([#2322](https://github.com/tailwindlabs/headlessui/pull/2322))
1314

1415
## [1.7.11] - 2023-02-24
1516

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

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import {
3939
getDialogs,
4040
getDialogOverlays,
4141
} from '../../test-utils/accessibility-assertions'
42-
import { click, mouseDrag, press, Keys, shift } from '../../test-utils/interactions'
42+
import { click, mouseDrag, press, Keys, shift, focus } from '../../test-utils/interactions'
4343
import { html } from '../../test-utils/html'
4444
import { useOpenClosedProvider, State } from '../../internal/open-closed'
4545

@@ -1769,6 +1769,67 @@ describe('Mouse interactions', () => {
17691769
})
17701770
)
17711771

1772+
it(
1773+
'should be possible to focus elements created by third party libraries',
1774+
suppressConsoleLogs(async () => {
1775+
let fn = jest.fn()
1776+
let handleFocus = jest.fn()
1777+
1778+
let ThirdPartyLibrary = defineComponent({
1779+
template: html`
1780+
<teleport to="body">
1781+
<button data-lib @click="fn">3rd party button</button>
1782+
</teleport>
1783+
`,
1784+
setup: () => ({ fn }),
1785+
})
1786+
1787+
renderTemplate({
1788+
components: { ThirdPartyLibrary },
1789+
template: `
1790+
<div>
1791+
<span>Main app</span>
1792+
<Dialog :open="isOpen" @close="setIsOpen">
1793+
<div>
1794+
Contents
1795+
<TabSentinel @focus="handleFocus" />
1796+
</div>
1797+
</Dialog>
1798+
<ThirdPartyLibrary />
1799+
</div>
1800+
`,
1801+
setup() {
1802+
let isOpen = ref(true)
1803+
return {
1804+
handleFocus,
1805+
isOpen,
1806+
setIsOpen(value: boolean) {
1807+
isOpen.value = value
1808+
},
1809+
}
1810+
},
1811+
})
1812+
1813+
await nextFrame()
1814+
1815+
// Verify it is open
1816+
assertDialog({ state: DialogState.Visible })
1817+
1818+
// Click the button inside the 3rd party library
1819+
await focus(document.querySelector('[data-lib]'))
1820+
1821+
// Verify that the focus is on the 3rd party button, and that we are not redirecting focus to
1822+
// the dialog again.
1823+
assertActiveElement(document.querySelector('[data-lib]'))
1824+
1825+
// This should only have been called once (when opening the Dialog)
1826+
expect(handleFocus).toHaveBeenCalledTimes(1)
1827+
1828+
// Verify the dialog is still open
1829+
assertDialog({ state: DialogState.Visible })
1830+
})
1831+
)
1832+
17721833
it(
17731834
'should be possible to click elements inside the dialog when they reside inside a shadow boundary',
17741835
suppressConsoleLogs(async () => {

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

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ export let Dialog = defineComponent({
9595
return props.open
9696
})
9797

98-
let containers = ref<Set<Ref<HTMLElement | null>>>(new Set())
9998
let internalDialogRef = ref<HTMLDivElement | null>(null)
10099

101100
// Reference to a node in the "main" tree, not in the portalled Dialog tree.
@@ -178,18 +177,12 @@ export let Dialog = defineComponent({
178177
type: 'Dialog',
179178
enabled: computed(() => dialogState.value === DialogStates.Open),
180179
element: internalDialogRef,
181-
onUpdate: (message, type, element) => {
180+
onUpdate: (message, type) => {
182181
if (type !== 'Dialog') return
183182

184183
return match(message, {
185-
[StackMessage.Add]() {
186-
containers.value.add(element)
187-
nestedDialogCount.value += 1
188-
},
189-
[StackMessage.Remove]() {
190-
containers.value.delete(element)
191-
nestedDialogCount.value -= 1
192-
},
184+
[StackMessage.Add]: () => (nestedDialogCount.value += 1),
185+
[StackMessage.Remove]: () => (nestedDialogCount.value -= 1),
193186
})
194187
},
195188
})
@@ -216,7 +209,7 @@ export let Dialog = defineComponent({
216209

217210
provide(DialogContext, api)
218211

219-
function resolveAllowedContainers() {
212+
function resolveRootContainers() {
220213
// Third party roots
221214
let rootContainers = Array.from(
222215
ownerDocument.value?.querySelectorAll('html > *, body > *, [data-headlessui-portal]') ?? []
@@ -239,7 +232,7 @@ export let Dialog = defineComponent({
239232
return true
240233
})
241234
useOutsideClick(
242-
() => resolveAllowedContainers(),
235+
() => resolveRootContainers(),
243236
(_event, target) => {
244237
api.close()
245238
nextTick(() => target?.focus())
@@ -271,7 +264,7 @@ export let Dialog = defineComponent({
271264
return true
272265
})
273266
useDocumentOverflowLockedEffect(ownerDocument, scrollLockEnabled, (meta) => ({
274-
containers: [...(meta.containers ?? []), resolveAllowedContainers],
267+
containers: [...(meta.containers ?? []), resolveRootContainers],
275268
}))
276269

277270
// Trigger close when the FocusTrap gets hidden
@@ -318,7 +311,7 @@ export let Dialog = defineComponent({
318311
FocusTrap,
319312
{
320313
initialFocus,
321-
containers,
314+
containers: resolveRootContainers,
322315
features: enabled.value
323316
? match(position.value, {
324317
parent: FocusTrap.features.RestoreFocus,

0 commit comments

Comments
 (0)