Skip to content

Commit 8adaeed

Browse files
authored
Ensure moving focus within a Portal component, does not close the Popover component (#2492)
* abstract resolving root containers to hook This way we can reuse it in other components when needed. * allow registering a `Portal` component to a parent This allows us to find all the `Portal` components that are nested in a given component without manually adding refs to every `Portal` component itself. This will come in handy in the `Popover` component where we will allow focus in the child `Portal` components otherwise a focus outside of the `Popover` will close the it. In other components we often crawl the DOM directly using `[data-headlessui-portal]` data attributes, however this will fetch _all_ the `Portal` components, not the ones that started in the current component. * allow focus in portalled containers The `Popover` component will close by default if focus is moved outside of it. However, if you use a `Portal` comopnent inside the `Popover.Panel` then from a DOM perspective you are moving the focus outside of the `Popover.Panel`. This prevents the closing, and allows the focus into the `Portal`. It currently only allows for `Portal` components that originated from the `Popover` component. This means that if you open a `Dialog` component from within the `Popover` component, the `Dialog` already renders a `Portal` but since this is part of the `Dialog` and not the `Popover` it will close the `Popover` when focus is moved to the `Dialog` component. * ensure `useNestedPortals` register/unregister with the parent This ensures that if you have a structure like this: ```jsx <Dialog> {/* Renders a portal internally */} <Popover> <Portal> {/* First level */} <Popover.Panel> <Menu> <Portal> {/* Second level */} <Menu.Items> {/* ... */} </Menu.Items> </Portal> </Menu> </Popover.Panel> </Portal> </Popover> </Dialog> ``` That working with the `Menu` doesn't close the `Popover` or the `Dialog`. * cleanup `useRootContainers` hook This will allow you to pass in portal elements as well. + cleanup of the resolving of all DOM nodes. * handle nested portals in `Dialog` component * expose `contains` function from `useRootContainers` Shorthand to check if any of the root containers contains the given element. * add tests to verify that actions in `Portal` components won't close the `Popover` * update changelog * re-order use-outside-click logic To make it similar between React & Vue * inject the `PortalWrapper` context in the correct spot * ensure to forward the incoming `attrs`
1 parent 9dff545 commit 8adaeed

File tree

17 files changed

+517
-124
lines changed

17 files changed

+517
-124
lines changed

packages/@headlessui-react/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717
- Stop `<Transition appear>` from overwriting classes on re-render ([#2457](https://github.com/tailwindlabs/headlessui/pull/2457))
1818
- Improve control over `Menu` and `Listbox` options while searching ([#2471](https://github.com/tailwindlabs/headlessui/pull/2471))
1919
- Consider clicks inside iframes to be "outside" ([#2485](https://github.com/tailwindlabs/headlessui/pull/2485))
20+
- Ensure moving focus within a `Portal` component, does not close the `Popover` component ([#2492](https://github.com/tailwindlabs/headlessui/pull/2492))
2021

2122
### Changed
2223

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

Lines changed: 28 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import { Keys } from '../keyboard'
3333
import { isDisabledReactIssue7711 } from '../../utils/bugs'
3434
import { useId } from '../../hooks/use-id'
3535
import { FocusTrap } from '../../components/focus-trap/focus-trap'
36-
import { Portal } from '../../components/portal/portal'
36+
import { Portal, useNestedPortals } from '../../components/portal/portal'
3737
import { ForcePortalRoot } from '../../internal/portal-force-root'
3838
import { ComponentDescription, Description, useDescriptions } from '../description/description'
3939
import { useOpenClosed, State } from '../../internal/open-closed'
@@ -42,10 +42,10 @@ import { StackProvider, StackMessage } from '../../internal/stack-context'
4242
import { useOutsideClick } from '../../hooks/use-outside-click'
4343
import { useOwnerDocument } from '../../hooks/use-owner'
4444
import { useEventListener } from '../../hooks/use-event-listener'
45-
import { Hidden, Features as HiddenFeatures } from '../../internal/hidden'
4645
import { useEvent } from '../../hooks/use-event'
4746
import { useDocumentOverflowLockedEffect } from '../../hooks/document-overflow/use-document-overflow'
4847
import { useInert } from '../../hooks/use-inert'
48+
import { useRootContainers } from '../../hooks/use-root-containers'
4949

5050
enum DialogStates {
5151
Open,
@@ -158,9 +158,6 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
158158
let internalDialogRef = useRef<HTMLDivElement | null>(null)
159159
let dialogRef = useSyncRefs(internalDialogRef, ref)
160160

161-
// Reference to a node in the "main" tree, not in the portalled Dialog tree.
162-
let mainTreeNode = useRef<HTMLDivElement | null>(null)
163-
164161
let ownerDocument = useOwnerDocument(internalDialogRef)
165162

166163
// Validations
@@ -212,6 +209,15 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
212209
let enabled = ready ? (__demoMode ? false : dialogState === DialogStates.Open) : false
213210
let hasNestedDialogs = nestedDialogCount > 1 // 1 is the current dialog
214211
let hasParentDialog = useContext(DialogContext) !== null
212+
let [portals, PortalWrapper] = useNestedPortals()
213+
let {
214+
resolveContainers: resolveRootContainers,
215+
mainTreeNodeRef,
216+
MainTreeNode,
217+
} = useRootContainers({
218+
portals,
219+
defaultContainers: [state.panelRef.current ?? internalDialogRef.current],
220+
})
215221

216222
// If there are multiple dialogs, then you can be the root, the leaf or one
217223
// in between. We only care abou whether you are the top most one or not.
@@ -238,9 +244,9 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
238244
if (root.id === 'headlessui-portal-root') return false
239245

240246
// Find the root of the main tree node
241-
return root.contains(mainTreeNode.current) && root instanceof HTMLElement
247+
return root.contains(mainTreeNodeRef.current) && root instanceof HTMLElement
242248
}) ?? null) as HTMLElement | null
243-
}, [mainTreeNode])
249+
}, [mainTreeNodeRef])
244250
useInert(resolveRootOfMainTreeNode, inertOthersEnabled)
245251

246252
// This would mark the parent dialogs as inert
@@ -250,34 +256,18 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
250256
})()
251257
let resolveRootOfParentDialog = useCallback(() => {
252258
return (Array.from(ownerDocument?.querySelectorAll('[data-headlessui-portal]') ?? []).find(
253-
(root) => root.contains(mainTreeNode.current) && root instanceof HTMLElement
259+
(root) => root.contains(mainTreeNodeRef.current) && root instanceof HTMLElement
254260
) ?? null) as HTMLElement | null
255-
}, [mainTreeNode])
261+
}, [mainTreeNodeRef])
256262
useInert(resolveRootOfParentDialog, inertParentDialogs)
257263

258-
let resolveRootContainers = useEvent(() => {
259-
// Third party roots
260-
let rootContainers = Array.from(
261-
ownerDocument?.querySelectorAll('html > *, body > *, [data-headlessui-portal]') ?? []
262-
).filter((container) => {
263-
if (container === document.body) return false // Skip `<body>`
264-
if (container === document.head) return false // Skip `<head>`
265-
if (!(container instanceof HTMLElement)) return false // Skip non-HTMLElements
266-
if (container.contains(mainTreeNode.current)) return false // Skip if it is the main app
267-
if (state.panelRef.current && container.contains(state.panelRef.current)) return false
268-
return true // Keep
269-
})
270-
271-
return [...rootContainers, state.panelRef.current ?? internalDialogRef.current] as HTMLElement[]
272-
})
273-
274264
// Close Dialog on outside click
275265
let outsideClickEnabled = (() => {
276266
if (!enabled) return false
277267
if (hasNestedDialogs) return false
278268
return true
279269
})()
280-
useOutsideClick(() => resolveRootContainers(), close, outsideClickEnabled)
270+
useOutsideClick(resolveRootContainers, close, outsideClickEnabled)
281271

282272
// Handle `Escape` to close
283273
let escapeToCloseEnabled = (() => {
@@ -375,23 +365,25 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
375365
: FocusTrap.features.None
376366
}
377367
>
378-
{render({
379-
ourProps,
380-
theirProps,
381-
slot,
382-
defaultTag: DEFAULT_DIALOG_TAG,
383-
features: DialogRenderFeatures,
384-
visible: dialogState === DialogStates.Open,
385-
name: 'Dialog',
386-
})}
368+
<PortalWrapper>
369+
{render({
370+
ourProps,
371+
theirProps,
372+
slot,
373+
defaultTag: DEFAULT_DIALOG_TAG,
374+
features: DialogRenderFeatures,
375+
visible: dialogState === DialogStates.Open,
376+
name: 'Dialog',
377+
})}
378+
</PortalWrapper>
387379
</FocusTrap>
388380
</DescriptionProvider>
389381
</ForcePortalRoot>
390382
</Portal.Group>
391383
</DialogContext.Provider>
392384
</Portal>
393385
</ForcePortalRoot>
394-
<Hidden features={HiddenFeatures.Hidden} ref={mainTreeNode} />
386+
<MainTreeNode />
395387
</StackProvider>
396388
)
397389
}

packages/@headlessui-react/src/components/popover/popover.test.tsx

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2623,6 +2623,80 @@ describe('Mouse interactions', () => {
26232623
assertActiveElement(getPopoverButton())
26242624
})
26252625
)
2626+
2627+
it(
2628+
'should not close the Popover if the focus is moved outside of the Popover but still in the same React tree using Portals',
2629+
suppressConsoleLogs(async () => {
2630+
let clickFn = jest.fn()
2631+
render(
2632+
<Popover>
2633+
<Popover.Button>Toggle</Popover.Button>
2634+
<Popover.Panel>
2635+
<Portal>
2636+
<button onClick={clickFn}>foo</button>
2637+
</Portal>
2638+
</Popover.Panel>
2639+
</Popover>
2640+
)
2641+
2642+
// Open the popover
2643+
await click(getPopoverButton())
2644+
2645+
// Verify it is open
2646+
assertPopoverPanel({ state: PopoverState.Visible })
2647+
2648+
// Click the button outside the Popover (DOM) but inside (Portal / React tree)
2649+
await click(getByText('foo'))
2650+
2651+
// Verify it is still open
2652+
assertPopoverPanel({ state: PopoverState.Visible })
2653+
2654+
// Verify the button was clicked
2655+
expect(clickFn).toHaveBeenCalled()
2656+
})
2657+
)
2658+
2659+
it(
2660+
'should not close the Popover if the focus is moved outside of the Popover but still in the same React tree using nested Portals',
2661+
suppressConsoleLogs(async () => {
2662+
let clickFn = jest.fn()
2663+
render(
2664+
<Popover>
2665+
<Popover.Button>Toggle</Popover.Button>
2666+
<Popover.Panel>
2667+
Level 0
2668+
<Portal>
2669+
Level 1
2670+
<Portal>
2671+
Level 2
2672+
<Portal>
2673+
Level 3
2674+
<Portal>
2675+
Level 4<button onClick={clickFn}>foo</button>
2676+
</Portal>
2677+
</Portal>
2678+
</Portal>
2679+
</Portal>
2680+
</Popover.Panel>
2681+
</Popover>
2682+
)
2683+
2684+
// Open the popover
2685+
await click(getPopoverButton())
2686+
2687+
// Verify it is open
2688+
assertPopoverPanel({ state: PopoverState.Visible })
2689+
2690+
// Click the button outside the Popover (DOM) but inside (Portal / React tree)
2691+
await click(getByText('foo'))
2692+
2693+
// Verify it is still open
2694+
assertPopoverPanel({ state: PopoverState.Visible })
2695+
2696+
// Verify the button was clicked
2697+
expect(clickFn).toHaveBeenCalled()
2698+
})
2699+
)
26262700
})
26272701

26282702
describe('Nested popovers', () => {

packages/@headlessui-react/src/components/popover/popover.tsx

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ import { useTabDirection, Direction as TabDirection } from '../../hooks/use-tab-
5454
import { microTask } from '../../utils/micro-task'
5555
import { useLatestValue } from '../../hooks/use-latest-value'
5656
import { useIsoMorphicEffect } from '../../hooks/use-iso-morphic-effect'
57+
import { useRootContainers } from '../../hooks/use-root-containers'
58+
import { useNestedPortals } from '../../components/portal/portal'
5759

5860
type MouseEvent<T> = Parameters<MouseEventHandler<T>>[0]
5961

@@ -309,18 +311,26 @@ function PopoverFn<TTag extends ElementType = typeof DEFAULT_POPOVER_TAG>(
309311

310312
useEffect(() => registerPopover?.(registerBag), [registerPopover, registerBag])
311313

314+
let [portals, PortalWrapper] = useNestedPortals()
315+
let root = useRootContainers({
316+
portals,
317+
defaultContainers: [button, panel],
318+
})
319+
312320
// Handle focus out
313321
useEventListener(
314322
ownerDocument?.defaultView,
315323
'focus',
316324
(event) => {
325+
if (event.target === window) return
326+
if (!(event.target instanceof HTMLElement)) return
317327
if (popoverState !== PopoverStates.Open) return
318328
if (isFocusWithinPopoverGroup()) return
319329
if (!button) return
320330
if (!panel) return
321-
if (event.target === window) return
322-
if (beforePanelSentinel.current?.contains?.(event.target as HTMLElement)) return
323-
if (afterPanelSentinel.current?.contains?.(event.target as HTMLElement)) return
331+
if (root.contains(event.target)) return
332+
if (beforePanelSentinel.current?.contains?.(event.target)) return
333+
if (afterPanelSentinel.current?.contains?.(event.target)) return
324334

325335
dispatch({ type: ActionTypes.ClosePopover })
326336
},
@@ -329,7 +339,7 @@ function PopoverFn<TTag extends ElementType = typeof DEFAULT_POPOVER_TAG>(
329339

330340
// Handle outside click
331341
useOutsideClick(
332-
[button, panel],
342+
root.resolveContainers,
333343
(event, target) => {
334344
dispatch({ type: ActionTypes.ClosePopover })
335345

@@ -385,13 +395,16 @@ function PopoverFn<TTag extends ElementType = typeof DEFAULT_POPOVER_TAG>(
385395
[PopoverStates.Closed]: State.Closed,
386396
})}
387397
>
388-
{render({
389-
ourProps,
390-
theirProps,
391-
slot,
392-
defaultTag: DEFAULT_POPOVER_TAG,
393-
name: 'Popover',
394-
})}
398+
<PortalWrapper>
399+
{render({
400+
ourProps,
401+
theirProps,
402+
slot,
403+
defaultTag: DEFAULT_POPOVER_TAG,
404+
name: 'Popover',
405+
})}
406+
<root.MainTreeNode />
407+
</PortalWrapper>
395408
</OpenClosedProvider>
396409
</PopoverAPIContext.Provider>
397410
</PopoverContext.Provider>

packages/@headlessui-react/src/components/portal/portal.tsx

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import React, {
1010
ElementType,
1111
MutableRefObject,
1212
Ref,
13+
useMemo,
14+
ContextType,
1315
} from 'react'
1416
import { createPortal } from 'react-dom'
1517

@@ -22,6 +24,7 @@ import { optionalRef, useSyncRefs } from '../../hooks/use-sync-refs'
2224
import { useOnUnmount } from '../../hooks/use-on-unmount'
2325
import { useOwnerDocument } from '../../hooks/use-owner'
2426
import { env } from '../../utils/env'
27+
import { useEvent } from '../../hooks/use-event'
2528

2629
function usePortalTarget(ref: MutableRefObject<HTMLElement | null>): HTMLElement | null {
2730
let forceInRoot = usePortalRoot()
@@ -87,7 +90,7 @@ function PortalFn<TTag extends ElementType = typeof DEFAULT_PORTAL_TAG>(
8790
let [element] = useState<HTMLDivElement | null>(() =>
8891
env.isServer ? null : ownerDocument?.createElement('div') ?? null
8992
)
90-
93+
let parent = useContext(PortalParentContext)
9194
let ready = useServerHandoffComplete()
9295

9396
useIsoMorphicEffect(() => {
@@ -101,6 +104,13 @@ function PortalFn<TTag extends ElementType = typeof DEFAULT_PORTAL_TAG>(
101104
}
102105
}, [target, element])
103106

107+
useIsoMorphicEffect(() => {
108+
if (!element) return
109+
if (!parent) return
110+
111+
return parent.register(element)
112+
}, [parent, element])
113+
104114
useOnUnmount(() => {
105115
if (!target || !element) return
106116

@@ -164,6 +174,45 @@ function GroupFn<TTag extends ElementType = typeof DEFAULT_GROUP_TAG>(
164174

165175
// ---
166176

177+
let PortalParentContext = createContext<{
178+
register: (portal: HTMLElement) => () => void
179+
unregister: (portal: HTMLElement) => void
180+
portals: MutableRefObject<HTMLElement[]>
181+
} | null>(null)
182+
183+
export function useNestedPortals() {
184+
let parent = useContext(PortalParentContext)
185+
let portals = useRef<HTMLElement[]>([])
186+
187+
let register = useEvent((portal: HTMLElement) => {
188+
portals.current.push(portal)
189+
if (parent) parent.register(portal)
190+
return () => unregister(portal)
191+
})
192+
193+
let unregister = useEvent((portal: HTMLElement) => {
194+
let idx = portals.current.indexOf(portal)
195+
if (idx !== -1) portals.current.splice(idx, 1)
196+
if (parent) parent.unregister(portal)
197+
})
198+
199+
let api = useMemo<ContextType<typeof PortalParentContext>>(
200+
() => ({ register, unregister, portals }),
201+
[register, unregister, portals]
202+
)
203+
204+
return [
205+
portals,
206+
useMemo(() => {
207+
return function PortalWrapper({ children }: { children: React.ReactNode }) {
208+
return <PortalParentContext.Provider value={api}>{children}</PortalParentContext.Provider>
209+
}
210+
}, [api]),
211+
] as const
212+
}
213+
214+
// ---
215+
167216
interface ComponentPortal extends HasDisplayName {
168217
<TTag extends ElementType = typeof DEFAULT_PORTAL_TAG>(
169218
props: PortalProps<TTag> & RefProp<typeof PortalFn>

0 commit comments

Comments
 (0)