diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a75b4e..0b61b8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Improve SSR for `Dialog` ([#477](https://github.com/tailwindlabs/headlessui/pull/477)) - Delay focus trap initialization ([#477](https://github.com/tailwindlabs/headlessui/pull/477)) +- Improve incorrect behaviour for nesting `Dialog` components ([#560](https://github.com/tailwindlabs/headlessui/pull/560)) ## [Unreleased - Vue] diff --git a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx index e49273a..6fa6127 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx @@ -14,6 +14,7 @@ import { getByText, assertActiveElement, getDialogs, + getDialogOverlays, } from '../../test-utils/accessibility-assertions' import { click, press, Keys } from '../../test-utils/interactions' import { PropsOf } from '../../types' @@ -637,79 +638,205 @@ describe('Mouse interactions', () => { }) describe('Nesting', () => { - it('should be possible to open nested Dialog components and close them with `Escape`', async () => { - function Nested({ onClose, level = 1 }: { onClose: (value: boolean) => void; level?: number }) { - let [showChild, setShowChild] = useState(false) + function Nested({ onClose, level = 1 }: { onClose: (value: boolean) => void; level?: number }) { + let [showChild, setShowChild] = useState(false) - return ( - <> - -
-

Level: {level}

- -
- {showChild && } - - - ) + return ( + <> + + + +
+

Level: {level}

+ + + +
+ {showChild && } + + + ) + } + + function Example() { + let [open, setOpen] = useState(false) + + return ( + <> + + {open && } + + ) + } + + it.each` + strategy | action + ${'with `Escape`'} | ${() => press(Keys.Escape)} + ${'with `Outside Click`'} | ${() => click(document.body)} + ${'with `Click on Dialog.Overlay`'} | ${() => click(getDialogOverlays().pop()!)} + `( + 'should be possible to open nested Dialog components and close them $strategy', + async ({ action }) => { + render() + + // Verify we have no open dialogs + expect(getDialogs()).toHaveLength(0) + + // Open Dialog 1 + await click(getByText('Open 1')) + + // Verify that we have 1 open dialog + expect(getDialogs()).toHaveLength(1) + + // Verify that the `Open 2 a` has focus + assertActiveElement(getByText('Open 2 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 a')) + + // Open Dialog 2 via the second button + await click(getByText('Open 2 b')) + + // Verify that we have 2 open dialogs + expect(getDialogs()).toHaveLength(2) + + // Verify that the `Open 3 a` has focus + assertActiveElement(getByText('Open 3 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 a')) + + // Close the top most Dialog + await action() + + // Verify that we have 1 open dialog + expect(getDialogs()).toHaveLength(1) + + // Verify that the `Open 2 b` button got focused again + assertActiveElement(getByText('Open 2 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 b')) + + // Open Dialog 2 via button b + await click(getByText('Open 2 b')) + + // Verify that the `Open 3 a` has focus + assertActiveElement(getByText('Open 3 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 a')) + + // Verify that we have 2 open dialogs + expect(getDialogs()).toHaveLength(2) + + // Open Dialog 3 via button c + await click(getByText('Open 3 c')) + + // Verify that the `Open 4 a` has focus + assertActiveElement(getByText('Open 4 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 4 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 4 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 4 a')) + + // Verify that we have 3 open dialogs + expect(getDialogs()).toHaveLength(3) + + // Close the top most Dialog + await action() + + // Verify that the `Open 3 c` button got focused again + assertActiveElement(getByText('Open 3 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 c')) + + // Verify that we have 2 open dialogs + expect(getDialogs()).toHaveLength(2) + + // Close the top most Dialog + await action() + + // Verify that we have 1 open dialog + expect(getDialogs()).toHaveLength(1) + + // Verify that the `Open 2 b` button got focused again + assertActiveElement(getByText('Open 2 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 b')) + + // Close the top most Dialog + await action() + + // Verify that we have 0 open dialogs + expect(getDialogs()).toHaveLength(0) + + // Verify that the `Open 1` button got focused again + assertActiveElement(getByText('Open 1')) } - - function Example() { - let [open, setOpen] = useState(false) - - return ( - <> - - {open && } - - ) - } - - render() - - // Verify we have no open dialogs - expect(getDialogs()).toHaveLength(0) - - // Open Dialog 1 - await click(getByText('Open 1')) - - // Verify that we have 1 open dialog - expect(getDialogs()).toHaveLength(1) - - // Open Dialog 2 - await click(getByText('Open 2')) - - // Verify that we have 2 open dialogs - expect(getDialogs()).toHaveLength(2) - - // Press escape to close the top most Dialog - await press(Keys.Escape) - - // Verify that we have 1 open dialog - expect(getDialogs()).toHaveLength(1) - - // Open Dialog 2 - await click(getByText('Open 2')) - - // Verify that we have 2 open dialogs - expect(getDialogs()).toHaveLength(2) - - // Open Dialog 3 - await click(getByText('Open 3')) - - // Verify that we have 3 open dialogs - expect(getDialogs()).toHaveLength(3) - - // Press escape to close the top most Dialog - await press(Keys.Escape) - - // Verify that we have 2 open dialogs - expect(getDialogs()).toHaveLength(2) - - // Press escape to close the top most Dialog - await press(Keys.Escape) - - // Verify that we have 1 open dialog - expect(getDialogs()).toHaveLength(1) - }) + ) }) diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx index 7319db6..2055bd4 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.tsx @@ -15,6 +15,7 @@ import React, { KeyboardEvent as ReactKeyboardEvent, MutableRefObject, Ref, + useState, } from 'react' import { Props } from '../../types' @@ -24,16 +25,15 @@ import { useSyncRefs } from '../../hooks/use-sync-refs' import { Keys } from '../keyboard' import { isDisabledReactIssue7711 } from '../../utils/bugs' import { useId } from '../../hooks/use-id' -import { useFocusTrap } from '../../hooks/use-focus-trap' +import { useFocusTrap, Features as FocusTrapFeatures } from '../../hooks/use-focus-trap' import { useInertOthers } from '../../hooks/use-inert-others' import { Portal } from '../../components/portal/portal' -import { StackProvider, StackMessage } from '../../internal/stack-context' import { ForcePortalRoot } from '../../internal/portal-force-root' -import { contains } from '../../internal/dom-containers' import { Description, useDescriptions } from '../description/description' import { useWindowEvent } from '../../hooks/use-window-event' import { useOpenClosed, State } from '../../internal/open-closed' import { useServerHandoffComplete } from '../../hooks/use-server-handoff-complete' +import { StackProvider, StackMessage } from '../../internal/stack-context' enum DialogStates { Open, @@ -117,6 +117,7 @@ let DialogRoot = forwardRefWithAs(function Dialog< ref: Ref ) { let { open, onClose, initialFocus, ...rest } = props + let [nestedDialogCount, setNestedDialogCount] = useState(0) let usesOpenClosedState = useOpenClosed() if (open === undefined && usesOpenClosedState !== null) { @@ -127,7 +128,7 @@ let DialogRoot = forwardRefWithAs(function Dialog< }) } - let containers = useRef>(new Set()) + let containers = useRef>>(new Set()) let internalDialogRef = useRef(null) let dialogRef = useSyncRefs(internalDialogRef, ref) @@ -184,13 +185,34 @@ let DialogRoot = forwardRefWithAs(function Dialog< [dispatch] ) + let ready = useServerHandoffComplete() + let enabled = ready && dialogState === DialogStates.Open + let hasNestedDialogs = nestedDialogCount > 1 // 1 is the current dialog + let hasParentDialog = useContext(DialogContext) !== null + + // 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. + let position = !hasNestedDialogs ? 'leaf' : 'parent' + + useFocusTrap( + internalDialogRef, + enabled + ? match(position, { + parent: FocusTrapFeatures.RestoreFocus, + leaf: FocusTrapFeatures.All, + }) + : FocusTrapFeatures.None, + { initialFocus, containers } + ) + useInertOthers(internalDialogRef, hasNestedDialogs ? enabled : false) + // Handle outside click useWindowEvent('mousedown', event => { let target = event.target as HTMLElement if (dialogState !== DialogStates.Open) return - if (containers.current.size !== 1) return - if (contains(containers.current, target)) return + if (hasNestedDialogs) return + if (internalDialogRef.current?.contains(target)) return close() }) @@ -198,6 +220,7 @@ let DialogRoot = forwardRefWithAs(function Dialog< // Scroll lock useEffect(() => { if (dialogState !== DialogStates.Open) return + if (hasParentDialog) return let overflow = document.documentElement.style.overflow let paddingRight = document.documentElement.style.paddingRight @@ -211,7 +234,7 @@ let DialogRoot = forwardRefWithAs(function Dialog< document.documentElement.style.overflow = overflow document.documentElement.style.paddingRight = paddingRight } - }, [dialogState]) + }, [dialogState, hasParentDialog]) // Trigger close when the FocusTrap gets hidden useEffect(() => { @@ -236,11 +259,6 @@ let DialogRoot = forwardRefWithAs(function Dialog< return () => observer.disconnect() }, [dialogState, internalDialogRef, close]) - let ready = useServerHandoffComplete() - let enabled = ready && dialogState === DialogStates.Open - - useFocusTrap(containers, enabled, { initialFocus }) - useInertOthers(internalDialogRef, enabled) let [describedby, DescriptionProvider] = useDescriptions() let id = `headlessui-dialog-${useId()}` @@ -269,7 +287,7 @@ let DialogRoot = forwardRefWithAs(function Dialog< onKeyDown(event: ReactKeyboardEvent) { if (event.key !== Keys.Escape) return if (dialogState !== DialogStates.Open) return - if (containers.current.size > 1) return // 1 is myself, otherwise other elements in the Stack + if (hasNestedDialogs) return event.preventDefault() event.stopPropagation() close() @@ -279,16 +297,22 @@ let DialogRoot = forwardRefWithAs(function Dialog< return ( { - return match(message, { - [StackMessage.AddElement]() { + type="Dialog" + element={internalDialogRef} + onUpdate={useCallback((message, type, element) => { + if (type !== 'Dialog') return + + match(message, { + [StackMessage.Add]() { containers.current.add(element) + setNestedDialogCount(count => count + 1) }, - [StackMessage.RemoveElement]() { - containers.current.delete(element) + [StackMessage.Remove]() { + containers.current.add(element) + setNestedDialogCount(count => count - 1) }, }) - }} + }, [])} > diff --git a/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx b/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx index ee587a7..360fb4b 100644 --- a/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx +++ b/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx @@ -8,7 +8,7 @@ import { import { Props } from '../../types' import { render } from '../../utils/render' -import { useFocusTrap } from '../../hooks/use-focus-trap' +import { useFocusTrap, Features as FocusTrapFeatures } from '../../hooks/use-focus-trap' import { useServerHandoffComplete } from '../../hooks/use-server-handoff-complete' let DEFAULT_FOCUS_TRAP_TAG = 'div' as const @@ -16,17 +16,14 @@ let DEFAULT_FOCUS_TRAP_TAG = 'div' as const export function FocusTrap( props: Props & { initialFocus?: MutableRefObject } ) { - let containers = useRef>(new Set()) + let container = useRef(null) let { initialFocus, ...passthroughProps } = props let ready = useServerHandoffComplete() - useFocusTrap(containers, ready, { initialFocus }) + useFocusTrap(container, ready ? FocusTrapFeatures.All : FocusTrapFeatures.None, { initialFocus }) let propsWeControl = { - ref(element: HTMLElement | null) { - if (!element) return - containers.current.add(element) - }, + ref: container, } return render({ diff --git a/packages/@headlessui-react/src/components/portal/portal.tsx b/packages/@headlessui-react/src/components/portal/portal.tsx index 93d55b3..24cd314 100644 --- a/packages/@headlessui-react/src/components/portal/portal.tsx +++ b/packages/@headlessui-react/src/components/portal/portal.tsx @@ -14,7 +14,6 @@ import { createPortal } from 'react-dom' import { Props } from '../../types' import { render } from '../../utils/render' import { useIsoMorphicEffect } from '../../hooks/use-iso-morphic-effect' -import { useElementStack, StackProvider } from '../../internal/stack-context' import { usePortalRoot } from '../../internal/portal-force-root' import { useServerHandoffComplete } from '../../hooks/use-server-handoff-complete' @@ -60,8 +59,6 @@ export function Portal( let ready = useServerHandoffComplete() - useElementStack(element) - useIsoMorphicEffect(() => { if (!target) return if (!element) return @@ -82,16 +79,12 @@ export function Portal( if (!ready) return null - return ( - - {!target || !element - ? null - : createPortal( - render({ props: passthroughProps, defaultTag: DEFAULT_PORTAL_TAG, name: 'Portal' }), - element - )} - - ) + return !target || !element + ? null + : createPortal( + render({ props: passthroughProps, defaultTag: DEFAULT_PORTAL_TAG, name: 'Portal' }), + element + ) } // --- diff --git a/packages/@headlessui-react/src/hooks/use-focus-trap.ts b/packages/@headlessui-react/src/hooks/use-focus-trap.ts index 600039b..2b7eb90 100644 --- a/packages/@headlessui-react/src/hooks/use-focus-trap.ts +++ b/packages/@headlessui-react/src/hooks/use-focus-trap.ts @@ -7,93 +7,124 @@ import { import { Keys } from '../components/keyboard' import { focusElement, focusIn, Focus, FocusResult } from '../utils/focus-management' -import { contains } from '../internal/dom-containers' import { useWindowEvent } from './use-window-event' +import { useIsMounted } from './use-is-mounted' + +export enum Features { + /** No features enabled for the `useFocusTrap` hook. */ + None = 1 << 0, + + /** Ensure that we move focus initially into the container. */ + InitialFocus = 1 << 1, + + /** Ensure that pressing `Tab` and `Shift+Tab` is trapped within the container. */ + TabLock = 1 << 2, + + /** Ensure that programmatically moving focus outside of the container is disallowed. */ + FocusLock = 1 << 3, + + /** Ensure that we restore the focus when unmounting the component that uses this `useFocusTrap` hook. */ + RestoreFocus = 1 << 4, + + /** Enable all features. */ + All = InitialFocus | TabLock | FocusLock | RestoreFocus, +} export function useFocusTrap( - containers: MutableRefObject>, - enabled: boolean = true, - options: { initialFocus?: MutableRefObject } = {} + container: MutableRefObject, + features: Features = Features.All, + { + initialFocus, + containers, + }: { + initialFocus?: MutableRefObject + containers?: MutableRefObject>> + } = {} ) { let restoreElement = useRef( typeof window !== 'undefined' ? (document.activeElement as HTMLElement) : null ) let previousActiveElement = useRef(null) - let mounted = useRef(false) + let mounted = useIsMounted() + + let featuresRestoreFocus = Boolean(features & Features.RestoreFocus) + let featuresInitialFocus = Boolean(features & Features.InitialFocus) + + // Capture the currently focused element, before we enable the focus trap. + useEffect(() => { + if (!featuresRestoreFocus) return + + restoreElement.current = document.activeElement as HTMLElement + }, [featuresRestoreFocus]) + + // Restore the focus when we unmount the component. + useEffect(() => { + if (!featuresRestoreFocus) return + + return () => { + focusElement(restoreElement.current) + restoreElement.current = null + } + }, [featuresRestoreFocus]) // Handle initial focus useEffect(() => { - if (!enabled) return - if (containers.current.size !== 1) return - - mounted.current = true + if (!featuresInitialFocus) return + if (!container.current) return let activeElement = document.activeElement as HTMLElement - if (options.initialFocus?.current) { - if (options.initialFocus?.current === activeElement) { + if (initialFocus?.current) { + if (initialFocus?.current === activeElement) { + previousActiveElement.current = activeElement return // Initial focus ref is already the active element } - } else if (contains(containers.current, activeElement)) { + } else if (container.current.contains(activeElement)) { + previousActiveElement.current = activeElement return // Already focused within Dialog } - restoreElement.current = activeElement - // Try to focus the initialFocus ref - if (options.initialFocus?.current) { - focusElement(options.initialFocus.current) + if (initialFocus?.current) { + focusElement(initialFocus.current) } else { - let couldFocus = false - for (let container of containers.current) { - let result = focusIn(container, Focus.First) - if (result === FocusResult.Success) { - couldFocus = true - break - } + if (focusIn(container.current, Focus.First) === FocusResult.Error) { + throw new Error('There are no focusable elements inside the ') } - - if (!couldFocus) throw new Error('There are no focusable elements inside the ') } previousActiveElement.current = document.activeElement as HTMLElement + }, [container, initialFocus, featuresInitialFocus]) - return () => { - mounted.current = false - focusElement(restoreElement.current) - restoreElement.current = null - previousActiveElement.current = null - } - }, [enabled, containers, mounted, options.initialFocus]) - - // Handle Tab & Shift+Tab keyboard events + // Handle `Tab` & `Shift+Tab` keyboard events useWindowEvent('keydown', event => { - if (!enabled) return + if (!(features & Features.TabLock)) return + + if (!container.current) return if (event.key !== Keys.Tab) return - if (!document.activeElement) return - if (containers.current.size !== 1) return event.preventDefault() - for (let element of containers.current) { - let result = focusIn( - element, + if ( + focusIn( + container.current, (event.shiftKey ? Focus.Previous : Focus.Next) | Focus.WrapAround - ) - - if (result === FocusResult.Success) { - previousActiveElement.current = document.activeElement as HTMLElement - break - } + ) === FocusResult.Success + ) { + previousActiveElement.current = document.activeElement as HTMLElement } }) - // Prevent programmatically escaping + // Prevent programmatically escaping the container useWindowEvent( 'focus', event => { - if (!enabled) return - if (containers.current.size !== 1) return + if (!(features & Features.FocusLock)) return + + let allContainers = new Set(containers?.current) + allContainers.add(container) + + if (!allContainers.size) return let previous = previousActiveElement.current if (!previous) return @@ -102,7 +133,7 @@ export function useFocusTrap( let toElement = event.target as HTMLElement | null if (toElement && toElement instanceof HTMLElement) { - if (!contains(containers.current, toElement)) { + if (!contains(allContainers, toElement)) { event.preventDefault() event.stopPropagation() focusElement(previous) @@ -117,3 +148,11 @@ export function useFocusTrap( true ) } + +function contains(containers: Set>, element: HTMLElement) { + for (let container of containers) { + if (container.current?.contains(element)) return true + } + + return false +} diff --git a/packages/@headlessui-react/src/hooks/use-is-mounted.ts b/packages/@headlessui-react/src/hooks/use-is-mounted.ts index 66d6c0a..8f890a6 100644 --- a/packages/@headlessui-react/src/hooks/use-is-mounted.ts +++ b/packages/@headlessui-react/src/hooks/use-is-mounted.ts @@ -1,9 +1,11 @@ import { useRef, useEffect } from 'react' export function useIsMounted() { - let mounted = useRef(true) + let mounted = useRef(false) useEffect(() => { + mounted.current = true + return () => { mounted.current = false } diff --git a/packages/@headlessui-react/src/hooks/use-window-event.ts b/packages/@headlessui-react/src/hooks/use-window-event.ts index afbce92..f3dda15 100644 --- a/packages/@headlessui-react/src/hooks/use-window-event.ts +++ b/packages/@headlessui-react/src/hooks/use-window-event.ts @@ -1,12 +1,19 @@ -import { useEffect } from 'react' +import { useEffect, useRef } from 'react' export function useWindowEvent( type: TType, listener: (this: Window, ev: WindowEventMap[TType]) => any, options?: boolean | AddEventListenerOptions ) { + let listenerRef = useRef(listener) + listenerRef.current = listener + useEffect(() => { - window.addEventListener(type, listener, options) - return () => window.removeEventListener(type, listener, options) - }, [type, listener, options]) + function handler(event: WindowEventMap[TType]) { + listenerRef.current.call(window, event) + } + + window.addEventListener(type, handler, options) + return () => window.removeEventListener(type, handler, options) + }, [type, options]) } diff --git a/packages/@headlessui-react/src/internal/dom-containers.ts b/packages/@headlessui-react/src/internal/dom-containers.ts deleted file mode 100644 index caa0c72..0000000 --- a/packages/@headlessui-react/src/internal/dom-containers.ts +++ /dev/null @@ -1,7 +0,0 @@ -export function contains(containers: Set, element: HTMLElement) { - for (let container of containers) { - if (container.contains(element)) return true - } - - return false -} diff --git a/packages/@headlessui-react/src/internal/stack-context.tsx b/packages/@headlessui-react/src/internal/stack-context.tsx index 8501120..525470f 100644 --- a/packages/@headlessui-react/src/internal/stack-context.tsx +++ b/packages/@headlessui-react/src/internal/stack-context.tsx @@ -1,37 +1,42 @@ -import React, { ReactNode, createContext, useContext, useCallback } from 'react' +import React, { + createContext, + useCallback, + useContext, + + // Types + MutableRefObject, + ReactNode, +} from 'react' import { useIsoMorphicEffect } from '../hooks/use-iso-morphic-effect' -type OnUpdate = (message: StackMessage, element: HTMLElement) => void +type OnUpdate = ( + message: StackMessage, + type: string, + element: MutableRefObject +) => void let StackContext = createContext(() => {}) StackContext.displayName = 'StackContext' export enum StackMessage { - AddElement, - RemoveElement, + Add, + Remove, } export function useStackContext() { return useContext(StackContext) } -export function useElementStack(element: HTMLElement | null) { - let notify = useStackContext() - - useIsoMorphicEffect(() => { - if (!element) return - - notify(StackMessage.AddElement, element) - return () => notify(StackMessage.RemoveElement, element) - }, [element]) -} - export function StackProvider({ children, onUpdate, + type, + element, }: { children: ReactNode onUpdate?: OnUpdate + type: string + element: MutableRefObject }) { let parentUpdate = useStackContext() @@ -46,5 +51,10 @@ export function StackProvider({ [parentUpdate, onUpdate] ) + useIsoMorphicEffect(() => { + notify(StackMessage.Add, type, element) + return () => notify(StackMessage.Remove, type, element) + }, [notify, type, element]) + return {children} } diff --git a/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts b/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts index 1d34c57..828d460 100644 --- a/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts +++ b/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts @@ -880,6 +880,10 @@ export function getDialogOverlay(): HTMLElement | null { return document.querySelector('[id^="headlessui-dialog-overlay-"]') } +export function getDialogOverlays(): HTMLElement[] { + return Array.from(document.querySelectorAll('[id^="headlessui-dialog-overlay-"]')) +} + // --- export enum DialogState { diff --git a/packages/@headlessui-react/src/utils/focus-management.ts b/packages/@headlessui-react/src/utils/focus-management.ts index f873373..b3d1104 100644 --- a/packages/@headlessui-react/src/utils/focus-management.ts +++ b/packages/@headlessui-react/src/utils/focus-management.ts @@ -44,9 +44,16 @@ export enum Focus { } export enum FocusResult { + /** Something went wrong while trying to focus. */ Error, + + /** When `Focus.WrapAround` is enabled, going from position `N` to `N+1` where `N` is the last index in the array, then we overflow. */ Overflow, + + /** Focus was successful. */ Success, + + /** When `Focus.WrapAround` is enabled, going from position `N` to `N-1` where `N` is the first index in the array, then we underflow. */ Underflow, } diff --git a/packages/@headlessui-vue/src/components/dialog/dialog.test.ts b/packages/@headlessui-vue/src/components/dialog/dialog.test.ts index cdfafec..e01d98e 100644 --- a/packages/@headlessui-vue/src/components/dialog/dialog.test.ts +++ b/packages/@headlessui-vue/src/components/dialog/dialog.test.ts @@ -15,6 +15,7 @@ import { getByText, assertActiveElement, getDialogs, + getDialogOverlays, } from '../../test-utils/accessibility-assertions' import { click, press, Keys } from '../../test-utils/interactions' import { html } from '../../test-utils/html' @@ -781,102 +782,243 @@ describe('Mouse interactions', () => { }) describe('Nesting', () => { - it('should be possible to open nested Dialog components and close them with `Escape`', async () => { - let Nested = defineComponent({ - components: { Dialog }, - emits: ['close'], - props: ['level'], - render() { - let level = this.$props.level ?? 1 - return h(Dialog, { open: true, onClose: this.onClose }, () => [ - h('div', [ - h('p', `Level: ${level}`), - h( - 'button', - { - onClick: () => { - this.showChild = true - }, + let Nested = defineComponent({ + components: { Dialog, DialogOverlay }, + emits: ['close'], + props: ['level'], + render() { + let level = this.$props.level ?? 1 + return h(Dialog, { open: true, onClose: this.onClose }, () => [ + h(DialogOverlay), + h('div', [ + h('p', `Level: ${level}`), + h( + 'button', + { + onClick: () => { + this.showChild = true }, - `Open ${level + 1}` - ), - ]), - this.showChild && - h(Nested, { - onClose: () => { - this.showChild = false + }, + `Open ${level + 1} a` + ), + h( + 'button', + { + onClick: () => { + this.showChild = true }, - level: level + 1, - }), - ]) - }, - setup(_props, { emit }) { - let showChild = ref(false) + }, + `Open ${level + 1} b` + ), + h( + 'button', + { + onClick: () => { + this.showChild = true + }, + }, + `Open ${level + 1} c` + ), + ]), + this.showChild && + h(Nested, { + onClose: () => { + this.showChild = false + }, + level: level + 1, + }), + ]) + }, + setup(_props, { emit }) { + let showChild = ref(false) - return { - showChild, - onClose() { - emit('close', false) - }, - } - }, - }) - - renderTemplate({ - components: { Nested }, - template: ` - - - `, - setup() { - let isOpen = ref(false) - return { isOpen } - }, - }) - - // Verify we have no open dialogs - expect(getDialogs()).toHaveLength(0) - - // Open Dialog 1 - await click(getByText('Open 1')) - - // Verify that we have 1 open dialog - expect(getDialogs()).toHaveLength(1) - - // Open Dialog 2 - await click(getByText('Open 2')) - - // Verify that we have 2 open dialogs - expect(getDialogs()).toHaveLength(2) - - // Press escape to close the top most Dialog - await press(Keys.Escape) - - // Verify that we have 1 open dialog - expect(getDialogs()).toHaveLength(1) - - // Open Dialog 2 - await click(getByText('Open 2')) - - // Verify that we have 2 open dialogs - expect(getDialogs()).toHaveLength(2) - - // Open Dialog 3 - await click(getByText('Open 3')) - - // Verify that we have 3 open dialogs - expect(getDialogs()).toHaveLength(3) - - // Press escape to close the top most Dialog - await press(Keys.Escape) - - // Verify that we have 2 open dialogs - expect(getDialogs()).toHaveLength(2) - - // Press escape to close the top most Dialog - await press(Keys.Escape) - - // Verify that we have 1 open dialog - expect(getDialogs()).toHaveLength(1) + return { + showChild, + onClose() { + emit('close', false) + }, + } + }, }) + + it.each` + strategy | action + ${'with `Escape`'} | ${() => press(Keys.Escape)} + ${'with `Outside Click`'} | ${() => click(document.body)} + ${'with `Click on Dialog.Overlay`'} | ${() => click(getDialogOverlays().pop()!)} + `( + 'should be possible to open nested Dialog components and close them $strategy', + async ({ action }) => { + renderTemplate({ + components: { Nested }, + template: ` + + + `, + setup() { + let isOpen = ref(false) + return { isOpen } + }, + }) + + // Verify we have no open dialogs + expect(getDialogs()).toHaveLength(0) + + // Open Dialog 1 + await click(getByText('Open 1')) + + // Verify that we have 1 open dialog + expect(getDialogs()).toHaveLength(1) + + // Verify that the `Open 2 a` has focus + assertActiveElement(getByText('Open 2 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 a')) + + // Open Dialog 2 via the second button + await click(getByText('Open 2 b')) + + // Verify that we have 2 open dialogs + expect(getDialogs()).toHaveLength(2) + + // Verify that the `Open 3 a` has focus + assertActiveElement(getByText('Open 3 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 a')) + + // Close the top most Dialog + await action() + + // Verify that we have 1 open dialog + expect(getDialogs()).toHaveLength(1) + + // Verify that the `Open 2 b` button got focused again + assertActiveElement(getByText('Open 2 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 b')) + + // Open Dialog 2 via button b + await click(getByText('Open 2 b')) + + // Verify that the `Open 3 a` has focus + assertActiveElement(getByText('Open 3 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 a')) + + // Verify that we have 2 open dialogs + expect(getDialogs()).toHaveLength(2) + + // Open Dialog 3 via button c + await click(getByText('Open 3 c')) + + // Verify that the `Open 4 a` has focus + assertActiveElement(getByText('Open 4 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 4 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 4 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 4 a')) + + // Verify that we have 3 open dialogs + expect(getDialogs()).toHaveLength(3) + + // Close the top most Dialog + await action() + + // Verify that the `Open 3 c` button got focused again + assertActiveElement(getByText('Open 3 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 3 c')) + + // Verify that we have 2 open dialogs + expect(getDialogs()).toHaveLength(2) + + // Close the top most Dialog + await action() + + // Verify that we have 1 open dialog + expect(getDialogs()).toHaveLength(1) + + // Verify that the `Open 2 b` button got focused again + assertActiveElement(getByText('Open 2 b')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 c')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 a')) + + // Verify that we can tab around + await press(Keys.Tab) + assertActiveElement(getByText('Open 2 b')) + + // Close the top most Dialog + await action() + + // Verify that we have 0 open dialogs + expect(getDialogs()).toHaveLength(0) + + // Verify that the `Open 1` button got focused again + assertActiveElement(getByText('Open 1')) + } + ) }) diff --git a/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts b/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts index 1d34c57..828d460 100644 --- a/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts +++ b/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts @@ -880,6 +880,10 @@ export function getDialogOverlay(): HTMLElement | null { return document.querySelector('[id^="headlessui-dialog-overlay-"]') } +export function getDialogOverlays(): HTMLElement[] { + return Array.from(document.querySelectorAll('[id^="headlessui-dialog-overlay-"]')) +} + // --- export enum DialogState {