diff --git a/.changeset/popover-freeze-on-close.md b/.changeset/popover-freeze-on-close.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/popover-freeze-on-close.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/headless/src/primitives/popover/README.md b/packages/headless/src/primitives/popover/README.md index dd2a537d7d0..5e75920e4b1 100644 --- a/packages/headless/src/primitives/popover/README.md +++ b/packages/headless/src/primitives/popover/README.md @@ -105,6 +105,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. ## ARIA diff --git a/packages/headless/src/primitives/popover/popover-popup.tsx b/packages/headless/src/primitives/popover/popover-popup.tsx index 8f8d5b5a13b..64e2bcbd1da 100644 --- a/packages/headless/src/primitives/popover/popover-popup.tsx +++ b/packages/headless/src/primitives/popover/popover-popup.tsx @@ -2,17 +2,22 @@ import React from 'react'; -import { type ComponentProps, mergeProps, useRender } from '../../utils'; +import { type ComponentProps, Freeze, mergeProps, useRender } from '../../utils'; import { usePopoverContext } from './popover-context'; export type PopoverPopupProps = ComponentProps<'div'>; export const PopoverPopup = React.forwardRef(function PopoverPopup(props, ref) { - const { render, ...otherProps } = props; - const { popupRef, transitionProps } = usePopoverContext(); + const { render, children, ...otherProps } = props; + const { open, popupRef, transitionProps } = usePopoverContext(); const defaultProps = { ...transitionProps, + // The popup outlives `open` by the length of its exit animation. Whatever closed it has + // usually changed the data behind it (switching account, picking an item), so the contents + // hold their last frame on the way out instead of swapping under the animation. The popup + // element itself stays live, so `data-closed` / `data-ending-style` still land. + children: {children}, }; return useRender({ diff --git a/packages/headless/src/utils/freeze.test.tsx b/packages/headless/src/utils/freeze.test.tsx new file mode 100644 index 00000000000..b0fb5adf2cd --- /dev/null +++ b/packages/headless/src/utils/freeze.test.tsx @@ -0,0 +1,72 @@ +import { cleanup, render, screen } from '@testing-library/react'; +import { afterEach, describe, expect, it } from 'vitest'; + +import { Freeze } from './freeze'; + +afterEach(() => { + cleanup(); +}); + +describe('Freeze', () => { + it('renders children while not frozen', () => { + render(Acme); + + expect(screen.getByText('Acme')).toBeInTheDocument(); + }); + + it('holds the committed DOM when children change while frozen', () => { + const { rerender } = render(Acme); + + rerender(Globex); + + expect(screen.getByText('Acme')).toBeInTheDocument(); + 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', () => { + const { rerender } = render(Acme); + + rerender(Globex); + rerender(Initech); + + expect(screen.getByText('Acme')).toBeVisible(); + }); + + it('commits the pending children once unfrozen', () => { + const { rerender } = render(Acme); + + rerender(Globex); + rerender(Globex); + + expect(screen.getByText('Globex')).toBeInTheDocument(); + expect(screen.queryByText('Acme')).toBeNull(); + }); + + it('holds state updates raised from inside the frozen subtree', () => { + function Counter({ count }: { count: number }) { + return count: {count}; + } + + const { rerender } = render( + + + , + ); + + rerender( + + + , + ); + + expect(screen.getByText('count: 0')).toBeInTheDocument(); + }); +}); diff --git a/packages/headless/src/utils/freeze.tsx b/packages/headless/src/utils/freeze.tsx new file mode 100644 index 00000000000..bdb5469ebf6 --- /dev/null +++ b/packages/headless/src/utils/freeze.tsx @@ -0,0 +1,64 @@ +'use client'; + +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; +} + +export interface FreezeProps { + /** While `true`, the DOM below holds whatever it last committed. */ + 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`. + * + * 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; + } + }, []); + + // 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]); + + return ( + + {frozen ? : null} +
+ {children} +
+ + ); +} diff --git a/packages/headless/src/utils/index.ts b/packages/headless/src/utils/index.ts index f2a2d4c17c4..87856e7a9c2 100644 --- a/packages/headless/src/utils/index.ts +++ b/packages/headless/src/utils/index.ts @@ -1,4 +1,5 @@ export { cssVars } from './css-vars'; +export { Freeze, type FreezeProps } from './freeze'; export { resetLayoutStyles } from './reset-layout-styles'; export { type ComponentProps, diff --git a/packages/ui/src/mosaic/components/popover/popover.test.tsx b/packages/ui/src/mosaic/components/popover/popover.test.tsx index 74178a2e6f9..68abbd3cd22 100644 --- a/packages/ui/src/mosaic/components/popover/popover.test.tsx +++ b/packages/ui/src/mosaic/components/popover/popover.test.tsx @@ -113,7 +113,7 @@ describe('Mosaic Popover', () => { , ); - const popup = screen.getByText('Body'); + const popup = document.querySelector('.cl-popover-popup'); expect(popup).toHaveClass('cl-popover-popup', 'my-popup'); expect(popup).toHaveStyle({ marginTop: '8px' }); }); @@ -235,6 +235,6 @@ describe('Mosaic Popover', () => { , ); - expect(ref.current).toBe(screen.getByText('Body')); + expect(ref.current).toBe(document.querySelector('.cl-popover-popup')); }); });