From 34a10538d6aa9e04d2d6e1fa05aa613c4248c019 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 3 May 2021 13:11:19 +0200 Subject: [PATCH 1/4] Open closed state (#466) * simplify examples by using the implicit open/closed state * introduce Open/Closed context (React) * use Open/Closed context in Dialog component (React) * use Open/Closed context in Disclosure component (React) * use Open/Closed context in Listbox component (React) * use Open/Closed context in Menu component (React) * use Open/Closed context in Popover component (React) * use Open/Closed context in Transition component (React) * introduce Open/Closed context (Vue) * use Open/Closed context in Dialog component (Vue) * use Open/Closed context in Disclosure component (Vue) * use Open/Closed context in Listbox component (Vue) * use Open/Closed context in Menu component (Vue) * use Open/Closed context in Popover component (Vue) * use Open/Closed context in Transition component (Vue) * use a ref in the Description comopnent This allows us to update the ref and everything should work after that. Currently we only saw the "current" state. * add more Vue examples * update changelog --- CHANGELOG.md | 8 +- .../@headlessui-react/pages/dialog/dialog.tsx | 162 ++++++++---------- .../pages/disclosure/disclosure.tsx | 29 ++-- .../menu/menu-with-transition-and-popper.tsx | 122 ++++++------- .../pages/menu/menu-with-transition.tsx | 104 ++++++----- .../pages/popover/popover.tsx | 52 +++--- .../src/components/dialog/dialog.test.tsx | 40 ++++- .../src/components/dialog/dialog.tsx | 24 ++- .../components/disclosure/disclosure.test.tsx | 70 +++++++- .../src/components/disclosure/disclosure.tsx | 31 +++- .../src/components/listbox/listbox.test.tsx | 69 +++++++- .../src/components/listbox/listbox.tsx | 31 +++- .../src/components/menu/menu.test.tsx | 69 +++++++- .../src/components/menu/menu.tsx | 21 ++- .../src/components/popover/popover.test.tsx | 64 ++++++- .../src/components/popover/popover.tsx | 42 ++++- .../src/components/transitions/transition.tsx | 37 ++-- .../src/internal/open-closed.tsx | 29 ++++ .../examples/src/components/dialog/dialog.vue | 16 +- .../src/components/disclosure/disclosure.vue | 33 ++++ .../@headlessui-vue/examples/src/routes.json | 11 ++ .../src/components/description/description.ts | 11 +- .../src/components/dialog/dialog.test.ts | 47 +++++ .../src/components/dialog/dialog.ts | 36 +++- .../components/disclosure/disclosure.test.ts | 122 ++++++++++++- .../src/components/disclosure/disclosure.ts | 22 ++- .../src/components/listbox/listbox.test.tsx | 121 +++++++++++++ .../src/components/listbox/listbox.ts | 23 ++- .../src/components/menu/menu.test.tsx | 121 ++++++++++++- .../src/components/menu/menu.ts | 23 ++- .../src/components/popover/popover.test.ts | 121 ++++++++++++- .../src/components/popover/popover.ts | 34 +++- .../src/components/transitions/transition.ts | 33 +++- .../src/internal/open-closed.ts | 23 +++ 34 files changed, 1460 insertions(+), 341 deletions(-) create mode 100644 packages/@headlessui-react/src/internal/open-closed.tsx create mode 100644 packages/@headlessui-vue/examples/src/components/disclosure/disclosure.vue create mode 100644 packages/@headlessui-vue/src/internal/open-closed.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b53949c..0515f31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased - React] -- Nothing yet! +### Added + +- Introduce Open/Closed state, to simplify component communication ([#466](https://github.com/tailwindlabs/headlessui/pull/466)) ## [Unreleased - Vue] -- Nothing yet! +### Added + +- Introduce Open/Closed state, to simplify component communication ([#466](https://github.com/tailwindlabs/headlessui/pull/466)) ## [@headlessui/react@v1.1.1] - 2021-04-28 diff --git a/packages/@headlessui-react/pages/dialog/dialog.tsx b/packages/@headlessui-react/pages/dialog/dialog.tsx index bf1b255..1717428 100644 --- a/packages/@headlessui-react/pages/dialog/dialog.tsx +++ b/packages/@headlessui-react/pages/dialog/dialog.tsx @@ -20,7 +20,7 @@ function Nested({ onClose, level = 0 }) { return ( <> - {true && } +
setNested(true)}>Show nested {nested && setNested(false)} />} - - + console.log('done')}> +
- {({ open }) => ( - <> - - - Choose a reason - - - - - - - + + Choose a reason + - - + + + + + + + +
+

Signed in as

+

+ tom@example.com +

+
+ +
+ -
-

Signed in as

-

- tom@example.com -

-
+ Account settings + + + Support + + + New feature (soon) + + + License + +
-
- - Account settings - - - Support - - - New feature (soon) - - - License - -
- -
- - Sign out - -
- - - - - )} +
+ + Sign out + +
+ + +
diff --git a/packages/@headlessui-react/pages/disclosure/disclosure.tsx b/packages/@headlessui-react/pages/disclosure/disclosure.tsx index 8fa9335..856beb2 100644 --- a/packages/@headlessui-react/pages/disclosure/disclosure.tsx +++ b/packages/@headlessui-react/pages/disclosure/disclosure.tsx @@ -6,25 +6,18 @@ export default function Home() {
- {({ open }) => ( - <> - Trigger + Trigger - - - Content - - - - )} + + Content +
diff --git a/packages/@headlessui-react/pages/menu/menu-with-transition-and-popper.tsx b/packages/@headlessui-react/pages/menu/menu-with-transition-and-popper.tsx index 2456ada..d78bb5e 100644 --- a/packages/@headlessui-react/pages/menu/menu-with-transition-and-popper.tsx +++ b/packages/@headlessui-react/pages/menu/menu-with-transition-and-popper.tsx @@ -26,73 +26,65 @@ export default function Home() {
- {({ open }) => ( - <> - - - Options - - - - - + + + Options + + + + + -
- - -
-

Signed in as

-

- tom@example.com -

-
+
+ + +
+

Signed in as

+

+ tom@example.com +

+
-
- - Account settings - - - {data => ( - - Support - - )} - - - New feature (soon) - - - License - -
-
- - Sign out - -
- - -
- - )} +
+ + Account settings + + + {data => ( + + Support + + )} + + + New feature (soon) + + + License + +
+
+ + Sign out + +
+ + +
diff --git a/packages/@headlessui-react/pages/menu/menu-with-transition.tsx b/packages/@headlessui-react/pages/menu/menu-with-transition.tsx index 70ec606..3cc32bb 100644 --- a/packages/@headlessui-react/pages/menu/menu-with-transition.tsx +++ b/packages/@headlessui-react/pages/menu/menu-with-transition.tsx @@ -18,65 +18,57 @@ export default function Home() {
- {({ open }) => ( - <> - - - Options - - - - - + + + Options + + + + + - - -
-

Signed in as

-

- tom@example.com -

-
+ + +
+

Signed in as

+

+ tom@example.com +

+
-
- - Account settings - - - Support - - - New feature (soon) - - - License - -
+
+ + Account settings + + + Support + + + New feature (soon) + + + License + +
-
- - Sign out - -
- - - - )} +
+ + Sign out + +
+ +
diff --git a/packages/@headlessui-react/pages/popover/popover.tsx b/packages/@headlessui-react/pages/popover/popover.tsx index 613c2e0..02bd34a 100644 --- a/packages/@headlessui-react/pages/popover/popover.tsx +++ b/packages/@headlessui-react/pages/popover/popover.tsx @@ -41,38 +41,30 @@ export default function Home() {
- + - {({ open }) => ( - <> - - - + + + - - Normal - - - {links.map((link, i) => ( - - Normal - {link} - - ))} - - - )} + + Normal + + + {links.map((link, i) => ( + + Normal - {link} + + ))} + diff --git a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx index a2178e7..e49273a 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx @@ -17,6 +17,7 @@ import { } from '../../test-utils/accessibility-assertions' import { click, press, Keys } from '../../test-utils/interactions' import { PropsOf } from '../../types' +import { Transition } from '../transitions/transition' jest.mock('../../hooks/use-id') @@ -95,7 +96,6 @@ describe('Rendering', () => { 'should complain when an `onClose` prop is provided without an `open` prop', suppressConsoleLogs(async () => { expect(() => - // @ts-expect-error render( {}} />) ).toThrowErrorMatchingInlineSnapshot( `"You provided an \`onClose\` prop to the \`Dialog\`, but forgot an \`open\` prop."` @@ -352,6 +352,44 @@ describe('Rendering', () => { }) }) +describe('Composition', () => { + it( + 'should be possible to open the Dialog via a Transition component', + suppressConsoleLogs(async () => { + render( + + + {JSON.stringify} + + + + ) + + assertDialog({ state: DialogState.Visible }) + assertDialogDescription({ + state: DialogState.Visible, + textContent: JSON.stringify({ open: true }), + }) + }) + ) + + it( + 'should be possible to close the Dialog via a Transition component', + suppressConsoleLogs(async () => { + render( + + + {JSON.stringify} + + + + ) + + assertDialog({ state: DialogState.InvisibleUnmounted }) + }) + ) +}) + describe('Keyboard interactions', () => { describe('`Escape` key', () => { it( diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx index d898d62..e39530f 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.tsx @@ -32,6 +32,7 @@ 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' enum DialogStates { Open, @@ -108,7 +109,7 @@ let DialogRoot = forwardRefWithAs(function Dialog< >( props: Props & PropsForFeatures & { - open: boolean + open?: boolean onClose(value: boolean): void initialFocus?: MutableRefObject }, @@ -116,12 +117,21 @@ let DialogRoot = forwardRefWithAs(function Dialog< ) { let { open, onClose, initialFocus, ...rest } = props + let usesOpenClosedState = useOpenClosed() + if (open === undefined && usesOpenClosedState !== null) { + // Update the `open` prop based on the open closed state + open = match(usesOpenClosedState, { + [State.Open]: true, + [State.Closed]: false, + }) + } + let containers = useRef>(new Set()) let internalDialogRef = useRef(null) let dialogRef = useSyncRefs(internalDialogRef, ref) // Validations - let hasOpen = props.hasOwnProperty('open') + let hasOpen = props.hasOwnProperty('open') || usesOpenClosedState !== null let hasOnClose = props.hasOwnProperty('onClose') if (!hasOpen && !hasOnClose) { throw new Error( @@ -152,8 +162,14 @@ let DialogRoot = forwardRefWithAs(function Dialog< `You provided an \`onClose\` prop to the \`Dialog\`, but the value is not a function. Received: ${onClose}` ) } - let dialogState = open ? DialogStates.Open : DialogStates.Closed + let visible = (() => { + if (usesOpenClosedState !== null) { + return usesOpenClosedState === State.Open + } + + return dialogState === DialogStates.Open + })() let [state, dispatch] = useReducer(stateReducer, { titleId: null, @@ -283,7 +299,7 @@ let DialogRoot = forwardRefWithAs(function Dialog< slot, defaultTag: DEFAULT_DIALOG_TAG, features: DialogRenderFeatures, - visible: dialogState === DialogStates.Open, + visible, name: 'Dialog', })} diff --git a/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx b/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx index aea9edd..06f432b 100644 --- a/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx +++ b/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx @@ -1,4 +1,4 @@ -import React, { createElement } from 'react' +import React, { createElement, useEffect } from 'react' import { render } from '@testing-library/react' import { Disclosure } from './disclosure' @@ -11,11 +11,22 @@ import { getDisclosurePanel, } from '../../test-utils/accessibility-assertions' import { click, press, Keys, MouseButton } from '../../test-utils/interactions' +import { Transition } from '../transitions/transition' jest.mock('../../hooks/use-id') afterAll(() => jest.restoreAllMocks()) +function nextFrame() { + return new Promise(resolve => { + requestAnimationFrame(() => { + requestAnimationFrame(() => { + resolve() + }) + }) + }) +} + describe('Safe guards', () => { it.each([ ['Disclosure.Button', Disclosure.Button], @@ -232,6 +243,63 @@ describe('Rendering', () => { }) }) +describe('Composition', () => { + function Debug({ fn, name }: { fn: (text: string) => void; name: string }) { + useEffect(() => { + fn(`Mounting - ${name}`) + return () => { + fn(`Unmounting - ${name}`) + } + }, [fn, name]) + return null + } + + it( + 'should be possible to control the Disclosure.Panel by wrapping it in a Transition component', + suppressConsoleLogs(async () => { + let orderFn = jest.fn() + render( + + Trigger + + + + + + + + + + + ) + + // Verify the Disclosure is hidden + assertDisclosurePanel({ state: DisclosureState.InvisibleUnmounted }) + + // Open the Disclosure component + await click(getDisclosureButton()) + + // Verify the Disclosure is visible + assertDisclosurePanel({ state: DisclosureState.Visible }) + + // Unmount the full tree + await click(getDisclosureButton()) + + // Wait for all transitions to finish + await nextFrame() + + // Verify that we tracked the `mounts` and `unmounts` in the correct order + expect(orderFn.mock.calls).toEqual([ + ['Mounting - Disclosure'], + ['Mounting - Transition'], + ['Mounting - Transition.Child'], + ['Unmounting - Transition.Child'], + ['Unmounting - Transition'], + ]) + }) + ) +}) + describe('Keyboard interactions', () => { describe('`Enter` key', () => { it( diff --git a/packages/@headlessui-react/src/components/disclosure/disclosure.tsx b/packages/@headlessui-react/src/components/disclosure/disclosure.tsx index f633b78..c578f10 100644 --- a/packages/@headlessui-react/src/components/disclosure/disclosure.tsx +++ b/packages/@headlessui-react/src/components/disclosure/disclosure.tsx @@ -23,6 +23,7 @@ import { useSyncRefs } from '../../hooks/use-sync-refs' import { useId } from '../../hooks/use-id' import { Keys } from '../keyboard' import { isDisabledReactIssue7711 } from '../../utils/bugs' +import { OpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' enum DisclosureStates { Open, @@ -137,12 +138,19 @@ export function Disclosure - {render({ - props: passthroughProps, - slot, - defaultTag: DEFAULT_DISCLOSURE_TAG, - name: 'Disclosure', - })} + + {render({ + props: passthroughProps, + slot, + defaultTag: DEFAULT_DISCLOSURE_TAG, + name: 'Disclosure', + })} + ) } @@ -248,6 +256,15 @@ let Panel = forwardRefWithAs(function Panel { + if (usesOpenClosedState !== null) { + return usesOpenClosedState === State.Open + } + + return state.disclosureState === DisclosureStates.Open + })() + // Unlink on "unmount" myself useEffect(() => () => dispatch({ type: ActionTypes.UnlinkPanel }), [dispatch]) @@ -273,7 +290,7 @@ let Panel = forwardRefWithAs(function Panel { ) }) +describe('Composition', () => { + function Debug({ fn, name }: { fn: (text: string) => void; name: string }) { + useEffect(() => { + fn(`Mounting - ${name}`) + return () => { + fn(`Unmounting - ${name}`) + } + }, [fn, name]) + return null + } + + it( + 'should be possible to wrap the Listbox.Options with a Transition component', + suppressConsoleLogs(async () => { + let orderFn = jest.fn() + render( + + Trigger + + + + + + {data => ( + <> + {JSON.stringify(data)} + + + )} + + + + + ) + + assertListboxButton({ + state: ListboxState.InvisibleUnmounted, + attributes: { id: 'headlessui-listbox-button-1' }, + }) + assertListbox({ state: ListboxState.InvisibleUnmounted }) + + await click(getListboxButton()) + + assertListboxButton({ + state: ListboxState.Visible, + attributes: { id: 'headlessui-listbox-button-1' }, + }) + assertListbox({ + state: ListboxState.Visible, + textContent: JSON.stringify({ active: false, selected: false, disabled: false }), + }) + + await click(getListboxButton()) + + // Verify that we tracked the `mounts` and `unmounts` in the correct order + expect(orderFn.mock.calls).toEqual([ + ['Mounting - Listbox'], + ['Mounting - Transition'], + ['Mounting - Listbox.Option'], + ['Unmounting - Transition'], + ['Unmounting - Listbox.Option'], + ]) + }) + ) +}) + describe('Keyboard interactions', () => { describe('`Enter` key', () => { it( diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index a4e9fc6..1d2ad6c 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -31,6 +31,7 @@ import { Focus, calculateActiveIndex } from '../../utils/calculate-active-index' import { isDisabledReactIssue7711 } from '../../utils/bugs' import { isFocusableElement, FocusableMode } from '../../utils/focus-management' import { useWindowEvent } from '../../hooks/use-window-event' +import { useOpenClosed, State, OpenClosedProvider } from '../../internal/open-closed' enum ListboxStates { Open, @@ -240,12 +241,19 @@ export function Listbox - {render({ - props: passThroughProps, - slot, - defaultTag: DEFAULT_LISTBOX_TAG, - name: 'Listbox', - })} + + {render({ + props: passThroughProps, + slot, + defaultTag: DEFAULT_LISTBOX_TAG, + name: 'Listbox', + })} + ) } @@ -426,6 +434,15 @@ let Options = forwardRefWithAs(function Options< let d = useDisposables() let searchDisposables = useDisposables() + let usesOpenClosedState = useOpenClosed() + let visible = (() => { + if (usesOpenClosedState !== null) { + return usesOpenClosedState === State.Open + } + + return state.listboxState === ListboxStates.Open + })() + useIsoMorphicEffect(() => { let container = state.optionsRef.current if (!container) return @@ -531,7 +548,7 @@ let Options = forwardRefWithAs(function Options< slot, defaultTag: DEFAULT_OPTIONS_TAG, features: OptionsRenderFeatures, - visible: state.listboxState === ListboxStates.Open, + visible, name: 'Listbox.Options', }) }) diff --git a/packages/@headlessui-react/src/components/menu/menu.test.tsx b/packages/@headlessui-react/src/components/menu/menu.test.tsx index f822874..45697cf 100644 --- a/packages/@headlessui-react/src/components/menu/menu.test.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.test.tsx @@ -1,4 +1,4 @@ -import React, { createElement } from 'react' +import React, { createElement, useEffect } from 'react' import { render } from '@testing-library/react' import { Menu } from './menu' @@ -31,6 +31,7 @@ import { Keys, MouseButton, } from '../../test-utils/interactions' +import { Transition } from '../transitions/transition' jest.mock('../../hooks/use-id') @@ -431,6 +432,72 @@ describe('Rendering composition', () => { ) }) +describe('Composition', () => { + function Debug({ fn, name }: { fn: (text: string) => void; name: string }) { + useEffect(() => { + fn(`Mounting - ${name}`) + return () => { + fn(`Unmounting - ${name}`) + } + }, [fn, name]) + return null + } + + it( + 'should be possible to wrap the Menu.Items with a Transition component', + suppressConsoleLogs(async () => { + let orderFn = jest.fn() + render( + + Trigger + + + + + + {data => ( + <> + {JSON.stringify(data)} + + + )} + + + + + ) + + assertMenuButton({ + state: MenuState.InvisibleUnmounted, + attributes: { id: 'headlessui-menu-button-1' }, + }) + assertMenu({ state: MenuState.InvisibleUnmounted }) + + await click(getMenuButton()) + + assertMenuButton({ + state: MenuState.Visible, + attributes: { id: 'headlessui-menu-button-1' }, + }) + assertMenu({ + state: MenuState.Visible, + textContent: JSON.stringify({ active: false, disabled: false }), + }) + + await click(getMenuButton()) + + // Verify that we tracked the `mounts` and `unmounts` in the correct order + expect(orderFn.mock.calls).toEqual([ + ['Mounting - Menu'], + ['Mounting - Transition'], + ['Mounting - Menu.Item'], + ['Unmounting - Transition'], + ['Unmounting - Menu.Item'], + ]) + }) + ) +}) + describe('Keyboard interactions', () => { describe('`Enter` key', () => { it( diff --git a/packages/@headlessui-react/src/components/menu/menu.tsx b/packages/@headlessui-react/src/components/menu/menu.tsx index d6a09e1..5440e99 100644 --- a/packages/@headlessui-react/src/components/menu/menu.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.tsx @@ -33,6 +33,7 @@ import { isDisabledReactIssue7711 } from '../../utils/bugs' import { isFocusableElement, FocusableMode } from '../../utils/focus-management' import { useWindowEvent } from '../../hooks/use-window-event' import { useTreeWalker } from '../../hooks/use-tree-walker' +import { useOpenClosed, State, OpenClosedProvider } from '../../internal/open-closed' enum MenuStates { Open, @@ -197,7 +198,14 @@ export function Menu( return ( - {render({ props, slot, defaultTag: DEFAULT_MENU_TAG, name: 'Menu' })} + + {render({ props, slot, defaultTag: DEFAULT_MENU_TAG, name: 'Menu' })} + ) } @@ -330,6 +338,15 @@ let Items = forwardRefWithAs(function Items { + if (usesOpenClosedState !== null) { + return usesOpenClosedState === State.Open + } + + return state.menuState === MenuStates.Open + })() + useEffect(() => { let container = state.itemsRef.current if (!container) return @@ -455,7 +472,7 @@ let Items = forwardRefWithAs(function Items jest.restoreAllMocks()) +function nextFrame() { + return new Promise(resolve => { + requestAnimationFrame(() => { + requestAnimationFrame(() => { + resolve() + }) + }) + }) +} + describe('Safe guards', () => { it.each([ ['Popover.Button', Popover.Button], @@ -376,6 +387,57 @@ describe('Rendering', () => { }) }) +describe('Composition', () => { + function Debug({ fn, name }: { fn: (text: string) => void; name: string }) { + useEffect(() => { + fn(`Mounting - ${name}`) + return () => { + fn(`Unmounting - ${name}`) + } + }, [fn, name]) + return null + } + + it( + 'should be possible to wrap the Popover.Panel with a Transition component', + suppressConsoleLogs(async () => { + let orderFn = jest.fn() + render( + + Trigger + + + + + + + + + + + ) + + // Open the popover + await click(getPopoverButton()) + + // Close the popover + await click(getPopoverButton()) + + // Wait for all transitions to finish + await nextFrame() + + // Verify that we tracked the `mounts` and `unmounts` in the correct order + expect(orderFn.mock.calls).toEqual([ + ['Mounting - Popover'], + ['Mounting - Transition'], + ['Mounting - Transition.Child'], + ['Unmounting - Transition.Child'], + ['Unmounting - Transition'], + ]) + }) + ) +}) + describe('Keyboard interactions', () => { describe('`Enter` key', () => { it( diff --git a/packages/@headlessui-react/src/components/popover/popover.tsx b/packages/@headlessui-react/src/components/popover/popover.tsx index 45504a4..beebc05 100644 --- a/packages/@headlessui-react/src/components/popover/popover.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.tsx @@ -33,6 +33,7 @@ import { FocusableMode, } from '../../utils/focus-management' import { useWindowEvent } from '../../hooks/use-window-event' +import { OpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' enum PopoverStates { Open, @@ -220,12 +221,19 @@ export function Popover( return ( - {render({ - props, - slot, - defaultTag: DEFAULT_POPOVER_TAG, - name: 'Popover', - })} + + {render({ + props, + slot, + defaultTag: DEFAULT_POPOVER_TAG, + name: 'Popover', + })} + ) } @@ -469,6 +477,15 @@ let Overlay = forwardRefWithAs(function Overlay< let id = `headlessui-popover-overlay-${useId()}` + let usesOpenClosedState = useOpenClosed() + let visible = (() => { + if (usesOpenClosedState !== null) { + return usesOpenClosedState === State.Open + } + + return popoverState === PopoverStates.Open + })() + let handleClick = useCallback( (event: ReactMouseEvent) => { if (isDisabledReactIssue7711(event.currentTarget)) return event.preventDefault() @@ -493,7 +510,7 @@ let Overlay = forwardRefWithAs(function Overlay< slot, defaultTag: DEFAULT_OVERLAY_TAG, features: OverlayRenderFeatures, - visible: popoverState === PopoverStates.Open, + visible, name: 'Popover.Overlay', }) }) @@ -521,6 +538,15 @@ let Panel = forwardRefWithAs(function Panel { + if (usesOpenClosedState !== null) { + return usesOpenClosedState === State.Open + } + + return state.popoverState === PopoverStates.Open + })() + let handleKeyDown = useCallback( (event: KeyboardEvent) => { switch (event.key) { @@ -630,7 +656,7 @@ let Panel = forwardRefWithAs(function Panel diff --git a/packages/@headlessui-react/src/components/transitions/transition.tsx b/packages/@headlessui-react/src/components/transitions/transition.tsx index 9e5310b..598e428 100644 --- a/packages/@headlessui-react/src/components/transitions/transition.tsx +++ b/packages/@headlessui-react/src/components/transitions/transition.tsx @@ -22,6 +22,7 @@ import { useIsoMorphicEffect } from '../../hooks/use-iso-morphic-effect' import { Features, PropsForFeatures, render, RenderStrategy } from '../../utils/render' import { Reason, transition } from './utils/transition' +import { OpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' type ID = ReturnType @@ -318,24 +319,40 @@ function TransitionChild - {render({ - props: { ...passthroughProps, ...propsWeControl }, - defaultTag: DEFAULT_TRANSITION_CHILD_TAG, - features: TransitionChildRenderFeatures, - visible: state === TreeStates.Visible, - name: 'Transition.Child', - })} + + {render({ + props: { ...passthroughProps, ...propsWeControl }, + defaultTag: DEFAULT_TRANSITION_CHILD_TAG, + features: TransitionChildRenderFeatures, + visible: state === TreeStates.Visible, + name: 'Transition.Child', + })} + ) } export function Transition( - props: TransitionChildProps & { show: boolean; appear?: boolean } + props: TransitionChildProps & { show?: boolean; appear?: boolean } ) { // @ts-expect-error let { show, appear = false, unmount, ...passthroughProps } = props as typeof props - if (![true, false].includes(show)) { + let usesOpenClosedState = useOpenClosed() + + if (show === undefined && usesOpenClosedState !== null) { + show = match(usesOpenClosedState, { + [State.Open]: true, + [State.Closed]: false, + }) + } + + if (![true, false].includes((show as unknown) as boolean)) { throw new Error('A is used but it is missing a `show={true | false}` prop.') } @@ -347,7 +364,7 @@ export function Transition( - () => ({ show, appear: appear || !initial }), + () => ({ show: show as boolean, appear: appear || !initial }), [show, appear, initial] ) diff --git a/packages/@headlessui-react/src/internal/open-closed.tsx b/packages/@headlessui-react/src/internal/open-closed.tsx new file mode 100644 index 0000000..143dc26 --- /dev/null +++ b/packages/@headlessui-react/src/internal/open-closed.tsx @@ -0,0 +1,29 @@ +import React, { + createContext, + useContext, + + // Types + ReactNode, + ReactElement, +} from 'react' + +let Context = createContext(null) +Context.displayName = 'OpenClosedContext' + +export enum State { + Open, + Closed, +} + +export function useOpenClosed() { + return useContext(Context) +} + +interface Props { + value: State + children: ReactNode +} + +export function OpenClosedProvider({ value, children }: Props): ReactElement { + return {children} +} diff --git a/packages/@headlessui-vue/examples/src/components/dialog/dialog.vue b/packages/@headlessui-vue/examples/src/components/dialog/dialog.vue index 4a215ce..4b744c5 100644 --- a/packages/@headlessui-vue/examples/src/components/dialog/dialog.vue +++ b/packages/@headlessui-vue/examples/src/components/dialog/dialog.vue @@ -8,7 +8,7 @@ - +
- + - + + @@ -135,6 +142,7 @@
+
diff --git a/packages/@headlessui-vue/examples/src/components/disclosure/disclosure.vue b/packages/@headlessui-vue/examples/src/components/disclosure/disclosure.vue new file mode 100644 index 0000000..1fb1b55 --- /dev/null +++ b/packages/@headlessui-vue/examples/src/components/disclosure/disclosure.vue @@ -0,0 +1,33 @@ + + + diff --git a/packages/@headlessui-vue/examples/src/routes.json b/packages/@headlessui-vue/examples/src/routes.json index ec649f2..1d76960 100644 --- a/packages/@headlessui-vue/examples/src/routes.json +++ b/packages/@headlessui-vue/examples/src/routes.json @@ -94,6 +94,17 @@ } ] }, + { + "name": "Disclosure", + "path": "/disclosure", + "children": [ + { + "name": "Disclosure", + "path": "/disclosure/disclosure", + "component": "./components/disclosure/disclosure.vue" + } + ] + }, { "name": "Dialog", "path": "/dialog", diff --git a/packages/@headlessui-vue/src/components/description/description.ts b/packages/@headlessui-vue/src/components/description/description.ts index d4968fd..80f8ac9 100644 --- a/packages/@headlessui-vue/src/components/description/description.ts +++ b/packages/@headlessui-vue/src/components/description/description.ts @@ -11,6 +11,7 @@ import { // Types ComputedRef, InjectionKey, + Ref, } from 'vue' import { useId } from '../../hooks/use-id' @@ -20,7 +21,7 @@ import { render } from '../../utils/render' let DescriptionContext = Symbol('DescriptionContext') as InjectionKey<{ register(value: string): () => void - slot: Record + slot: Ref> name: string props: Record }> @@ -34,11 +35,11 @@ function useDescriptionContext() { } export function useDescriptions({ - slot = {}, + slot = ref({}), name = 'Description', props = {}, }: { - slot?: Record + slot?: Ref> name?: string props?: Record } = {}): ComputedRef { @@ -70,7 +71,7 @@ export let Description = defineComponent({ as: { type: [Object, String], default: 'p' }, }, render() { - let { name = 'Description', slot = {}, props = {} } = this.context + let { name = 'Description', slot = ref({}), props = {} } = this.context let passThroughProps = this.$props let propsWeControl = { ...Object.entries(props).reduce( @@ -82,7 +83,7 @@ export let Description = defineComponent({ return render({ props: { ...passThroughProps, ...propsWeControl }, - slot, + slot: slot.value, attrs: this.$attrs, slots: this.$slots, name, diff --git a/packages/@headlessui-vue/src/components/dialog/dialog.test.ts b/packages/@headlessui-vue/src/components/dialog/dialog.test.ts index d490cad..cdfafec 100644 --- a/packages/@headlessui-vue/src/components/dialog/dialog.test.ts +++ b/packages/@headlessui-vue/src/components/dialog/dialog.test.ts @@ -2,6 +2,7 @@ import { defineComponent, ref, nextTick, h } from 'vue' import { render } from '../../test-utils/vue-testing-library' import { Dialog, DialogOverlay, DialogTitle, DialogDescription } from './dialog' +import { TransitionRoot } from '../transitions/transition' import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs' import { DialogState, @@ -429,6 +430,52 @@ describe('Rendering', () => { }) }) +describe('Composition', () => { + it( + 'should be possible to open the Dialog via a Transition component', + suppressConsoleLogs(async () => { + renderTemplate({ + components: { TransitionRoot }, + template: ` + + + {{JSON.stringify(data)}} + + + + `, + }) + + await new Promise(nextTick) + + assertDialog({ state: DialogState.Visible }) + assertDialogDescription({ + state: DialogState.Visible, + textContent: JSON.stringify({ open: true }), + }) + }) + ) + + it( + 'should be possible to close the Dialog via a Transition component', + suppressConsoleLogs(async () => { + renderTemplate({ + components: { TransitionRoot }, + template: ` + + + {{JSON.stringify(data)}} + + + + `, + }) + + assertDialog({ state: DialogState.InvisibleUnmounted }) + }) + ) +}) + describe('Keyboard interactions', () => { describe('`Escape` key', () => { it( diff --git a/packages/@headlessui-vue/src/components/dialog/dialog.ts b/packages/@headlessui-vue/src/components/dialog/dialog.ts index 25b9c96..5c9fc04 100644 --- a/packages/@headlessui-vue/src/components/dialog/dialog.ts +++ b/packages/@headlessui-vue/src/components/dialog/dialog.ts @@ -31,6 +31,7 @@ import { match } from '../../utils/match' import { ForcePortalRoot } from '../../internal/portal-force-root' import { Description, useDescriptions } from '../description/description' import { dom } from '../../utils/dom' +import { useOpenClosed, State } from '../../internal/open-closed' enum DialogStates { Open, @@ -88,7 +89,8 @@ export let Dialog = defineComponent({ onClick: this.handleClick, onKeydown: this.handleKeyDown, } - let { open, initialFocus, ...passThroughProps } = this.$props + let { open: _, initialFocus, ...passThroughProps } = this.$props + let slot = { open: this.dialogState === DialogStates.Open } return h(ForcePortalRoot, { force: true }, () => @@ -100,7 +102,7 @@ export let Dialog = defineComponent({ slot, attrs: this.$attrs, slots: this.$slots, - visible: open, + visible: this.visible, features: Features.RenderStrategy | Features.Static, name: 'Dialog', }) @@ -112,23 +114,43 @@ export let Dialog = defineComponent({ setup(props, { emit }) { let containers = ref>(new Set()) + let usesOpenClosedState = useOpenClosed() + let open = computed(() => { + // @ts-expect-error We are comparing to a uuid stirng at runtime + if (props.open === Missing && usesOpenClosedState !== null) { + // Update the `open` prop based on the open closed state + return match(usesOpenClosedState.value, { + [State.Open]: true, + [State.Closed]: false, + }) + } + return props.open + }) + // Validations // @ts-expect-error We are comparing to a uuid stirng at runtime - let hasOpen = props.open !== Missing + let hasOpen = props.open !== Missing || usesOpenClosedState !== null if (!hasOpen) { throw new Error(`You forgot to provide an \`open\` prop to the \`Dialog\`.`) } - if (typeof props.open !== 'boolean') { + if (typeof open.value !== 'boolean') { throw new Error( `You provided an \`open\` prop to the \`Dialog\`, but the value is not a boolean. Received: ${ - props.open === Missing ? undefined : props.open + open.value === Missing ? undefined : props.open }` ) } let dialogState = computed(() => (props.open ? DialogStates.Open : DialogStates.Closed)) + let visible = computed(() => { + if (usesOpenClosedState !== null) { + return usesOpenClosedState.value === State.Open + } + + return dialogState.value === DialogStates.Open + }) let internalDialogRef = ref(null) let enabled = ref(dialogState.value === DialogStates.Open) @@ -154,7 +176,7 @@ export let Dialog = defineComponent({ let describedby = useDescriptions({ name: 'DialogDescription', - slot: { open: props.open }, + slot: computed(() => ({ open: open.value })), }) let titleId = ref(null) @@ -235,6 +257,8 @@ export let Dialog = defineComponent({ dialogState, titleId, describedby, + visible, + open, handleClick(event: MouseEvent) { event.stopPropagation() }, diff --git a/packages/@headlessui-vue/src/components/disclosure/disclosure.test.ts b/packages/@headlessui-vue/src/components/disclosure/disclosure.test.ts index d5d7260..e0fd49d 100644 --- a/packages/@headlessui-vue/src/components/disclosure/disclosure.test.ts +++ b/packages/@headlessui-vue/src/components/disclosure/disclosure.test.ts @@ -1,4 +1,4 @@ -import { defineComponent, nextTick } from 'vue' +import { defineComponent, nextTick, ref, watch } from 'vue' import { render } from '../../test-utils/vue-testing-library' import { Disclosure, DisclosureButton, DisclosurePanel } from './disclosure' import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs' @@ -11,14 +11,10 @@ import { } from '../../test-utils/accessibility-assertions' import { click, press, Keys, MouseButton } from '../../test-utils/interactions' import { html } from '../../test-utils/html' +import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' jest.mock('../../hooks/use-id') -beforeAll(() => { - jest.spyOn(window, 'requestAnimationFrame').mockImplementation(setImmediate as any) - jest.spyOn(window, 'cancelAnimationFrame').mockImplementation(clearImmediate as any) -}) - afterAll(() => jest.restoreAllMocks()) function renderTemplate(input: string | Partial[0]>) { @@ -266,6 +262,120 @@ describe('Rendering', () => { }) }) +describe('Composition', () => { + let OpenClosedWrite = defineComponent({ + props: { open: { type: Boolean } }, + setup(props, { slots }) { + useOpenClosedProvider(ref(props.open ? State.Open : State.Closed)) + return () => slots.default?.() + }, + }) + + let OpenClosedRead = defineComponent({ + emits: ['read'], + setup(_, { slots, emit }) { + let state = useOpenClosed() + watch([state], ([value]) => emit('read', value)) + return () => slots.default?.() + }, + }) + + it( + 'should always open the DisclosurePanel because of a wrapping OpenClosed component', + suppressConsoleLogs(async () => { + renderTemplate({ + components: { OpenClosedWrite }, + template: ` + + Trigger + + + {{JSON.stringify(data)}} + + + + `, + }) + + // Verify the Disclosure is visible + assertDisclosurePanel({ state: DisclosureState.Visible }) + + // Let's try and open the Disclosure + await click(getDisclosureButton()) + + // Verify the Disclosure is still visible + assertDisclosurePanel({ state: DisclosureState.Visible }) + }) + ) + + it( + 'should always close the DisclosurePanel because of a wrapping OpenClosed component', + suppressConsoleLogs(async () => { + renderTemplate({ + components: { OpenClosedWrite }, + template: ` + + Trigger + + + {{JSON.stringify(data)}} + + + + `, + }) + + // Verify the Disclosure is hidden + assertDisclosurePanel({ state: DisclosureState.InvisibleUnmounted }) + + // Let's try and open the Disclosure + await click(getDisclosureButton()) + + // Verify the Disclosure is still hidden + assertDisclosurePanel({ state: DisclosureState.InvisibleUnmounted }) + }) + ) + + it( + 'should be possible to read the OpenClosed state', + suppressConsoleLogs(async () => { + let readFn = jest.fn() + renderTemplate({ + components: { OpenClosedRead }, + template: ` + + Trigger + + + + + `, + setup() { + return { readFn } + }, + }) + + await new Promise(nextTick) + + // Verify the Disclosure is hidden + assertDisclosurePanel({ state: DisclosureState.InvisibleUnmounted }) + + // Let's toggle the Disclosure 3 times + await click(getDisclosureButton()) + await click(getDisclosureButton()) + await click(getDisclosureButton()) + + // Verify the Disclosure is visible + assertDisclosurePanel({ state: DisclosureState.Visible }) + + expect(readFn).toHaveBeenCalledTimes(3) + expect(readFn).toHaveBeenNthCalledWith(1, State.Open) + expect(readFn).toHaveBeenNthCalledWith(2, State.Closed) + expect(readFn).toHaveBeenNthCalledWith(3, State.Open) + }) + ) +}) + describe('Keyboard interactions', () => { describe('`Enter` key', () => { it( diff --git a/packages/@headlessui-vue/src/components/disclosure/disclosure.ts b/packages/@headlessui-vue/src/components/disclosure/disclosure.ts index 3aabf04..ba485e3 100644 --- a/packages/@headlessui-vue/src/components/disclosure/disclosure.ts +++ b/packages/@headlessui-vue/src/components/disclosure/disclosure.ts @@ -6,6 +6,7 @@ import { match } from '../../utils/match' import { render, Features } from '../../utils/render' import { useId } from '../../hooks/use-id' import { dom } from '../../utils/dom' +import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' enum DisclosureStates { Open, @@ -61,6 +62,14 @@ export let Disclosure = defineComponent({ } as StateDefinition provide(DisclosureContext, api) + useOpenClosedProvider( + computed(() => { + return match(disclosureState.value, { + [DisclosureStates.Open]: State.Open, + [DisclosureStates.Closed]: State.Closed, + }) + }) + ) return () => { let { defaultOpen: _, ...passThroughProps } = props @@ -159,7 +168,7 @@ export let DisclosurePanel = defineComponent({ attrs: this.$attrs, slots: this.$slots, features: Features.RenderStrategy | Features.Static, - visible: slot.open, + visible: this.visible, name: 'DisclosurePanel', }) }, @@ -167,6 +176,15 @@ export let DisclosurePanel = defineComponent({ let api = useDisclosureContext('DisclosurePanel') let panelId = `headlessui-disclosure-panel-${useId()}` - return { id: panelId, el: api.panelRef } + let usesOpenClosedState = useOpenClosed() + let visible = computed(() => { + if (usesOpenClosedState !== null) { + return usesOpenClosedState.value === State.Open + } + + return api.disclosureState.value === DisclosureStates.Open + }) + + return { id: panelId, el: api.panelRef, visible } }, }) diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx index 286580d..790d0db 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx @@ -35,6 +35,7 @@ import { MouseButton, } from '../../test-utils/interactions' import { html } from '../../test-utils/html' +import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' jest.mock('../../hooks/use-id') @@ -594,6 +595,126 @@ describe('Rendering composition', () => { ) }) +describe('Composition', () => { + let OpenClosedWrite = defineComponent({ + props: { open: { type: Boolean } }, + setup(props, { slots }) { + useOpenClosedProvider(ref(props.open ? State.Open : State.Closed)) + return () => slots.default?.() + }, + }) + + let OpenClosedRead = defineComponent({ + emits: ['read'], + setup(_, { slots, emit }) { + let state = useOpenClosed() + watch([state], ([value]) => emit('read', value)) + return () => slots.default?.() + }, + }) + + it( + 'should always open the ListboxOptions because of a wrapping OpenClosed component', + suppressConsoleLogs(async () => { + renderTemplate({ + components: { OpenClosedWrite }, + template: html` + + Trigger + + + {{JSON.stringify(data)}} + + + + `, + }) + + await new Promise(nextTick) + + // Verify the Listbox is visible + assertListbox({ state: ListboxState.Visible }) + + // Let's try and open the Listbox + await click(getListboxButton()) + + // Verify the Listbox is still visible + assertListbox({ state: ListboxState.Visible }) + }) + ) + + it( + 'should always close the ListboxOptions because of a wrapping OpenClosed component', + suppressConsoleLogs(async () => { + renderTemplate({ + components: { OpenClosedWrite }, + template: html` + + Trigger + + + {{JSON.stringify(data)}} + + + + `, + }) + + await new Promise(nextTick) + + // Verify the Listbox is hidden + assertListbox({ state: ListboxState.InvisibleUnmounted }) + + // Let's try and open the Listbox + await click(getListboxButton()) + + // Verify the Listbox is still hidden + assertListbox({ state: ListboxState.InvisibleUnmounted }) + }) + ) + + it( + 'should be possible to read the OpenClosed state', + suppressConsoleLogs(async () => { + let readFn = jest.fn() + renderTemplate({ + components: { OpenClosedRead }, + template: html` + + Trigger + + + Option A + + + + `, + setup() { + return { value: ref(null), readFn } + }, + }) + + await new Promise(nextTick) + + // Verify the Listbox is hidden + assertListbox({ state: ListboxState.InvisibleUnmounted }) + + // Let's toggle the Listbox 3 times + await click(getListboxButton()) + await click(getListboxButton()) + await click(getListboxButton()) + + // Verify the Listbox is visible + assertListbox({ state: ListboxState.Visible }) + + expect(readFn).toHaveBeenCalledTimes(3) + expect(readFn).toHaveBeenNthCalledWith(1, State.Open) + expect(readFn).toHaveBeenNthCalledWith(2, State.Closed) + expect(readFn).toHaveBeenNthCalledWith(3, State.Open) + }) + ) +}) + describe('Keyboard interactions', () => { describe('`Enter` key', () => { it( diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.ts b/packages/@headlessui-vue/src/components/listbox/listbox.ts index ef1db2c..9e7dc7f 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.ts +++ b/packages/@headlessui-vue/src/components/listbox/listbox.ts @@ -22,6 +22,8 @@ import { calculateActiveIndex, Focus } from '../../utils/calculate-active-index' import { resolvePropValue } from '../../utils/resolve-prop-value' import { dom } from '../../utils/dom' import { useWindowEvent } from '../../hooks/use-window-event' +import { useOpenClosed, State, useOpenClosedProvider } from '../../internal/open-closed' +import { match } from '../../utils/match' enum ListboxStates { Open, @@ -194,6 +196,14 @@ export let Listbox = defineComponent({ // @ts-expect-error Types of property 'dataRef' are incompatible. provide(ListboxContext, api) + useOpenClosedProvider( + computed(() => + match(listboxState.value, { + [ListboxStates.Open]: State.Open, + [ListboxStates.Closed]: State.Closed, + }) + ) + ) return () => { let slot = { open: listboxState.value === ListboxStates.Open, disabled } @@ -360,7 +370,7 @@ export let ListboxOptions = defineComponent({ attrs: this.$attrs, slots: this.$slots, features: Features.RenderStrategy | Features.Static, - visible: slot.open, + visible: this.visible, name: 'ListboxOptions', }) }, @@ -437,7 +447,16 @@ export let ListboxOptions = defineComponent({ } } - return { id, el: api.optionsRef, handleKeyDown } + let usesOpenClosedState = useOpenClosed() + let visible = computed(() => { + if (usesOpenClosedState !== null) { + return usesOpenClosedState.value === State.Open + } + + return api.listboxState.value === ListboxStates.Open + }) + + return { id, el: api.optionsRef, handleKeyDown, visible } }, }) diff --git a/packages/@headlessui-vue/src/components/menu/menu.test.tsx b/packages/@headlessui-vue/src/components/menu/menu.test.tsx index 13a8d42..8b44998 100644 --- a/packages/@headlessui-vue/src/components/menu/menu.test.tsx +++ b/packages/@headlessui-vue/src/components/menu/menu.test.tsx @@ -1,4 +1,4 @@ -import { defineComponent, h, nextTick } from 'vue' +import { defineComponent, h, nextTick, ref, watch } from 'vue' import { render } from '../../test-utils/vue-testing-library' import { Menu, MenuButton, MenuItems, MenuItem } from './menu' import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs' @@ -30,6 +30,7 @@ import { MouseButton, } from '../../test-utils/interactions' import { jsx } from '../../test-utils/html' +import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' jest.mock('../../hooks/use-id') @@ -763,6 +764,124 @@ describe('Rendering composition', () => { ) }) +describe('Composition', () => { + let OpenClosedWrite = defineComponent({ + props: { open: { type: Boolean } }, + setup(props, { slots }) { + useOpenClosedProvider(ref(props.open ? State.Open : State.Closed)) + return () => slots.default?.() + }, + }) + + let OpenClosedRead = defineComponent({ + emits: ['read'], + setup(_, { slots, emit }) { + let state = useOpenClosed() + watch([state], ([value]) => emit('read', value)) + return () => slots.default?.() + }, + }) + + it( + 'should always open the MenuItems because of a wrapping OpenClosed component', + suppressConsoleLogs(async () => { + renderTemplate({ + components: { OpenClosedWrite }, + template: jsx` + + Trigger + + + {{JSON.stringify(data)}} + + + + `, + }) + + await new Promise(nextTick) + + // Verify the Menu is visible + assertMenu({ state: MenuState.Visible }) + + // Let's try and open the Menu + await click(getMenuButton()) + + // Verify the Menu is still visible + assertMenu({ state: MenuState.Visible }) + }) + ) + + it( + 'should always close the MenuItems because of a wrapping OpenClosed component', + suppressConsoleLogs(async () => { + renderTemplate({ + components: { OpenClosedWrite }, + template: jsx` + + Trigger + + + {{JSON.stringify(data)}} + + + + `, + }) + + await new Promise(nextTick) + + // Verify the Menu is hidden + assertMenu({ state: MenuState.InvisibleUnmounted }) + + // Let's try and open the Menu + await click(getMenuButton()) + + // Verify the Menu is still hidden + assertMenu({ state: MenuState.InvisibleUnmounted }) + }) + ) + + it( + 'should be possible to read the OpenClosed state', + suppressConsoleLogs(async () => { + let readFn = jest.fn() + renderTemplate({ + components: { OpenClosedRead }, + template: jsx` + + Trigger + + + + + `, + setup() { + return { readFn } + }, + }) + + await new Promise(nextTick) + + // Verify the Menu is hidden + assertMenu({ state: MenuState.InvisibleUnmounted }) + + // Let's toggle the Menu 3 times + await click(getMenuButton()) + await click(getMenuButton()) + await click(getMenuButton()) + + // Verify the Menu is visible + assertMenu({ state: MenuState.Visible }) + + expect(readFn).toHaveBeenCalledTimes(3) + expect(readFn).toHaveBeenNthCalledWith(1, State.Open) + expect(readFn).toHaveBeenNthCalledWith(2, State.Closed) + expect(readFn).toHaveBeenNthCalledWith(3, State.Open) + }) + ) +}) + describe('Keyboard interactions', () => { describe('`Enter` key', () => { it('should be possible to open the menu with Enter', async () => { diff --git a/packages/@headlessui-vue/src/components/menu/menu.ts b/packages/@headlessui-vue/src/components/menu/menu.ts index b2824ca..ddb3b15 100644 --- a/packages/@headlessui-vue/src/components/menu/menu.ts +++ b/packages/@headlessui-vue/src/components/menu/menu.ts @@ -19,6 +19,8 @@ import { resolvePropValue } from '../../utils/resolve-prop-value' import { dom } from '../../utils/dom' import { useWindowEvent } from '../../hooks/use-window-event' import { useTreeWalker } from '../../hooks/use-tree-walker' +import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' +import { match } from '../../utils/match' enum MenuStates { Open, @@ -153,6 +155,14 @@ export let Menu = defineComponent({ // @ts-expect-error Types of property 'dataRef' are incompatible. provide(MenuContext, api) + useOpenClosedProvider( + computed(() => + match(menuState.value, { + [MenuStates.Open]: State.Open, + [MenuStates.Closed]: State.Closed, + }) + ) + ) return () => { let slot = { open: menuState.value === MenuStates.Open } @@ -283,7 +293,7 @@ export let MenuItems = defineComponent({ attrs: this.$attrs, slots: this.$slots, features: Features.RenderStrategy | Features.Static, - visible: slot.open, + visible: this.visible, name: 'MenuItems', }) }, @@ -384,7 +394,16 @@ export let MenuItems = defineComponent({ } } - return { id, el: api.itemsRef, handleKeyDown, handleKeyUp } + let usesOpenClosedState = useOpenClosed() + let visible = computed(() => { + if (usesOpenClosedState !== null) { + return usesOpenClosedState.value === State.Open + } + + return api.menuState.value === MenuStates.Open + }) + + return { id, el: api.itemsRef, handleKeyDown, handleKeyUp, visible } }, }) diff --git a/packages/@headlessui-vue/src/components/popover/popover.test.ts b/packages/@headlessui-vue/src/components/popover/popover.test.ts index e511090..538257e 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.test.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.test.ts @@ -1,4 +1,4 @@ -import { defineComponent, nextTick } from 'vue' +import { defineComponent, nextTick, ref, watch } from 'vue' import { render } from '../../test-utils/vue-testing-library' import { Popover, PopoverGroup, PopoverButton, PopoverPanel, PopoverOverlay } from './popover' @@ -17,6 +17,7 @@ import { } from '../../test-utils/accessibility-assertions' import { click, press, Keys, MouseButton, shift } from '../../test-utils/interactions' import { html } from '../../test-utils/html' +import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' jest.mock('../../hooks/use-id') @@ -429,6 +430,124 @@ describe('Rendering', () => { }) }) +describe('Composition', () => { + let OpenClosedWrite = defineComponent({ + props: { open: { type: Boolean } }, + setup(props, { slots }) { + useOpenClosedProvider(ref(props.open ? State.Open : State.Closed)) + return () => slots.default?.() + }, + }) + + let OpenClosedRead = defineComponent({ + emits: ['read'], + setup(_, { slots, emit }) { + let state = useOpenClosed() + watch([state], ([value]) => emit('read', value)) + return () => slots.default?.() + }, + }) + + it( + 'should always open the PopoverPanel because of a wrapping OpenClosed component', + suppressConsoleLogs(async () => { + renderTemplate({ + components: { OpenClosedWrite }, + template: html` + + Trigger + + + {{JSON.stringify(data)}} + + + + `, + }) + + await new Promise(nextTick) + + // Verify the Popover is visible + assertPopoverPanel({ state: PopoverState.Visible }) + + // Let's try and open the Popover + await click(getPopoverButton()) + + // Verify the Popover is still visible + assertPopoverPanel({ state: PopoverState.Visible }) + }) + ) + + it( + 'should always close the PopoverPanel because of a wrapping OpenClosed component', + suppressConsoleLogs(async () => { + renderTemplate({ + components: { OpenClosedWrite }, + template: html` + + Trigger + + + {{JSON.stringify(data)}} + + + + `, + }) + + await new Promise(nextTick) + + // Verify the Popover is hidden + assertPopoverPanel({ state: PopoverState.InvisibleUnmounted }) + + // Let's try and open the Popover + await click(getPopoverButton()) + + // Verify the Popover is still hidden + assertPopoverPanel({ state: PopoverState.InvisibleUnmounted }) + }) + ) + + it( + 'should be possible to read the OpenClosed state', + suppressConsoleLogs(async () => { + let readFn = jest.fn() + renderTemplate({ + components: { OpenClosedRead }, + template: html` + + Trigger + + + + + `, + setup() { + return { readFn } + }, + }) + + await new Promise(nextTick) + + // Verify the Popover is hidden + assertPopoverPanel({ state: PopoverState.InvisibleUnmounted }) + + // Let's toggle the Popover 3 times + await click(getPopoverButton()) + await click(getPopoverButton()) + await click(getPopoverButton()) + + // Verify the Popover is visible + assertPopoverPanel({ state: PopoverState.Visible }) + + expect(readFn).toHaveBeenCalledTimes(3) + expect(readFn).toHaveBeenNthCalledWith(1, State.Open) + expect(readFn).toHaveBeenNthCalledWith(2, State.Closed) + expect(readFn).toHaveBeenNthCalledWith(3, State.Open) + }) + ) +}) + describe('Keyboard interactions', () => { describe('`Enter` key', () => { it( diff --git a/packages/@headlessui-vue/src/components/popover/popover.ts b/packages/@headlessui-vue/src/components/popover/popover.ts index ca1384c..458e336 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.ts @@ -9,6 +9,7 @@ import { // Types InjectionKey, Ref, + computed, } from 'vue' import { match } from '../../utils/match' @@ -25,6 +26,7 @@ import { } from '../../utils/focus-management' import { dom } from '../../utils/dom' import { useWindowEvent } from '../../hooks/use-window-event' +import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' enum PopoverStates { Open, @@ -113,6 +115,14 @@ export let Popover = defineComponent({ } as StateDefinition provide(PopoverContext, api) + useOpenClosedProvider( + computed(() => + match(popoverState.value, { + [PopoverStates.Open]: State.Open, + [PopoverStates.Closed]: State.Closed, + }) + ) + ) let registerBag = { buttonId, @@ -377,18 +387,28 @@ export let PopoverOverlay = defineComponent({ attrs: this.$attrs, slots: this.$slots, features: Features.RenderStrategy | Features.Static, - visible: slot.open, + visible: this.visible, name: 'PopoverOverlay', }) }, setup() { let api = usePopoverContext('PopoverOverlay') + let usesOpenClosedState = useOpenClosed() + let visible = computed(() => { + if (usesOpenClosedState !== null) { + return usesOpenClosedState.value === State.Open + } + + return api.popoverState.value === PopoverStates.Open + }) + return { id: `headlessui-popover-overlay-${useId()}`, handleClick() { api.closePopover() }, + visible, } }, }) @@ -419,7 +439,7 @@ export let PopoverPanel = defineComponent({ attrs: this.$attrs, slots: this.$slots, features: Features.RenderStrategy | Features.Static, - visible: slot.open, + visible: this.visible, name: 'PopoverPanel', }) }, @@ -498,6 +518,15 @@ export let PopoverPanel = defineComponent({ true ) + let usesOpenClosedState = useOpenClosed() + let visible = computed(() => { + if (usesOpenClosedState !== null) { + return usesOpenClosedState.value === State.Open + } + + return api.popoverState.value === PopoverStates.Open + }) + return { id: api.panelId, el: api.panel, @@ -513,6 +542,7 @@ export let PopoverPanel = defineComponent({ break } }, + visible, } }, }) diff --git a/packages/@headlessui-vue/src/components/transitions/transition.ts b/packages/@headlessui-vue/src/components/transitions/transition.ts index ebd56ee..61e5d67 100644 --- a/packages/@headlessui-vue/src/components/transitions/transition.ts +++ b/packages/@headlessui-vue/src/components/transitions/transition.ts @@ -21,6 +21,7 @@ import { match } from '../../utils/match' import { Features, render, RenderStrategy } from '../../utils/render' import { Reason, transition } from './utils/transition' import { dom } from '../../utils/dom' +import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' type ID = ReturnType @@ -277,9 +278,16 @@ export let TransitionChild = defineComponent({ { immediate: true } ) }) - // onUpdated(() => executeTransition(() => {})) provide(NestingContext, nesting) + useOpenClosedProvider( + computed(() => + match(state.value, { + [TreeStates.Visible]: State.Open, + [TreeStates.Hidden]: State.Closed, + }) + ) + ) return { el: container, state } }, @@ -329,13 +337,26 @@ export let TransitionRoot = defineComponent({ }) }, setup(props) { + let usesOpenClosedState = useOpenClosed() + + let show = computed(() => { + if (props.show === null && usesOpenClosedState !== null) { + return match(usesOpenClosedState.value, { + [State.Open]: true, + [State.Closed]: false, + }) + } + + return props.show + }) + watchEffect(() => { - if (![true, false].includes(props.show)) { + if (![true, false].includes(show.value)) { throw new Error('A is used but it is missing a `:show="true | false"` prop.') } }) - let state = ref(props.show ? TreeStates.Visible : TreeStates.Hidden) + let state = ref(show.value ? TreeStates.Visible : TreeStates.Hidden) let nestingBag = useNesting(() => { state.value = TreeStates.Hidden @@ -343,7 +364,7 @@ export let TransitionRoot = defineComponent({ let initial = { value: true } let transitionBag = { - show: computed(() => props.show), + show, appear: computed(() => props.appear || !initial.value), } @@ -351,7 +372,7 @@ export let TransitionRoot = defineComponent({ watchEffect(() => { initial.value = false - if (props.show) { + if (show.value) { state.value = TreeStates.Visible } else if (!hasChildren(nestingBag)) { state.value = TreeStates.Hidden @@ -362,6 +383,6 @@ export let TransitionRoot = defineComponent({ provide(NestingContext, nestingBag) provide(TransitionContext, transitionBag) - return { state } + return { state, show } }, }) diff --git a/packages/@headlessui-vue/src/internal/open-closed.ts b/packages/@headlessui-vue/src/internal/open-closed.ts new file mode 100644 index 0000000..4f95240 --- /dev/null +++ b/packages/@headlessui-vue/src/internal/open-closed.ts @@ -0,0 +1,23 @@ +import { + inject, + provide, + + // Types + InjectionKey, + Ref, +} from 'vue' + +let Context = Symbol('Context') as InjectionKey> + +export enum State { + Open, + Closed, +} + +export function useOpenClosed() { + return inject(Context, null) +} + +export function useOpenClosedProvider(value: Ref) { + provide(Context, value) +} From ab92811e13a9c3567e5ef5c7fd713f20bd6d3476 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 3 May 2021 13:12:23 +0200 Subject: [PATCH 2/4] parallelize GitHub actions (#463) --- .github/workflows/main.yml | 60 ++++++++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 15 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 15b6f77..c61462c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,43 +1,73 @@ name: CI + on: [push] + +env: + NODE_VERSION: 12.x + jobs: - build: + install: runs-on: ubuntu-latest steps: - name: Begin CI... uses: actions/checkout@v2 - - - name: Use Node 12 + - name: Use Node ${{ env.NODE_VERSION }} uses: actions/setup-node@v2 with: - node-version: 12 - - # - name: Use cached node_modules - # id: cache - # uses: actions/cache@v2 - # with: - # path: node_modules - # key: nodeModules-${{ hashFiles('**/yarn.lock') }} - # restore-keys: | - # nodeModules- - + node-version: ${{ env.NODE_VERSION }} + - uses: actions/cache@v2 + with: + path: '**/node_modules' + key: ${{ runner.os }}-${{ env.NODE_VERSION }}-modules-${{ hashFiles('**/yarn.lock') }} - name: Install dependencies - # if: steps.cache.outputs.cache-hit != 'true' run: yarn install --frozen-lockfile env: CI: true + lint: + runs-on: ubuntu-latest + needs: [install] + + steps: + - name: Begin CI... + uses: actions/checkout@v2 + - uses: actions/cache@v2 + with: + path: '**/node_modules' + key: ${{ runner.os }}-${{ env.NODE_VERSION }}-modules-${{ hashFiles('**/yarn.lock') }} - name: Lint run: yarn lint env: CI: true + test: + runs-on: ubuntu-latest + needs: [install] + + steps: + - name: Begin CI... + uses: actions/checkout@v2 + - uses: actions/cache@v2 + with: + path: '**/node_modules' + key: ${{ runner.os }}-${{ env.NODE_VERSION }}-modules-${{ hashFiles('**/yarn.lock') }} - name: Test run: yarn test env: CI: true + build: + runs-on: ubuntu-latest + needs: [install] + + steps: + - name: Begin CI... + uses: actions/checkout@v2 + - uses: actions/cache@v2 + with: + path: '**/node_modules' + key: ${{ runner.os }}-${{ env.NODE_VERSION }}-modules-${{ hashFiles('**/yarn.lock') }} - name: Build run: yarn build env: From c13e6b77529e81441a6efaf821495d6e98f340cc Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 4 May 2021 12:18:53 +0200 Subject: [PATCH 3/4] Improve dialog and SSR (#477) * delay initialization of Dialog We were using a useLayoutEffect, now let's use a useEffect instead. It still moves focus to the correct element, but that process is now a bit delayed. This means that users will less-likely be urged to "hack" around the issue by using fake focusable elements which will result in worse accessibility. * add hook to deal with server handoff This will allow us to delay certain features. For example we can delay the focus trapping until it is fully hydrated. We can also delay rendering the Portal to ensure hydration works correctly. * use server handoff complete hook * update changelog --- CHANGELOG.md | 5 +++++ .../src/components/dialog/dialog.tsx | 4 +++- .../src/components/focus-trap/focus-trap.tsx | 4 +++- .../src/components/portal/portal.tsx | 11 ++++++----- .../src/components/transitions/transition.tsx | 7 +++++-- .../src/hooks/use-focus-trap.ts | 4 ++-- .../@headlessui-react/src/hooks/use-id.ts | 11 ++++------- .../src/hooks/use-server-handoff-complete.ts | 19 +++++++++++++++++++ 8 files changed, 47 insertions(+), 18 deletions(-) create mode 100644 packages/@headlessui-react/src/hooks/use-server-handoff-complete.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0515f31..9a75b4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Introduce Open/Closed state, to simplify component communication ([#466](https://github.com/tailwindlabs/headlessui/pull/466)) +### Fixes + +- Improve SSR for `Dialog` ([#477](https://github.com/tailwindlabs/headlessui/pull/477)) +- Delay focus trap initialization ([#477](https://github.com/tailwindlabs/headlessui/pull/477)) + ## [Unreleased - Vue] ### Added diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx index e39530f..7319db6 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.tsx @@ -33,6 +33,7 @@ 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' enum DialogStates { Open, @@ -235,7 +236,8 @@ let DialogRoot = forwardRefWithAs(function Dialog< return () => observer.disconnect() }, [dialogState, internalDialogRef, close]) - let enabled = dialogState === DialogStates.Open + let ready = useServerHandoffComplete() + let enabled = ready && dialogState === DialogStates.Open useFocusTrap(containers, enabled, { initialFocus }) useInertOthers(internalDialogRef, enabled) 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 2d565b0..ee587a7 100644 --- a/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx +++ b/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx @@ -9,6 +9,7 @@ import { import { Props } from '../../types' import { render } from '../../utils/render' import { useFocusTrap } from '../../hooks/use-focus-trap' +import { useServerHandoffComplete } from '../../hooks/use-server-handoff-complete' let DEFAULT_FOCUS_TRAP_TAG = 'div' as const @@ -18,7 +19,8 @@ export function FocusTrap>(new Set()) let { initialFocus, ...passthroughProps } = props - useFocusTrap(containers, true, { initialFocus }) + let ready = useServerHandoffComplete() + useFocusTrap(containers, ready, { initialFocus }) let propsWeControl = { ref(element: HTMLElement | null) { diff --git a/packages/@headlessui-react/src/components/portal/portal.tsx b/packages/@headlessui-react/src/components/portal/portal.tsx index 24c1fb9..93d55b3 100644 --- a/packages/@headlessui-react/src/components/portal/portal.tsx +++ b/packages/@headlessui-react/src/components/portal/portal.tsx @@ -16,6 +16,7 @@ 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' function usePortalTarget(): HTMLElement | null { let forceInRoot = usePortalRoot() @@ -57,6 +58,8 @@ export function Portal( typeof window === 'undefined' ? null : document.createElement('div') ) + let ready = useServerHandoffComplete() + useElementStack(element) useIsoMorphicEffect(() => { @@ -77,16 +80,14 @@ export function Portal( } }, [target, element]) + if (!ready) return null + return ( {!target || !element ? null : createPortal( - render({ - props: passthroughProps, - defaultTag: DEFAULT_PORTAL_TAG, - name: 'Portal', - }), + render({ props: passthroughProps, defaultTag: DEFAULT_PORTAL_TAG, name: 'Portal' }), element )} diff --git a/packages/@headlessui-react/src/components/transitions/transition.tsx b/packages/@headlessui-react/src/components/transitions/transition.tsx index 598e428..fc4585e 100644 --- a/packages/@headlessui-react/src/components/transitions/transition.tsx +++ b/packages/@headlessui-react/src/components/transitions/transition.tsx @@ -23,6 +23,7 @@ import { useIsoMorphicEffect } from '../../hooks/use-iso-morphic-effect' import { Features, PropsForFeatures, render, RenderStrategy } from '../../utils/render' import { Reason, transition } from './utils/transition' import { OpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' +import { useServerHandoffComplete } from '../../hooks/use-server-handoff-complete' type ID = ReturnType @@ -260,11 +261,13 @@ function TransitionChild { - if (state === TreeStates.Visible && container.current === null) { + if (ready && state === TreeStates.Visible && container.current === null) { throw new Error('Did you forget to passthrough the `ref` to the actual DOM node?') } - }, [container, state]) + }, [container, state, ready]) // Skipping initial transition let skip = initial && !appear diff --git a/packages/@headlessui-react/src/hooks/use-focus-trap.ts b/packages/@headlessui-react/src/hooks/use-focus-trap.ts index 4e0938a..600039b 100644 --- a/packages/@headlessui-react/src/hooks/use-focus-trap.ts +++ b/packages/@headlessui-react/src/hooks/use-focus-trap.ts @@ -2,10 +2,10 @@ import { useRef, // Types MutableRefObject, + useEffect, } from 'react' import { Keys } from '../components/keyboard' -import { useIsoMorphicEffect } from './use-iso-morphic-effect' import { focusElement, focusIn, Focus, FocusResult } from '../utils/focus-management' import { contains } from '../internal/dom-containers' import { useWindowEvent } from './use-window-event' @@ -22,7 +22,7 @@ export function useFocusTrap( let mounted = useRef(false) // Handle initial focus - useIsoMorphicEffect(() => { + useEffect(() => { if (!enabled) return if (containers.current.size !== 1) return diff --git a/packages/@headlessui-react/src/hooks/use-id.ts b/packages/@headlessui-react/src/hooks/use-id.ts index d805022..b692971 100644 --- a/packages/@headlessui-react/src/hooks/use-id.ts +++ b/packages/@headlessui-react/src/hooks/use-id.ts @@ -1,5 +1,6 @@ -import { useState, useEffect } from 'react' +import { useState } from 'react' import { useIsoMorphicEffect } from './use-iso-morphic-effect' +import { useServerHandoffComplete } from './use-server-handoff-complete' // We used a "simple" approach first which worked for SSR and rehydration on the client. However we // didn't take care of the Suspense case. To fix this we used the approach the @reach-ui/auto-id @@ -7,22 +8,18 @@ import { useIsoMorphicEffect } from './use-iso-morphic-effect' // // Credits: https://github.com/reach/reach-ui/blob/develop/packages/auto-id/src/index.tsx -let state = { serverHandoffComplete: false } let id = 0 function generateId() { return ++id } export function useId() { - let [id, setId] = useState(state.serverHandoffComplete ? generateId : null) + let ready = useServerHandoffComplete() + let [id, setId] = useState(ready ? generateId : null) useIsoMorphicEffect(() => { if (id === null) setId(generateId()) }, [id]) - useEffect(() => { - if (state.serverHandoffComplete === false) state.serverHandoffComplete = true - }, []) - return id != null ? '' + id : undefined } diff --git a/packages/@headlessui-react/src/hooks/use-server-handoff-complete.ts b/packages/@headlessui-react/src/hooks/use-server-handoff-complete.ts new file mode 100644 index 0000000..b24bc66 --- /dev/null +++ b/packages/@headlessui-react/src/hooks/use-server-handoff-complete.ts @@ -0,0 +1,19 @@ +import { useState, useEffect } from 'react' + +let state = { serverHandoffComplete: false } + +export function useServerHandoffComplete() { + let [serverHandoffComplete, setServerHandoffComplete] = useState(state.serverHandoffComplete) + + useEffect(() => { + if (serverHandoffComplete === true) return + + setServerHandoffComplete(true) + }, [serverHandoffComplete]) + + useEffect(() => { + if (state.serverHandoffComplete === false) state.serverHandoffComplete = true + }, []) + + return serverHandoffComplete +} From 084a2497d80d28a0bd76e60b482b9b020b6f405e Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 7 May 2021 16:29:35 +0200 Subject: [PATCH 4/4] Fix incorrect nested Dialogs behaviour (#489) * add tests to verify the nested Dialog behaviour * set mounted to true once rendered once * cache useWindowEvent listener We only care about the very last version of the listener function. This allows us to only change the event listener if the event name (string) and options (boolean | object) change. * add/delete messages when mounting/unmounting We don't require a dedicated hook anymore, so this is a bit of cleanup! * add comments to the FocusResult enum * splitup functionality and make it a bit more clear using feature flags * add getDialogOverlays helper * simplify the Portal component We don't need to add the current element to the Stack. We only want to take care of that in the Dialog component itself. * drop dom-containers Currently it is only used in a single spot, so I inlined it into that file. * simplify the FocusTrap component, use new API * improve Dialog component * update CHANGELOG --- CHANGELOG.md | 1 + .../src/components/dialog/dialog.test.tsx | 273 +++++++++++---- .../src/components/dialog/dialog.tsx | 62 +++- .../src/components/focus-trap/focus-trap.tsx | 11 +- .../src/components/portal/portal.tsx | 19 +- .../src/hooks/use-focus-trap.ts | 139 +++++--- .../src/hooks/use-is-mounted.ts | 4 +- .../src/hooks/use-window-event.ts | 15 +- .../src/internal/dom-containers.ts | 7 - .../src/internal/stack-context.tsx | 40 ++- .../test-utils/accessibility-assertions.ts | 4 + .../src/utils/focus-management.ts | 7 + .../src/components/dialog/dialog.test.ts | 330 +++++++++++++----- .../test-utils/accessibility-assertions.ts | 4 + 14 files changed, 633 insertions(+), 283 deletions(-) delete mode 100644 packages/@headlessui-react/src/internal/dom-containers.ts 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 {