From 8adaeeda4537ca35f824e432be37e4413edab541 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 19 May 2023 15:37:01 +0200 Subject: [PATCH] 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 {/* Renders a portal internally */} {/* First level */} {/* Second level */} {/* ... */} ``` 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` --- packages/@headlessui-react/CHANGELOG.md | 1 + .../src/components/dialog/dialog.tsx | 64 +++++------ .../src/components/popover/popover.test.tsx | 74 +++++++++++++ .../src/components/popover/popover.tsx | 35 ++++-- .../src/components/portal/portal.tsx | 51 ++++++++- .../src/hooks/use-outside-click.ts | 18 ++-- .../src/hooks/use-root-containers.tsx | 64 +++++++++++ packages/@headlessui-react/src/index.test.ts | 2 +- packages/@headlessui-react/src/index.ts | 2 +- packages/@headlessui-vue/CHANGELOG.md | 1 + .../src/components/dialog/dialog.ts | 62 +++++------ .../src/components/popover/popover.test.ts | 101 +++++++++++++++--- .../src/components/popover/popover.ts | 36 ++++--- .../src/components/portal/portal.ts | 58 +++++++++- .../src/hooks/use-root-containers.ts | 62 +++++++++++ packages/@headlessui-vue/src/index.test.ts | 8 +- packages/@headlessui-vue/src/index.ts | 2 +- 17 files changed, 517 insertions(+), 124 deletions(-) create mode 100644 packages/@headlessui-react/src/hooks/use-root-containers.tsx create mode 100644 packages/@headlessui-vue/src/hooks/use-root-containers.ts diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index db9b4eb8e4..55c84c231c 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Stop `` from overwriting classes on re-render ([#2457](https://github.com/tailwindlabs/headlessui/pull/2457)) - Improve control over `Menu` and `Listbox` options while searching ([#2471](https://github.com/tailwindlabs/headlessui/pull/2471)) - Consider clicks inside iframes to be "outside" ([#2485](https://github.com/tailwindlabs/headlessui/pull/2485)) +- Ensure moving focus within a `Portal` component, does not close the `Popover` component ([#2492](https://github.com/tailwindlabs/headlessui/pull/2492)) ### Changed diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx index 43a10da8df..d1909e0deb 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.tsx @@ -33,7 +33,7 @@ import { Keys } from '../keyboard' import { isDisabledReactIssue7711 } from '../../utils/bugs' import { useId } from '../../hooks/use-id' import { FocusTrap } from '../../components/focus-trap/focus-trap' -import { Portal } from '../../components/portal/portal' +import { Portal, useNestedPortals } from '../../components/portal/portal' import { ForcePortalRoot } from '../../internal/portal-force-root' import { ComponentDescription, Description, useDescriptions } from '../description/description' import { useOpenClosed, State } from '../../internal/open-closed' @@ -42,10 +42,10 @@ import { StackProvider, StackMessage } from '../../internal/stack-context' import { useOutsideClick } from '../../hooks/use-outside-click' import { useOwnerDocument } from '../../hooks/use-owner' import { useEventListener } from '../../hooks/use-event-listener' -import { Hidden, Features as HiddenFeatures } from '../../internal/hidden' import { useEvent } from '../../hooks/use-event' import { useDocumentOverflowLockedEffect } from '../../hooks/document-overflow/use-document-overflow' import { useInert } from '../../hooks/use-inert' +import { useRootContainers } from '../../hooks/use-root-containers' enum DialogStates { Open, @@ -158,9 +158,6 @@ function DialogFn( let internalDialogRef = useRef(null) let dialogRef = useSyncRefs(internalDialogRef, ref) - // Reference to a node in the "main" tree, not in the portalled Dialog tree. - let mainTreeNode = useRef(null) - let ownerDocument = useOwnerDocument(internalDialogRef) // Validations @@ -212,6 +209,15 @@ function DialogFn( let enabled = ready ? (__demoMode ? false : dialogState === DialogStates.Open) : false let hasNestedDialogs = nestedDialogCount > 1 // 1 is the current dialog let hasParentDialog = useContext(DialogContext) !== null + let [portals, PortalWrapper] = useNestedPortals() + let { + resolveContainers: resolveRootContainers, + mainTreeNodeRef, + MainTreeNode, + } = useRootContainers({ + portals, + defaultContainers: [state.panelRef.current ?? internalDialogRef.current], + }) // If there are multiple dialogs, then you can be the root, the leaf or one // in between. We only care abou whether you are the top most one or not. @@ -238,9 +244,9 @@ function DialogFn( if (root.id === 'headlessui-portal-root') return false // Find the root of the main tree node - return root.contains(mainTreeNode.current) && root instanceof HTMLElement + return root.contains(mainTreeNodeRef.current) && root instanceof HTMLElement }) ?? null) as HTMLElement | null - }, [mainTreeNode]) + }, [mainTreeNodeRef]) useInert(resolveRootOfMainTreeNode, inertOthersEnabled) // This would mark the parent dialogs as inert @@ -250,34 +256,18 @@ function DialogFn( })() let resolveRootOfParentDialog = useCallback(() => { return (Array.from(ownerDocument?.querySelectorAll('[data-headlessui-portal]') ?? []).find( - (root) => root.contains(mainTreeNode.current) && root instanceof HTMLElement + (root) => root.contains(mainTreeNodeRef.current) && root instanceof HTMLElement ) ?? null) as HTMLElement | null - }, [mainTreeNode]) + }, [mainTreeNodeRef]) useInert(resolveRootOfParentDialog, inertParentDialogs) - let resolveRootContainers = useEvent(() => { - // Third party roots - let rootContainers = Array.from( - ownerDocument?.querySelectorAll('html > *, body > *, [data-headlessui-portal]') ?? [] - ).filter((container) => { - if (container === document.body) return false // Skip `` - if (container === document.head) return false // Skip `` - if (!(container instanceof HTMLElement)) return false // Skip non-HTMLElements - if (container.contains(mainTreeNode.current)) return false // Skip if it is the main app - if (state.panelRef.current && container.contains(state.panelRef.current)) return false - return true // Keep - }) - - return [...rootContainers, state.panelRef.current ?? internalDialogRef.current] as HTMLElement[] - }) - // Close Dialog on outside click let outsideClickEnabled = (() => { if (!enabled) return false if (hasNestedDialogs) return false return true })() - useOutsideClick(() => resolveRootContainers(), close, outsideClickEnabled) + useOutsideClick(resolveRootContainers, close, outsideClickEnabled) // Handle `Escape` to close let escapeToCloseEnabled = (() => { @@ -375,15 +365,17 @@ function DialogFn( : FocusTrap.features.None } > - {render({ - ourProps, - theirProps, - slot, - defaultTag: DEFAULT_DIALOG_TAG, - features: DialogRenderFeatures, - visible: dialogState === DialogStates.Open, - name: 'Dialog', - })} + + {render({ + ourProps, + theirProps, + slot, + defaultTag: DEFAULT_DIALOG_TAG, + features: DialogRenderFeatures, + visible: dialogState === DialogStates.Open, + name: 'Dialog', + })} + @@ -391,7 +383,7 @@ function DialogFn( - + ) } diff --git a/packages/@headlessui-react/src/components/popover/popover.test.tsx b/packages/@headlessui-react/src/components/popover/popover.test.tsx index c43414993a..af1f0f6b75 100644 --- a/packages/@headlessui-react/src/components/popover/popover.test.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.test.tsx @@ -2623,6 +2623,80 @@ describe('Mouse interactions', () => { assertActiveElement(getPopoverButton()) }) ) + + it( + 'should not close the Popover if the focus is moved outside of the Popover but still in the same React tree using Portals', + suppressConsoleLogs(async () => { + let clickFn = jest.fn() + render( + + Toggle + + + + + + + ) + + // Open the popover + await click(getPopoverButton()) + + // Verify it is open + assertPopoverPanel({ state: PopoverState.Visible }) + + // Click the button outside the Popover (DOM) but inside (Portal / React tree) + await click(getByText('foo')) + + // Verify it is still open + assertPopoverPanel({ state: PopoverState.Visible }) + + // Verify the button was clicked + expect(clickFn).toHaveBeenCalled() + }) + ) + + it( + 'should not close the Popover if the focus is moved outside of the Popover but still in the same React tree using nested Portals', + suppressConsoleLogs(async () => { + let clickFn = jest.fn() + render( + + Toggle + + Level 0 + + Level 1 + + Level 2 + + Level 3 + + Level 4 + + + + + + + ) + + // Open the popover + await click(getPopoverButton()) + + // Verify it is open + assertPopoverPanel({ state: PopoverState.Visible }) + + // Click the button outside the Popover (DOM) but inside (Portal / React tree) + await click(getByText('foo')) + + // Verify it is still open + assertPopoverPanel({ state: PopoverState.Visible }) + + // Verify the button was clicked + expect(clickFn).toHaveBeenCalled() + }) + ) }) describe('Nested popovers', () => { diff --git a/packages/@headlessui-react/src/components/popover/popover.tsx b/packages/@headlessui-react/src/components/popover/popover.tsx index b9b3e589fb..a094d5f971 100644 --- a/packages/@headlessui-react/src/components/popover/popover.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.tsx @@ -54,6 +54,8 @@ import { useTabDirection, Direction as TabDirection } from '../../hooks/use-tab- import { microTask } from '../../utils/micro-task' import { useLatestValue } from '../../hooks/use-latest-value' import { useIsoMorphicEffect } from '../../hooks/use-iso-morphic-effect' +import { useRootContainers } from '../../hooks/use-root-containers' +import { useNestedPortals } from '../../components/portal/portal' type MouseEvent = Parameters>[0] @@ -309,18 +311,26 @@ function PopoverFn( useEffect(() => registerPopover?.(registerBag), [registerPopover, registerBag]) + let [portals, PortalWrapper] = useNestedPortals() + let root = useRootContainers({ + portals, + defaultContainers: [button, panel], + }) + // Handle focus out useEventListener( ownerDocument?.defaultView, 'focus', (event) => { + if (event.target === window) return + if (!(event.target instanceof HTMLElement)) return if (popoverState !== PopoverStates.Open) return if (isFocusWithinPopoverGroup()) return if (!button) return if (!panel) return - if (event.target === window) return - if (beforePanelSentinel.current?.contains?.(event.target as HTMLElement)) return - if (afterPanelSentinel.current?.contains?.(event.target as HTMLElement)) return + if (root.contains(event.target)) return + if (beforePanelSentinel.current?.contains?.(event.target)) return + if (afterPanelSentinel.current?.contains?.(event.target)) return dispatch({ type: ActionTypes.ClosePopover }) }, @@ -329,7 +339,7 @@ function PopoverFn( // Handle outside click useOutsideClick( - [button, panel], + root.resolveContainers, (event, target) => { dispatch({ type: ActionTypes.ClosePopover }) @@ -385,13 +395,16 @@ function PopoverFn( [PopoverStates.Closed]: State.Closed, })} > - {render({ - ourProps, - theirProps, - slot, - defaultTag: DEFAULT_POPOVER_TAG, - name: 'Popover', - })} + + {render({ + ourProps, + theirProps, + slot, + defaultTag: DEFAULT_POPOVER_TAG, + name: 'Popover', + })} + + diff --git a/packages/@headlessui-react/src/components/portal/portal.tsx b/packages/@headlessui-react/src/components/portal/portal.tsx index 748eaa6d46..77d854939b 100644 --- a/packages/@headlessui-react/src/components/portal/portal.tsx +++ b/packages/@headlessui-react/src/components/portal/portal.tsx @@ -10,6 +10,8 @@ import React, { ElementType, MutableRefObject, Ref, + useMemo, + ContextType, } from 'react' import { createPortal } from 'react-dom' @@ -22,6 +24,7 @@ import { optionalRef, useSyncRefs } from '../../hooks/use-sync-refs' import { useOnUnmount } from '../../hooks/use-on-unmount' import { useOwnerDocument } from '../../hooks/use-owner' import { env } from '../../utils/env' +import { useEvent } from '../../hooks/use-event' function usePortalTarget(ref: MutableRefObject): HTMLElement | null { let forceInRoot = usePortalRoot() @@ -87,7 +90,7 @@ function PortalFn( let [element] = useState(() => env.isServer ? null : ownerDocument?.createElement('div') ?? null ) - + let parent = useContext(PortalParentContext) let ready = useServerHandoffComplete() useIsoMorphicEffect(() => { @@ -101,6 +104,13 @@ function PortalFn( } }, [target, element]) + useIsoMorphicEffect(() => { + if (!element) return + if (!parent) return + + return parent.register(element) + }, [parent, element]) + useOnUnmount(() => { if (!target || !element) return @@ -164,6 +174,45 @@ function GroupFn( // --- +let PortalParentContext = createContext<{ + register: (portal: HTMLElement) => () => void + unregister: (portal: HTMLElement) => void + portals: MutableRefObject +} | null>(null) + +export function useNestedPortals() { + let parent = useContext(PortalParentContext) + let portals = useRef([]) + + let register = useEvent((portal: HTMLElement) => { + portals.current.push(portal) + if (parent) parent.register(portal) + return () => unregister(portal) + }) + + let unregister = useEvent((portal: HTMLElement) => { + let idx = portals.current.indexOf(portal) + if (idx !== -1) portals.current.splice(idx, 1) + if (parent) parent.unregister(portal) + }) + + let api = useMemo>( + () => ({ register, unregister, portals }), + [register, unregister, portals] + ) + + return [ + portals, + useMemo(() => { + return function PortalWrapper({ children }: { children: React.ReactNode }) { + return {children} + } + }, [api]), + ] as const +} + +// --- + interface ComponentPortal extends HasDisplayName { ( props: PortalProps & RefProp diff --git a/packages/@headlessui-react/src/hooks/use-outside-click.ts b/packages/@headlessui-react/src/hooks/use-outside-click.ts index f023310618..c9b57c2a56 100644 --- a/packages/@headlessui-react/src/hooks/use-outside-click.ts +++ b/packages/@headlessui-react/src/hooks/use-outside-click.ts @@ -38,6 +38,15 @@ export function useOutsideClick( // behaviour so that only the Menu closes and not the Dialog (yet) if (event.defaultPrevented) return + let target = resolveTarget(event) + + if (target === null) { + return + } + + // Ignore if the target doesn't exist in the DOM anymore + if (!target.getRootNode().contains(target)) return + let _containers = (function resolve(containers): ContainerCollection { if (typeof containers === 'function') { return resolve(containers()) @@ -54,15 +63,6 @@ export function useOutsideClick( return [containers] })(containers) - let target = resolveTarget(event) - - if (target === null) { - return - } - - // Ignore if the target doesn't exist in the DOM anymore - if (!target.getRootNode().contains(target)) return - // Ignore if the target exists in one of the containers for (let container of _containers) { if (container === null) continue diff --git a/packages/@headlessui-react/src/hooks/use-root-containers.tsx b/packages/@headlessui-react/src/hooks/use-root-containers.tsx new file mode 100644 index 0000000000..04bf746478 --- /dev/null +++ b/packages/@headlessui-react/src/hooks/use-root-containers.tsx @@ -0,0 +1,64 @@ +import React, { useRef, useMemo, MutableRefObject } from 'react' +import { Hidden, Features as HiddenFeatures } from '../internal/hidden' +import { useEvent } from './use-event' +import { useOwnerDocument } from './use-owner' + +export function useRootContainers({ + defaultContainers = [], + portals, +}: { + defaultContainers?: (HTMLElement | null | MutableRefObject)[] + portals?: MutableRefObject +} = {}) { + // Reference to a node in the "main" tree, not in the portalled Dialog tree. + let mainTreeNodeRef = useRef(null) + let ownerDocument = useOwnerDocument(mainTreeNodeRef) + + let resolveContainers = useEvent(() => { + let containers: HTMLElement[] = [] + + // Resolve default containers + for (let container of defaultContainers) { + if (container === null) continue + if (container instanceof HTMLElement) { + containers.push(container) + } else if ('current' in container && container.current instanceof HTMLElement) { + containers.push(container.current) + } + } + + // Resolve portal containers + if (portals?.current) { + for (let portal of portals.current) { + containers.push(portal) + } + } + + // Resolve third party (root) containers + for (let container of ownerDocument?.querySelectorAll('html > *, body > *') ?? []) { + if (container === document.body) continue // Skip `` + if (container === document.head) continue // Skip `` + if (!(container instanceof HTMLElement)) continue // Skip non-HTMLElements + if (container.id === 'headlessui-portal-root') continue // Skip the Headless UI portal root + if (container.contains(mainTreeNodeRef.current)) continue // Skip if it is the main app + if (containers.some((defaultContainer) => container.contains(defaultContainer))) continue // Skip if the current container is part of a container we've already seen (e.g.: default container / portal) + + containers.push(container) + } + + return containers + }) + + return { + resolveContainers, + contains: useEvent((element: HTMLElement) => + resolveContainers().some((container) => container.contains(element)) + ), + mainTreeNodeRef, + MainTreeNode: useMemo(() => { + return function MainTreeNode() { + return + } + }, [mainTreeNodeRef]), + } +} diff --git a/packages/@headlessui-react/src/index.test.ts b/packages/@headlessui-react/src/index.test.ts index 1058a5ed5b..15c12a3471 100644 --- a/packages/@headlessui-react/src/index.test.ts +++ b/packages/@headlessui-react/src/index.test.ts @@ -6,6 +6,7 @@ import * as HeadlessUI from './index' */ it('should expose the correct components', () => { expect(Object.keys(HeadlessUI)).toEqual([ + 'Portal', 'Combobox', 'Dialog', 'Disclosure', @@ -13,7 +14,6 @@ it('should expose the correct components', () => { 'Listbox', 'Menu', 'Popover', - 'Portal', 'RadioGroup', 'Switch', 'Tab', diff --git a/packages/@headlessui-react/src/index.ts b/packages/@headlessui-react/src/index.ts index db3f84f533..b6c2f2b58d 100644 --- a/packages/@headlessui-react/src/index.ts +++ b/packages/@headlessui-react/src/index.ts @@ -7,8 +7,8 @@ export * from './components/focus-trap/focus-trap' export * from './components/listbox/listbox' export * from './components/menu/menu' export * from './components/popover/popover' -export * from './components/portal/portal' export * from './components/radio-group/radio-group' export * from './components/switch/switch' export * from './components/tabs/tabs' export * from './components/transitions/transition' +export { Portal } from './components/portal/portal' diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index f1b0c20bd6..e6a33bf5ba 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure the exposed `activeIndex` is up to date for the `Combobox` component ([#2463](https://github.com/tailwindlabs/headlessui/pull/2463)) - Improve control over `Menu` and `Listbox` options while searching ([#2471](https://github.com/tailwindlabs/headlessui/pull/2471)) - Consider clicks inside iframes to be "outside" ([#2485](https://github.com/tailwindlabs/headlessui/pull/2485)) +- Ensure moving focus within a `Portal` component, does not close the `Popover` component ([#2492](https://github.com/tailwindlabs/headlessui/pull/2492)) ### Changed diff --git a/packages/@headlessui-vue/src/components/dialog/dialog.ts b/packages/@headlessui-vue/src/components/dialog/dialog.ts index 14cd9cd64a..ef3ffbb0f7 100644 --- a/packages/@headlessui-vue/src/components/dialog/dialog.ts +++ b/packages/@headlessui-vue/src/components/dialog/dialog.ts @@ -22,7 +22,7 @@ import { Keys } from '../../keyboard' import { useId } from '../../hooks/use-id' import { FocusTrap } from '../../components/focus-trap/focus-trap' import { useInert } from '../../hooks/use-inert' -import { Portal, PortalGroup } from '../portal/portal' +import { Portal, PortalGroup, useNestedPortals } from '../portal/portal' import { StackMessage, useStackProvider } from '../../internal/stack-context' import { match } from '../../utils/match' import { ForcePortalRoot } from '../../internal/portal-force-root' @@ -32,8 +32,8 @@ import { useOpenClosed, State } from '../../internal/open-closed' import { useOutsideClick } from '../../hooks/use-outside-click' import { getOwnerDocument } from '../../utils/owner' import { useEventListener } from '../../hooks/use-event-listener' -import { Hidden, Features as HiddenFeatures } from '../../internal/hidden' import { useDocumentOverflowLockedEffect } from '../../hooks/document-overflow/use-document-overflow' +import { useRootContainers } from '../../hooks/use-root-containers' enum DialogStates { Open, @@ -97,9 +97,6 @@ export let Dialog = defineComponent({ let internalDialogRef = ref(null) - // Reference to a node in the "main" tree, not in the portalled Dialog tree. - let mainTreeNode = ref(null) - let ownerDocument = computed(() => getOwnerDocument(internalDialogRef)) expose({ el: internalDialogRef, $el: internalDialogRef }) @@ -126,6 +123,15 @@ export let Dialog = defineComponent({ let hasNestedDialogs = computed(() => nestedDialogCount.value > 1) // 1 is the current dialog let hasParentDialog = inject(DialogContext, null) !== null + let [portals, PortalWrapper] = useNestedPortals() + let { + resolveContainers: resolveRootContainers, + mainTreeNodeRef, + MainTreeNode, + } = useRootContainers({ + portals, + defaultContainers: [computed(() => api.panelRef.value ?? internalDialogRef.value)], + }) // If there are multiple dialogs, then you can be the root, the leaf or one // in between. We only care abou whether you are the top most one or not. @@ -155,7 +161,7 @@ export let Dialog = defineComponent({ if (root.id === 'headlessui-portal-root') return false // Find the root of the main tree node - return root.contains(dom(mainTreeNode)) && root instanceof HTMLElement + return root.contains(dom(mainTreeNodeRef)) && root instanceof HTMLElement }) ?? null) as HTMLElement | null }) useInert(resolveRootOfMainTreeNode, inertOthersEnabled) @@ -168,7 +174,7 @@ export let Dialog = defineComponent({ let resolveRootOfParentDialog = computed(() => { return (Array.from( ownerDocument.value?.querySelectorAll('[data-headlessui-portal]') ?? [] - ).find((root) => root.contains(dom(mainTreeNode)) && root instanceof HTMLElement) ?? + ).find((root) => root.contains(dom(mainTreeNodeRef)) && root instanceof HTMLElement) ?? null) as HTMLElement | null }) useInert(resolveRootOfParentDialog, inertParentDialogs) @@ -209,22 +215,6 @@ export let Dialog = defineComponent({ provide(DialogContext, api) - function resolveRootContainers() { - // Third party roots - let rootContainers = Array.from( - ownerDocument.value?.querySelectorAll('html > *, body > *, [data-headlessui-portal]') ?? [] - ).filter((container) => { - if (container === document.body) return false // Skip `` - if (container === document.head) return false // Skip `` - if (!(container instanceof HTMLElement)) return false // Skip non-HTMLElements - if (container.contains(dom(mainTreeNode))) return false // Skip if it is the main app - if (api.panelRef.value && container.contains(api.panelRef.value)) return false - return true // Keep - }) - - return [...rootContainers, api.panelRef.value ?? internalDialogRef.value] as HTMLElement[] - } - // Handle outside click let outsideClickEnabled = computed(() => { if (!enabled.value) return false @@ -232,7 +222,7 @@ export let Dialog = defineComponent({ return true }) useOutsideClick( - () => resolveRootContainers(), + resolveRootContainers, (_event, target) => { api.close() nextTick(() => target?.focus()) @@ -320,21 +310,23 @@ export let Dialog = defineComponent({ : FocusTrap.features.None, }, () => - render({ - ourProps, - theirProps, - slot, - attrs, - slots, - visible: dialogState.value === DialogStates.Open, - features: Features.RenderStrategy | Features.Static, - name: 'Dialog', - }) + h(PortalWrapper, {}, () => + render({ + ourProps, + theirProps: { ...theirProps, ...attrs }, + slot, + attrs, + slots, + visible: dialogState.value === DialogStates.Open, + features: Features.RenderStrategy | Features.Static, + name: 'Dialog', + }) + ) ) ) ) ), - h(Hidden, { features: HiddenFeatures.Hidden, ref: mainTreeNode }), + h(MainTreeNode), ]) } }, diff --git a/packages/@headlessui-vue/src/components/popover/popover.test.ts b/packages/@headlessui-vue/src/components/popover/popover.test.ts index 825f2b33b4..449bb0d3b5 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.test.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.test.ts @@ -28,18 +28,14 @@ beforeAll(() => { afterAll(() => jest.restoreAllMocks()) -function getDefaultComponents() { - return { - Popover, - PopoverGroup, - PopoverButton, - PopoverPanel, - PopoverOverlay, - Portal, - } -} - -const renderTemplate = createRenderTemplate(getDefaultComponents()) +const renderTemplate = createRenderTemplate({ + Popover, + PopoverGroup, + PopoverButton, + PopoverPanel, + PopoverOverlay, + Portal, +}) describe('Safe guards', () => { it.each([ @@ -2573,6 +2569,87 @@ describe('Mouse interactions', () => { assertActiveElement(getPopoverButton()) }) ) + + it( + 'should not close the Popover if the focus is moved outside of the Popover but still in the same React tree using Portals', + suppressConsoleLogs(async () => { + let clickFn = jest.fn() + renderTemplate({ + template: html` + + Toggle + + + + + + + `, + setup: () => ({ clickFn }), + }) + + // Open the popover + await click(getPopoverButton()) + + // Verify it is open + assertPopoverPanel({ state: PopoverState.Visible }) + + // Click the button outside the Popover (DOM) but inside (Portal / React tree) + await click(getByText('foo')) + + // Verify it is still open + assertPopoverPanel({ state: PopoverState.Visible }) + + // Verify the button was clicked + expect(clickFn).toHaveBeenCalled() + }) + ) + + it( + 'should not close the Popover if the focus is moved outside of the Popover but still in the same React tree using nested Portals', + suppressConsoleLogs(async () => { + let clickFn = jest.fn() + renderTemplate({ + template: html` + + Toggle + + Level 0 + + Level 1 + + Level 2 + + Level 3 + + Level 4 + + + + + + + + `, + setup: () => ({ clickFn }), + }) + + // Open the popover + await click(getPopoverButton()) + + // Verify it is open + assertPopoverPanel({ state: PopoverState.Visible }) + + // Click the button outside the Popover (DOM) but inside (Portal / React tree) + await click(getByText('foo')) + + // Verify it is still open + assertPopoverPanel({ state: PopoverState.Visible }) + + // Verify the button was clicked + expect(clickFn).toHaveBeenCalled() + }) + ) }) describe('Nested popovers', () => { diff --git a/packages/@headlessui-vue/src/components/popover/popover.ts b/packages/@headlessui-vue/src/components/popover/popover.ts index 8a19b91c4d..858e814418 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.ts @@ -37,6 +37,8 @@ import { useEventListener } from '../../hooks/use-event-listener' import { Hidden, Features as HiddenFeatures } from '../../internal/hidden' import { useTabDirection, Direction as TabDirection } from '../../hooks/use-tab-direction' import { microTask } from '../../utils/micro-task' +import { useRootContainers } from '../../hooks/use-root-containers' +import { useNestedPortals } from '../../components/portal/portal' enum PopoverStates { Open, @@ -204,6 +206,12 @@ export let Popover = defineComponent({ let groupContext = usePopoverGroupContext() let registerPopover = groupContext?.registerPopover + let [portals, PortalWrapper] = useNestedPortals() + let root = useRootContainers({ + portals, + defaultContainers: [button, panel], + }) + function isFocusWithinPopoverGroup() { return ( groupContext?.isFocusWithinPopoverGroup() ?? @@ -220,13 +228,15 @@ export let Popover = defineComponent({ ownerDocument.value?.defaultView, 'focus', (event) => { + if (event.target === window) return + if (!(event.target instanceof HTMLElement)) return if (popoverState.value !== PopoverStates.Open) return if (isFocusWithinPopoverGroup()) return if (!button) return if (!panel) return - if (event.target === window) return - if (dom(api.beforePanelSentinel)?.contains(event.target as HTMLElement)) return - if (dom(api.afterPanelSentinel)?.contains(event.target as HTMLElement)) return + if (root.contains(event.target)) return + if (dom(api.beforePanelSentinel)?.contains(event.target)) return + if (dom(api.afterPanelSentinel)?.contains(event.target)) return api.closePopover() }, @@ -235,7 +245,7 @@ export let Popover = defineComponent({ // Handle outside click useOutsideClick( - [button, panel], + root.resolveContainers, (event, target) => { api.closePopover() @@ -249,14 +259,16 @@ export let Popover = defineComponent({ return () => { let slot = { open: popoverState.value === PopoverStates.Open, close: api.close } - return render({ - theirProps: props, - ourProps: { ref: internalPopoverRef }, - slot, - slots, - attrs, - name: 'Popover', - }) + return h(PortalWrapper, {}, () => + render({ + theirProps: { ...props, ...attrs }, + ourProps: { ref: internalPopoverRef }, + slot, + slots, + attrs, + name: 'Popover', + }) + ) } }, }) diff --git a/packages/@headlessui-vue/src/components/portal/portal.ts b/packages/@headlessui-vue/src/components/portal/portal.ts index d1aee879d1..f82eef33b1 100644 --- a/packages/@headlessui-vue/src/components/portal/portal.ts +++ b/packages/@headlessui-vue/src/components/portal/portal.ts @@ -1,8 +1,10 @@ import { Teleport, + computed, defineComponent, h, inject, + onMounted, onUnmounted, provide, reactive, @@ -12,11 +14,14 @@ import { // Types InjectionKey, PropType, - computed, + Ref, } from 'vue' import { render } from '../../utils/render' import { usePortalRoot } from '../../internal/portal-force-root' import { getOwnerDocument } from '../../utils/owner' +import { dom } from '../../utils/dom' + +type ContextType = T extends InjectionKey ? V : never // --- @@ -64,6 +69,15 @@ export let Portal = defineComponent({ myTarget.value = groupContext.resolveTarget() }) + let parent = inject(PortalParentContext, null) + onMounted(() => { + let domElement = dom(element) + if (!domElement) return + if (!parent) return + + onUnmounted(parent.register(domElement)) + }) + onUnmounted(() => { let root = ownerDocument.value?.getElementById('headlessui-portal-root') if (!root) return @@ -102,6 +116,48 @@ export let Portal = defineComponent({ // --- +let PortalParentContext = Symbol('PortalParentContext') as InjectionKey<{ + register: (portal: HTMLElement) => () => void + unregister: (portal: HTMLElement) => void + portals: Ref +}> + +export function useNestedPortals() { + let parent = inject(PortalParentContext, null) + let portals = ref([]) + + function register(portal: HTMLElement) { + portals.value.push(portal) + if (parent) parent.register(portal) + return () => unregister(portal) + } + + function unregister(portal: HTMLElement) { + let idx = portals.value.indexOf(portal) + if (idx !== -1) portals.value.splice(idx, 1) + if (parent) parent.unregister(portal) + } + + let api = { + register, + unregister, + portals, + } as ContextType + + return [ + portals, + defineComponent({ + name: 'PortalWrapper', + setup(_, { slots }) { + provide(PortalParentContext, api) + return () => slots.default?.() + }, + }), + ] as const +} + +// --- + let PortalGroupContext = Symbol('PortalGroupContext') as InjectionKey<{ resolveTarget(): HTMLElement | null }> diff --git a/packages/@headlessui-vue/src/hooks/use-root-containers.ts b/packages/@headlessui-vue/src/hooks/use-root-containers.ts new file mode 100644 index 0000000000..bf0590a24c --- /dev/null +++ b/packages/@headlessui-vue/src/hooks/use-root-containers.ts @@ -0,0 +1,62 @@ +import { ref, h, Ref } from 'vue' +import { Hidden, Features as HiddenFeatures } from '../internal/hidden' +import { getOwnerDocument } from '../utils/owner' +import { dom } from '../utils/dom' + +export function useRootContainers({ + defaultContainers = [], + portals, +}: { + defaultContainers?: (HTMLElement | null | Ref)[] + portals?: Ref +} = {}) { + // Reference to a node in the "main" tree, not in the portalled Dialog tree. + let mainTreeNodeRef = ref(null) + let ownerDocument = getOwnerDocument(mainTreeNodeRef) + + function resolveContainers() { + let containers: HTMLElement[] = [] + + // Resolve default containers + for (let container of defaultContainers) { + if (container === null) continue + if (container instanceof HTMLElement) { + containers.push(container) + } else if ('value' in container && container.value instanceof HTMLElement) { + containers.push(container.value) + } + } + + // Resolve portal containers + if (portals?.value) { + for (let portal of portals.value) { + containers.push(portal) + } + } + + // Resolve third party (root) containers + for (let container of ownerDocument?.querySelectorAll('html > *, body > *') ?? []) { + if (container === document.body) continue // Skip `` + if (container === document.head) continue // Skip `` + if (!(container instanceof HTMLElement)) continue // Skip non-HTMLElements + if (container.id === 'headlessui-portal-root') continue // Skip the Headless UI portal root + if (container.contains(dom(mainTreeNodeRef))) continue // Skip if it is the main app + if (containers.some((defaultContainer) => container.contains(defaultContainer))) continue // Skip if the current container is part of a container we've already seen (e.g.: default container / portal) + + containers.push(container) + } + + return containers + } + + return { + resolveContainers, + contains(element: HTMLElement) { + return resolveContainers().some((container) => container.contains(element)) + }, + mainTreeNodeRef, + MainTreeNode() { + return h(Hidden, { features: HiddenFeatures.Hidden, ref: mainTreeNodeRef }) + }, + } +} diff --git a/packages/@headlessui-vue/src/index.test.ts b/packages/@headlessui-vue/src/index.test.ts index 8352a47c4f..607f559a34 100644 --- a/packages/@headlessui-vue/src/index.test.ts +++ b/packages/@headlessui-vue/src/index.test.ts @@ -6,6 +6,10 @@ import * as HeadlessUI from './index' */ it('should expose the correct components', () => { expect(Object.keys(HeadlessUI)).toEqual([ + // Portal + 'Portal', + 'PortalGroup', + // Combobox 'Combobox', 'ComboboxLabel', @@ -50,10 +54,6 @@ it('should expose the correct components', () => { 'PopoverPanel', 'PopoverGroup', - // Portal - 'Portal', - 'PortalGroup', - // RadioGroup 'RadioGroup', 'RadioGroupOption', diff --git a/packages/@headlessui-vue/src/index.ts b/packages/@headlessui-vue/src/index.ts index 2ba6c32e78..fb87322695 100644 --- a/packages/@headlessui-vue/src/index.ts +++ b/packages/@headlessui-vue/src/index.ts @@ -5,8 +5,8 @@ export * from './components/focus-trap/focus-trap' export * from './components/listbox/listbox' export * from './components/menu/menu' export * from './components/popover/popover' -export * from './components/portal/portal' export * from './components/radio-group/radio-group' export * from './components/switch/switch' export * from './components/tabs/tabs' export * from './components/transitions/transition' +export { Portal, PortalGroup } from './components/portal/portal'