From b7c04d1a891e753fb8d7936e6fa99c93c02a3293 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Wed, 2 Sep 2026 16:31:13 -0600 Subject: [PATCH] feat(headless): hold exiting popup contents by snapshot and add onOpenChangeComplete Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01QqhW8vxHmRMqHiuZHN2HCp --- .changeset/exit-freeze-snapshot.md | 2 + .../headless/src/hooks/use-transition.test.ts | 59 ++++++++ packages/headless/src/hooks/use-transition.ts | 38 +++-- .../headless/src/primitives/dialog/README.md | 32 ++-- .../src/primitives/dialog/dialog-root.tsx | 7 + .../src/primitives/flow/flow-step.tsx | 11 +- .../headless/src/primitives/popover/README.md | 21 +-- .../src/primitives/popover/popover-root.tsx | 7 + packages/headless/src/utils/freeze.test.tsx | 137 +++++++++++++++--- packages/headless/src/utils/freeze.tsx | 79 ++++------ packages/headless/src/utils/index.ts | 2 +- .../swingset/src/stories/dialog.component.mdx | 21 ++- .../src/stories/dialog.component.stories.tsx | 77 +++++++++- .../mosaic/blocks/destructive/destructive.tsx | 18 +-- .../src/mosaic/components/dialog/dialog.tsx | 2 + .../mosaic/user-button/user-button.view.tsx | 23 ++- 16 files changed, 405 insertions(+), 131 deletions(-) create mode 100644 .changeset/exit-freeze-snapshot.md diff --git a/.changeset/exit-freeze-snapshot.md b/.changeset/exit-freeze-snapshot.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/exit-freeze-snapshot.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/headless/src/hooks/use-transition.test.ts b/packages/headless/src/hooks/use-transition.test.ts index a1fbe7e9f69..98776f9e3f0 100644 --- a/packages/headless/src/hooks/use-transition.test.ts +++ b/packages/headless/src/hooks/use-transition.test.ts @@ -224,4 +224,63 @@ describe('useTransition', () => { expect(result.current.mounted).toBe(true); expect(result.current.transitionProps).toEqual({ 'data-open': '' }); }); + + it('reports the close complete once the exit settles and the element has unmounted', async () => { + const { ref, resolveAnim } = createAnimatingRef(); + const onOpenChangeComplete = vi.fn(); + const { result, rerender } = renderHook(({ open }) => useTransition({ open, ref, onOpenChangeComplete }), { + initialProps: { open: true }, + }); + act(() => flushRaf()); + onOpenChangeComplete.mockClear(); + + rerender({ open: false }); + expect(onOpenChangeComplete).not.toHaveBeenCalled(); + + await act(async () => { + resolveAnim(); + await new Promise(r => setTimeout(r, 0)); + }); + + expect(result.current.mounted).toBe(false); + expect(onOpenChangeComplete).toHaveBeenCalledTimes(1); + expect(onOpenChangeComplete).toHaveBeenCalledWith(false); + }); + + it('reports the open complete once the enter settles', async () => { + const { ref, resolveAnim } = createAnimatingRef(); + const onOpenChangeComplete = vi.fn(); + renderHook(({ open }) => useTransition({ open, ref, onOpenChangeComplete }), { + initialProps: { open: true }, + }); + expect(onOpenChangeComplete).not.toHaveBeenCalled(); + + act(() => flushRaf()); + await act(async () => { + resolveAnim(); + await new Promise(r => setTimeout(r, 0)); + }); + + expect(onOpenChangeComplete).toHaveBeenCalledTimes(1); + expect(onOpenChangeComplete).toHaveBeenCalledWith(true); + }); + + it('does not report a close that reopening interrupted', async () => { + const { ref, resolveAnim } = createAnimatingRef(); + const onOpenChangeComplete = vi.fn(); + const { rerender } = renderHook(({ open }) => useTransition({ open, ref, onOpenChangeComplete }), { + initialProps: { open: true }, + }); + act(() => flushRaf()); + onOpenChangeComplete.mockClear(); + + rerender({ open: false }); + rerender({ open: true }); + await act(async () => { + resolveAnim(); + await new Promise(r => setTimeout(r, 0)); + }); + + expect(onOpenChangeComplete).not.toHaveBeenCalledWith(false); + }); }); diff --git a/packages/headless/src/hooks/use-transition.ts b/packages/headless/src/hooks/use-transition.ts index 0641f6e9ff5..d34e7870df3 100644 --- a/packages/headless/src/hooks/use-transition.ts +++ b/packages/headless/src/hooks/use-transition.ts @@ -1,6 +1,6 @@ 'use client'; -import { type CSSProperties, type RefObject, useEffect, useMemo } from 'react'; +import { type CSSProperties, type RefObject, useEffect, useLayoutEffect, useMemo, useRef } from 'react'; import { useAnimationsFinished } from './use-animations-finished'; import { type TransitionStatus, useTransitionStatus } from './use-transition-status'; @@ -8,6 +8,12 @@ import { type TransitionStatus, useTransitionStatus } from './use-transition-sta export interface UseTransitionOptions { open: boolean; ref: RefObject; + /** + * Fires once the enter or exit animation has finished — on exit, right after the element + * unmounts. State that should reset when the element closes belongs here rather than in + * `onOpenChange`, so the reset cannot show through the exit animation. + */ + onOpenChangeComplete?: (open: boolean) => void; } export interface TransitionProps { @@ -38,21 +44,31 @@ export interface UseTransitionReturn { * transitions (via `[data-starting-style]` / `[data-ending-style]`) and * CSS keyframe animations (via `[data-open]` / `[data-closed]`). */ -export function useTransition({ open, ref }: UseTransitionOptions): UseTransitionReturn { +export function useTransition({ open, ref, onOpenChangeComplete }: UseTransitionOptions): UseTransitionReturn { const { mounted, transitionStatus, setMounted } = useTransitionStatus(open); const runOnAnimationsFinished = useAnimationsFinished(ref, open); + const onOpenChangeCompleteRef = useRef(onOpenChangeComplete); + useLayoutEffect(() => { + onOpenChangeCompleteRef.current = onOpenChangeComplete; + }); + useEffect(() => { - if (transitionStatus !== 'ending') { - return; + // `ending` outlives a reopen by one frame (the status clears on the next rAF), and the + // element must not be scheduled to unmount in that window. + if (!open && transitionStatus === 'ending') { + // Cancelling on cleanup is what makes an exit interruptible: reopening + // mid-exit must abandon the pending unmount, not unmount once the + // retargeted transition settles. + return runOnAnimationsFinished(() => { + setMounted(false); + onOpenChangeCompleteRef.current?.(false); + }); + } + if (open && transitionStatus === undefined) { + return runOnAnimationsFinished(() => onOpenChangeCompleteRef.current?.(true)); } - // Cancelling on cleanup is what makes an exit interruptible: reopening - // mid-exit must abandon the pending unmount, not unmount once the - // retargeted transition settles. - return runOnAnimationsFinished(() => { - setMounted(false); - }); - }, [transitionStatus, runOnAnimationsFinished, setMounted]); + }, [open, transitionStatus, runOnAnimationsFinished, setMounted]); const transitionProps = useMemo(() => { const props: TransitionProps = {}; diff --git a/packages/headless/src/primitives/dialog/README.md b/packages/headless/src/primitives/dialog/README.md index a85d843fe89..dedd471341f 100644 --- a/packages/headless/src/primitives/dialog/README.md +++ b/packages/headless/src/primitives/dialog/README.md @@ -148,17 +148,18 @@ the close was pointer-driven, where focus is left where the pointer put it (see ### `Dialog.Root` -| Prop | Type | Default | Description | -| -------------- | ----------------------------------------------------------- | ---------- | --------------------------------------------------------------------- | -| `open` | `boolean` | — | Controlled open state | -| `defaultOpen` | `boolean` | `false` | Initial open state (uncontrolled) | -| `onOpenChange` | `(open: boolean, details: DialogOpenChangeDetails) => void` | — | Called when open state changes; `details` names the trigger behind it | -| `modal` | `boolean` | `true` | Traps focus and blocks page interaction | -| `role` | `'dialog' \| 'alertdialog'` | `'dialog'` | The popup's ARIA role | -| `closedBy` | `'any' \| 'closerequest' \| 'none'` | `'any'` | Which gestures dismiss the dialog | -| `handle` | `DialogHandle` | — | Connects detached triggers (see `Dialog.createHandle()`) | -| `triggerId` | `string \| null` | — | Controls which trigger the open is attributed to | -| `children` | `ReactNode \| ({ payload }) => ReactNode` | — | Content, or a render function of the active `payload` | +| Prop | Type | Default | Description | +| ---------------------- | ----------------------------------------------------------- | ---------- | --------------------------------------------------------------------------------------- | +| `open` | `boolean` | — | Controlled open state | +| `defaultOpen` | `boolean` | `false` | Initial open state (uncontrolled) | +| `onOpenChange` | `(open: boolean, details: DialogOpenChangeDetails) => void` | — | Called when open state changes; `details` names the trigger behind it | +| `onOpenChangeComplete` | `(open: boolean) => void` | — | Called once the open or close animation has finished; reset what the dialog showed here | +| `modal` | `boolean` | `true` | Traps focus and blocks page interaction | +| `role` | `'dialog' \| 'alertdialog'` | `'dialog'` | The popup's ARIA role | +| `closedBy` | `'any' \| 'closerequest' \| 'none'` | `'any'` | Which gestures dismiss the dialog | +| `handle` | `DialogHandle` | — | Connects detached triggers (see `Dialog.createHandle()`) | +| `triggerId` | `string \| null` | — | Controls which trigger the open is attributed to | +| `children` | `ReactNode \| ({ payload }) => ReactNode` | — | Content, or a render function of the active `payload` | #### `closedBy` @@ -218,9 +219,12 @@ root, and `initialFocus={false}` on the popup so mounting does not steal focus. `DialogFocusTarget` is `boolean | RefObject | (interactionType) => boolean | void | HTMLElement | null`. -The popup's children are held at their last committed frame while it exits (`Freeze`), so state -that resets on close — a machine returning to its initial state — does not flash through the -fade. The popup element itself stays live for `data-closed` / `data-ending-style`. +The popup's children are held while it exits (`Freeze` from `@clerk/headless/utils`): the popup +keeps rendering the element it was handed on the last open render, so state a parent bakes into +that JSX — a machine returning to its initial state, a form clearing — does not flash through the +fade. Components inside keep rendering, so a value they read through a hook (context, a store) +still moves; wrap that read in `useFrozenValue`, or reset it in `onOpenChangeComplete`, which +fires after the exit. The popup element itself stays live for `data-closed` / `data-ending-style`. ### `Dialog.Backdrop`, `Dialog.Title`, `Dialog.Description`, `Dialog.Close` diff --git a/packages/headless/src/primitives/dialog/dialog-root.tsx b/packages/headless/src/primitives/dialog/dialog-root.tsx index 0d2f3c1a01e..bc3cd23998e 100644 --- a/packages/headless/src/primitives/dialog/dialog-root.tsx +++ b/packages/headless/src/primitives/dialog/dialog-root.tsx @@ -56,6 +56,12 @@ export interface DialogProps { open?: boolean; defaultOpen?: boolean; onOpenChange?: (open: boolean, details: DialogOpenChangeDetails) => void; + /** + * Fires once the open or close animation has finished. Reset what the dialog showed here + * rather than in `onOpenChange`: the popup's contents hold their last frame while closing, and + * this is the first moment a reset cannot show through the animation. + */ + onOpenChangeComplete?: (open: boolean) => void; /** When true, the dialog traps focus and blocks interaction with the rest of the page. Default: true */ modal?: boolean; /** Which gestures dismiss the dialog. Default: `any` */ @@ -207,6 +213,7 @@ function DialogInner(props: DialogProps & { isNested: boolean const { mounted, transitionProps } = useTransition({ open, ref: popupRef, + onOpenChangeComplete: props.onOpenChangeComplete, }); // Below `useTransition` because it needs `mounted`: what a stacked child has to key off is diff --git a/packages/headless/src/primitives/flow/flow-step.tsx b/packages/headless/src/primitives/flow/flow-step.tsx index 608e97e21b8..8dc4b670c3d 100644 --- a/packages/headless/src/primitives/flow/flow-step.tsx +++ b/packages/headless/src/primitives/flow/flow-step.tsx @@ -4,7 +4,7 @@ import { inertProps } from '@clerk/shared/inert'; import React, { useLayoutEffect, useRef } from 'react'; import { useTransition } from '../../hooks/use-transition'; -import { type ComponentProps, mergeProps, useRender } from '../../utils'; +import { type ComponentProps, Freeze, mergeProps, useRender } from '../../utils'; import { useFlowContext } from './flow-context'; export interface FlowStepProps extends ComponentProps<'div'> { @@ -16,12 +16,9 @@ export const FlowStep = React.forwardRef(function const { value, direction, registerActiveStep, unregisterActiveStep } = useFlowContext(); const open = ids.includes(value); const stepRef = useRef(null); - const activeChildrenRef = useRef(children); const hasBeenClosed = useRef(false); - if (open) { - activeChildrenRef.current = children; - } else { + if (!open) { hasBeenClosed.current = true; } @@ -49,7 +46,9 @@ export const FlowStep = React.forwardRef(function ...effectiveTransitionProps.style, ['--cl-flow-transition-direction' as string]: String(direction), }, - children: open ? children : activeChildrenRef.current, + // An outgoing step keeps showing what it showed while active, not what its controller has + // since moved on to. + children: {children}, }; return useRender({ diff --git a/packages/headless/src/primitives/popover/README.md b/packages/headless/src/primitives/popover/README.md index 5e75920e4b1..5210c67456e 100644 --- a/packages/headless/src/primitives/popover/README.md +++ b/packages/headless/src/primitives/popover/README.md @@ -63,15 +63,16 @@ const [open, setOpen] = useState(false); ### `Popover.Root` -| Prop | Type | Default | Description | -| -------------- | ------------------------- | ---------- | ----------------------------------- | -| `open` | `boolean` | — | Controlled open state | -| `defaultOpen` | `boolean` | `false` | Initial open state (uncontrolled) | -| `onOpenChange` | `(open: boolean) => void` | — | Called when open state changes | -| `placement` | `Placement` | `"bottom"` | Floating UI placement | -| `sideOffset` | `number` | `4` | Gap between trigger and popup (px) | -| `alignOffset` | `number` | `0` | Nudge along the alignment axis (px) | -| `modal` | `boolean` | `false` | Traps focus within the popover | +| Prop | Type | Default | Description | +| ---------------------- | ------------------------- | ---------- | -------------------------------------------------------------------------------------- | +| `open` | `boolean` | — | Controlled open state | +| `defaultOpen` | `boolean` | `false` | Initial open state (uncontrolled) | +| `onOpenChange` | `(open: boolean) => void` | — | Called when open state changes | +| `onOpenChangeComplete` | `(open: boolean) => void` | — | Called once the open or close animation has finished; reset what the popup showed here | +| `placement` | `Placement` | `"bottom"` | Floating UI placement | +| `sideOffset` | `number` | `4` | Gap between trigger and popup (px) | +| `alignOffset` | `number` | `0` | Nudge along the alignment axis (px) | +| `modal` | `boolean` | `false` | Traps focus within the popover | ### `Popover.Trigger`, `Popover.Positioner`, `Popover.Popup`, `Popover.Title`, `Popover.Description`, `Popover.Close` @@ -105,7 +106,7 @@ Middleware stack: `offset` -> `flip` -> `shift` -> `arrow` -> CSS vars. The popu - **Title and Description are optional but recommended.** They wire `aria-labelledby` and `aria-describedby` to the positioner. If omitted, those attributes are simply absent. - **Non-modal by default.** Unlike Dialog, the page remains interactive behind the popover. Set `modal={true}` for a stricter focus trap. - **Nested popovers are supported.** The `FloatingTree` pattern handles nesting automatically. -- **Popup contents freeze while closing.** The popup outlives `open` by its exit animation, so its children are wrapped in `Freeze` (`@clerk/headless/utils`) and hold their last frame instead of re-rendering under the animation. The popup element itself keeps updating, so `data-closed` / `data-ending-style` still land. Freezing wraps the children in a `display: contents` element and detaches refs inside them until the popup reopens. +- **Popup contents hold while closing.** The popup outlives `open` by its exit animation, so its children are wrapped in `Freeze` (`@clerk/headless/utils`): the popup keeps rendering the element it was handed on the last open render, so props, conditionals, and callbacks baked into that JSX hold instead of changing under the animation. Components inside keep rendering, so a value read through a hook (context, a store) still moves — wrap that read in `useFrozenValue` to hold it too, or reset it in `onOpenChangeComplete`, which fires after the exit. The popup element itself keeps updating, so `data-closed` / `data-ending-style` still land. ## ARIA diff --git a/packages/headless/src/primitives/popover/popover-root.tsx b/packages/headless/src/primitives/popover/popover-root.tsx index 212277c3e4c..f384b6ed959 100644 --- a/packages/headless/src/primitives/popover/popover-root.tsx +++ b/packages/headless/src/primitives/popover/popover-root.tsx @@ -29,6 +29,12 @@ export interface PopoverProps { open?: boolean; defaultOpen?: boolean; onOpenChange?: (open: boolean) => void; + /** + * Fires once the open or close animation has finished. Reset what the popup showed here + * rather than in `onOpenChange`: the contents hold their last frame while closing, and this is + * the first moment a reset cannot show through the animation. + */ + onOpenChangeComplete?: (open: boolean) => void; placement?: Placement; sideOffset?: number; alignOffset?: number; @@ -97,6 +103,7 @@ function PopoverInner(props: PopoverProps) { const { mounted, transitionProps } = useTransition({ open, ref: popupRef, + onOpenChangeComplete: props.onOpenChangeComplete, }); const click = useClick(floatingContext); diff --git a/packages/headless/src/utils/freeze.test.tsx b/packages/headless/src/utils/freeze.test.tsx index b3f8f3773e6..e0b344f9dd8 100644 --- a/packages/headless/src/utils/freeze.test.tsx +++ b/packages/headless/src/utils/freeze.test.tsx @@ -2,7 +2,7 @@ import { act, cleanup, render, screen } from '@testing-library/react'; import * as React from 'react'; import { afterEach, describe, expect, it } from 'vitest'; -import { Freeze } from './freeze'; +import { Freeze, useFrozenValue, useIsFrozen } from './freeze'; afterEach(() => { cleanup(); @@ -15,7 +15,7 @@ describe('Freeze', () => { expect(screen.getByText('Acme')).toBeInTheDocument(); }); - it('holds the committed DOM when children change while frozen', () => { + it('holds the last unfrozen children when they change while frozen', () => { const { rerender } = render(Acme); rerender(Globex); @@ -24,15 +24,7 @@ describe('Freeze', () => { expect(screen.queryByText('Globex')).toBeNull(); }); - it('keeps the held DOM visible', () => { - const { rerender } = render(Acme); - - rerender(Globex); - - expect(screen.getByText('Acme')).toBeVisible(); - }); - - it('keeps the held DOM visible across further updates while frozen', () => { + it('keeps holding across further updates while frozen', () => { const { rerender } = render(Acme); rerender(Globex); @@ -41,7 +33,7 @@ describe('Freeze', () => { expect(screen.getByText('Acme')).toBeVisible(); }); - it('commits the pending children once unfrozen', () => { + it('renders the pending children once unfrozen', () => { const { rerender } = render(Acme); rerender(Globex); @@ -51,7 +43,43 @@ describe('Freeze', () => { expect(screen.queryByText('Acme')).toBeNull(); }); - it('holds a state update raised by the subtree itself', () => { + it('holds children that render to nothing once closed', () => { + function Harness({ item }: { item: string | null }) { + return {item ? {item} : null}; + } + const { rerender } = render(); + + rerender(); + + expect(screen.getByText('Acme')).toBeInTheDocument(); + }); + + it('commits when the freezing update runs inside a transition', async () => { + let setState: (s: { frozen: boolean; label: string }) => void = () => {}; + function Harness() { + const [state, set] = React.useState({ frozen: false, label: 'Acme' }); + setState = set; + return ( +
+ {state.frozen ? 'closed' : 'open'} + {state.label} +
+ ); + } + render(); + + await act(async () => { + React.startTransition(() => setState({ frozen: true, label: 'Globex' })); + await new Promise(r => setTimeout(r, 0)); + }); + + // The suspend-based version stalled the whole transition here: neither the sibling nor + // anything else in the update ever committed. + expect(screen.getByTestId('outside').textContent).toBe('closed'); + expect(screen.getByText('Acme')).toBeInTheDocument(); + }); + + it('lets components inside keep rendering their own state', () => { let bump = () => {}; function Counter() { const [count, setCount] = React.useState(0); @@ -72,23 +100,46 @@ describe('Freeze', () => { act(() => bump()); - expect(screen.getByText('count: 0')).toBeInTheDocument(); + expect(screen.getByText('count: 1')).toBeInTheDocument(); + }); +}); - rerender( +describe('useIsFrozen', () => { + it('reports the nearest Freeze', () => { + function Reader() { + return {useIsFrozen() ? 'frozen' : 'live'}; + } + const { rerender } = render( - + , ); + expect(screen.getByText('live')).toBeInTheDocument(); - expect(screen.getByText('count: 1')).toBeInTheDocument(); + rerender( + + + , + ); + expect(screen.getByText('frozen')).toBeInTheDocument(); }); - it('holds a context change read from inside the frozen subtree', () => { - const NameContext = React.createContext('Acme'); + it('is false outside any Freeze', () => { function Reader() { - return {React.useContext(NameContext)}; + return {useIsFrozen() ? 'frozen' : 'live'}; } + render(); + expect(screen.getByText('live')).toBeInTheDocument(); + }); +}); + +describe('useFrozenValue', () => { + const NameContext = React.createContext('Acme'); + function Reader() { + return {useFrozenValue(React.useContext(NameContext))}; + } + it('holds a context value read from inside the frozen subtree', () => { const { rerender } = render( @@ -116,4 +167,50 @@ describe('Freeze', () => { expect(screen.getByText('Globex')).toBeInTheDocument(); }); + + it('holds a state update raised by the subtree itself', () => { + let bump = () => {}; + function Counter() { + const [count, setCount] = React.useState(0); + bump = () => setCount(n => n + 1); + return count: {useFrozenValue(count)}; + } + + const { rerender } = render( + + + , + ); + rerender( + + + , + ); + + act(() => bump()); + + expect(screen.getByText('count: 0')).toBeInTheDocument(); + + rerender( + + + , + ); + + expect(screen.getByText('count: 1')).toBeInTheDocument(); + }); + + it('passes the value through outside any Freeze', () => { + const { rerender } = render( + + + , + ); + rerender( + + + , + ); + expect(screen.getByText('Globex')).toBeInTheDocument(); + }); }); diff --git a/packages/headless/src/utils/freeze.tsx b/packages/headless/src/utils/freeze.tsx index bdb5469ebf6..5a599ae881f 100644 --- a/packages/headless/src/utils/freeze.tsx +++ b/packages/headless/src/utils/freeze.tsx @@ -2,63 +2,48 @@ import * as React from 'react'; -/** - * Never settles. Throwing it suspends the enclosing boundary indefinitely: React keeps - * rendering the subtree but holds the commit, so the DOM keeps painting its last frame. - */ -const never = new Promise(() => {}); - -function Suspend(): null { - // eslint-disable-next-line @typescript-eslint/only-throw-error -- Suspending is React's thrown-thenable protocol, not an error. `React.use()` would say this more plainly but needs React 19.2; this package supports React 18. - throw never; -} +const FreezeContext = React.createContext(false); export interface FreezeProps { - /** While `true`, the DOM below holds whatever it last committed. */ + /** While `true`, renders the children captured on the last unfrozen render. */ frozen: boolean; children?: React.ReactNode; } /** - * Holds its subtree's DOM at the last committed frame while `frozen`. Renders keep - * happening, they just don't reach the DOM; the pending one commits when `frozen` flips - * back to `false`. + * Holds `children` at the element it received on the last unfrozen render. Everything baked + * into that element — props, conditionals, inline callbacks — is held with it; components inside + * keep rendering, so a value they read through a hook (context, a store) still moves. Wrap such + * a read in `useFrozenValue` to hold it too. * - * Use it to stop content from visibly changing under an exit animation — a popover that - * closes because the thing it was showing changed would otherwise swap its contents on the - * way out. + * Use it to stop content from visibly changing under an exit animation — a popover that closes + * because the thing it was showing changed would otherwise swap its contents on the way out. */ export function Freeze({ frozen, children }: FreezeProps) { - const contentRef = React.useRef(null); - - // Hold onto the node ourselves rather than reading a plain ref: hiding a boundary's children - // detaches their refs, so by the time the effect below runs a normal ref reads `null`. - const setContent = React.useCallback((node: HTMLDivElement | null) => { - if (node) { - contentRef.current = node; - } - }, []); + // Render-phase derived state rather than an effect: React re-runs this render before committing, + // so the DOM never shows a frame the snapshot has not caught up with. + const [snapshot, setSnapshot] = React.useState(children); + if (!frozen && snapshot !== children) { + setSnapshot(children); + } + return {frozen ? snapshot : children}; +} - // React hides a suspended boundary's host children with `display: none !important`, which is - // the opposite of what this is for. Undo it on the commit that applies it: insertion effects - // run after the boundary's mutation and before paint, so the held frame never blinks out. - // `display: contents` is also what the wrapper renders with, so React puts it back on unfreeze - // and the wrapper stays out of the layout it is spliced into. - React.useInsertionEffect(() => { - if (frozen) { - contentRef.current?.style.setProperty('display', 'contents'); - } - }, [frozen]); +/** `true` while the nearest enclosing `Freeze` is holding — for a popup, while it animates out. */ +export function useIsFrozen(): boolean { + return React.useContext(FreezeContext); +} - return ( - - {frozen ? : null} -
- {children} -
- - ); +/** + * Returns `value`, except while the nearest enclosing `Freeze` is holding, when it returns the + * value seen on the last unfrozen render. For content that reads what it shows through a hook + * rather than receiving it as props. + */ +export function useFrozenValue(value: T): T { + const frozen = useIsFrozen(); + const [held, setHeld] = React.useState(value); + if (!frozen && !Object.is(held, value)) { + setHeld(value); + } + return frozen ? held : value; } diff --git a/packages/headless/src/utils/index.ts b/packages/headless/src/utils/index.ts index d839a1690a1..b8106f93eeb 100644 --- a/packages/headless/src/utils/index.ts +++ b/packages/headless/src/utils/index.ts @@ -1,5 +1,5 @@ export { cssVars } from './css-vars'; -export { Freeze, type FreezeProps } from './freeze'; +export { Freeze, type FreezeProps, useFrozenValue, useIsFrozen } from './freeze'; export { isKeyboardEvent, isKeyboardOpen } from './interaction-modality'; export { resetLayoutStyles } from './reset-layout-styles'; export { resolveSideOffset, type SideOffset } from './side-offset'; diff --git a/packages/swingset/src/stories/dialog.component.mdx b/packages/swingset/src/stories/dialog.component.mdx index aaaf023a069..473bed14ad4 100644 --- a/packages/swingset/src/stories/dialog.component.mdx +++ b/packages/swingset/src/stories/dialog.component.mdx @@ -23,6 +23,7 @@ trapping, scroll lock and ARIA wiring come from the primitive. { name: 'open', type: 'boolean' }, { name: 'defaultOpen', type: 'boolean', default: 'false' }, { name: 'onOpenChange', type: '(open: boolean, details: DialogOpenChangeDetails) => void' }, + { name: 'onOpenChangeComplete', type: '(open: boolean) => void' }, { name: 'modal', type: 'boolean', default: 'true' }, { name: 'closedBy', type: "'any' | 'closerequest' | 'none'", default: "'any'" }, ]} @@ -247,8 +248,24 @@ action as its submit — that is how Enter reaches it: ### Exit animations -The popup's contents hold their last frame while it fades out, so state that resets on close (a -machine returning to idle) does not flash through the exit. +The popup keeps rendering what it was handed on the last open render while it fades out, so state +that resets on close (a machine returning to idle, a form clearing) does not flash through the +exit. Two things follow: + +- Reset state in `onOpenChangeComplete`, which fires once the exit has finished, rather than in + `onOpenChange`. A reset there is what the hold protects against, so doing it later is cheaper + than doing it and hiding it. +- The hold covers the JSX you pass. A component inside the popup that reads what it shows through + a hook (a store, a context) keeps updating; wrap that read in `useFrozenValue` from + `@clerk/headless/utils` to hold it too. + +The hold is plain state, so a close issued inside `startTransition` (or a React 19 form action) +commits like any other. + + ## Parts diff --git a/packages/swingset/src/stories/dialog.component.stories.tsx b/packages/swingset/src/stories/dialog.component.stories.tsx index ebebe39f18c..85818c02411 100644 --- a/packages/swingset/src/stories/dialog.component.stories.tsx +++ b/packages/swingset/src/stories/dialog.component.stories.tsx @@ -107,12 +107,7 @@ export function DiscardChanges() { const onOpenChange = useConfirmedClose({ handle: confirm, when: () => value.trim() !== '', - onOpenChange: next => { - setOpen(next); - if (!next) { - setValue(''); - } - }, + onOpenChange: setOpen, confirm: { title: 'Discard changes?', description: 'You have not finished adding this address. It will not be saved.', @@ -127,6 +122,11 @@ export function DiscardChanges() { closedBy='closerequest' open={open} onOpenChange={onOpenChange} + onOpenChangeComplete={next => { + if (!next) { + setValue(''); + } + }} > @@ -569,3 +569,68 @@ export function CustomFocus() { ); } + +/** + * What the popup shows is held through the exit. The parent here resets the "saved" state the + * instant it closes the dialog, the way a machine returning to `idle` would; without the hold, the + * spinner and the typed name would snap back to their blank state under the fade. The second + * button closes from inside `startTransition` — React 19 form actions do the same — so the exit + * has to work as a transition too. + */ +export function ExitHold() { + const [open, setOpen] = React.useState(false); + const [name, setName] = React.useState(''); + const [status, setStatus] = React.useState<'idle' | 'saving' | 'saved'>('idle'); + + const save = (close: (fn: () => void) => void) => { + setStatus('saving'); + setTimeout(() => { + close(() => { + setStatus('saved'); + setOpen(false); + }); + }, 600); + }; + + return ( + { + if (!next) { + setName(''); + setStatus('idle'); + } + }} + > + } /> + + + }>Rename workspace + }> + {status === 'saved' ? 'Saved.' : 'The name is cleared and the status reset once the dialog has closed.'} + + setName(event.target.value)} + /> + }>Cancel + + + + + ); +} diff --git a/packages/ui/src/mosaic/blocks/destructive/destructive.tsx b/packages/ui/src/mosaic/blocks/destructive/destructive.tsx index fa0804dc8f5..6747f44d03e 100644 --- a/packages/ui/src/mosaic/blocks/destructive/destructive.tsx +++ b/packages/ui/src/mosaic/blocks/destructive/destructive.tsx @@ -1,5 +1,5 @@ import type { FormEvent } from 'react'; -import { useEffect, useId, useState } from 'react'; +import { useId, useState } from 'react'; import { Button, SubmitButton } from '../../components/button'; import { Card } from '../../components/card'; @@ -74,14 +74,6 @@ export function Destructive({ const formId = useId(); const [typedValue, setTypedValue] = useState(''); - // The caller may close the dialog without going through the trigger or Cancel, so the - // field is cleared on close rather than in a handler. - useEffect(() => { - if (!open) { - setTypedValue(''); - } - }, [open]); - const isConfirmed = typedValue === confirmationValue; // The action sits in the footer, outside the form, so `form={formId}` associates the two. @@ -100,6 +92,14 @@ export function Destructive({ closedBy='closerequest' open={open} onOpenChange={onOpenChange} + // The caller may close the dialog without going through the trigger or Cancel, so the + // field is cleared on close rather than in a handler — and after the exit animation, so the + // phrase does not empty out under the fade. + onOpenChangeComplete={nextOpen => { + if (!nextOpen) { + setTypedValue(''); + } + }} > {trigger ? : null} diff --git a/packages/ui/src/mosaic/components/dialog/dialog.tsx b/packages/ui/src/mosaic/components/dialog/dialog.tsx index 2959ff4fde7..e18dbe29a68 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.tsx +++ b/packages/ui/src/mosaic/components/dialog/dialog.tsx @@ -153,6 +153,7 @@ function Root({ open, defaultOpen, onOpenChange, + onOpenChangeComplete, modal, children, ...rest @@ -172,6 +173,7 @@ function Root({ open={inline ? true : open} defaultOpen={inline ? undefined : defaultOpen} onOpenChange={inline ? undefined : onOpenChange} + onOpenChangeComplete={inline ? undefined : onOpenChangeComplete} > {children} diff --git a/packages/ui/src/mosaic/user-button/user-button.view.tsx b/packages/ui/src/mosaic/user-button/user-button.view.tsx index da1d2fb09d3..8fb1fcebe0f 100644 --- a/packages/ui/src/mosaic/user-button/user-button.view.tsx +++ b/packages/ui/src/mosaic/user-button/user-button.view.tsx @@ -2,6 +2,7 @@ import { Button as HeadlessButton } from '@clerk/headless/button'; import type { PopoverProps } from '@clerk/headless/popover'; +import { useFrozenValue } from '@clerk/headless/utils'; import * as stylex from '@stylexjs/stylex'; import type { ReactElement, ReactNode } from 'react'; import React from 'react'; @@ -1105,17 +1106,29 @@ export function UserButtonTrigger({ ); } +/** + * The popup's parts read their data through context, so the popup's exit hold (which keeps only + * the elements it was handed) would not cover them: an action that closes the popup as it settles + * — switching workspace — also swaps the data underneath. Re-provide the context held instead. + */ +function HeldUserButtonContext({ children }: { children: ReactNode }): ReactElement { + const data = useFrozenValue(useUserButtonContext()); + return {children}; +} + /** The popover surface: header, organizations, and footer. */ export function UserButtonPopup(): ReactElement { const { renderBranding } = useUserButtonContext(); return ( - -
- -