Skip to content

Commit 9656849

Browse files
feat(headless): hold Popover contents while it closes (#9310)
1 parent 59ee78c commit 9656849

7 files changed

Lines changed: 150 additions & 5 deletions

File tree

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
---
2+
---

packages/headless/src/primitives/popover/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ Middleware stack: `offset` -> `flip` -> `shift` -> `arrow` -> CSS vars. The popu
105105
- **Title and Description are optional but recommended.** They wire `aria-labelledby` and `aria-describedby` to the positioner. If omitted, those attributes are simply absent.
106106
- **Non-modal by default.** Unlike Dialog, the page remains interactive behind the popover. Set `modal={true}` for a stricter focus trap.
107107
- **Nested popovers are supported.** The `FloatingTree` pattern handles nesting automatically.
108+
- **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.
108109

109110
## ARIA
110111

packages/headless/src/primitives/popover/popover-popup.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,22 @@
22

33
import React from 'react';
44

5-
import { type ComponentProps, mergeProps, useRender } from '../../utils';
5+
import { type ComponentProps, Freeze, mergeProps, useRender } from '../../utils';
66
import { usePopoverContext } from './popover-context';
77

88
export type PopoverPopupProps = ComponentProps<'div'>;
99

1010
export const PopoverPopup = React.forwardRef<HTMLDivElement, PopoverPopupProps>(function PopoverPopup(props, ref) {
11-
const { render, ...otherProps } = props;
12-
const { popupRef, transitionProps } = usePopoverContext();
11+
const { render, children, ...otherProps } = props;
12+
const { open, popupRef, transitionProps } = usePopoverContext();
1313

1414
const defaultProps = {
1515
...transitionProps,
16+
// The popup outlives `open` by the length of its exit animation. Whatever closed it has
17+
// usually changed the data behind it (switching account, picking an item), so the contents
18+
// hold their last frame on the way out instead of swapping under the animation. The popup
19+
// element itself stays live, so `data-closed` / `data-ending-style` still land.
20+
children: <Freeze frozen={!open}>{children}</Freeze>,
1621
};
1722

1823
return useRender({
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { cleanup, render, screen } from '@testing-library/react';
2+
import { afterEach, describe, expect, it } from 'vitest';
3+
4+
import { Freeze } from './freeze';
5+
6+
afterEach(() => {
7+
cleanup();
8+
});
9+
10+
describe('Freeze', () => {
11+
it('renders children while not frozen', () => {
12+
render(<Freeze frozen={false}>Acme</Freeze>);
13+
14+
expect(screen.getByText('Acme')).toBeInTheDocument();
15+
});
16+
17+
it('holds the committed DOM when children change while frozen', () => {
18+
const { rerender } = render(<Freeze frozen={false}>Acme</Freeze>);
19+
20+
rerender(<Freeze frozen>Globex</Freeze>);
21+
22+
expect(screen.getByText('Acme')).toBeInTheDocument();
23+
expect(screen.queryByText('Globex')).toBeNull();
24+
});
25+
26+
it('keeps the held DOM visible', () => {
27+
const { rerender } = render(<Freeze frozen={false}>Acme</Freeze>);
28+
29+
rerender(<Freeze frozen>Globex</Freeze>);
30+
31+
expect(screen.getByText('Acme')).toBeVisible();
32+
});
33+
34+
it('keeps the held DOM visible across further updates while frozen', () => {
35+
const { rerender } = render(<Freeze frozen={false}>Acme</Freeze>);
36+
37+
rerender(<Freeze frozen>Globex</Freeze>);
38+
rerender(<Freeze frozen>Initech</Freeze>);
39+
40+
expect(screen.getByText('Acme')).toBeVisible();
41+
});
42+
43+
it('commits the pending children once unfrozen', () => {
44+
const { rerender } = render(<Freeze frozen={false}>Acme</Freeze>);
45+
46+
rerender(<Freeze frozen>Globex</Freeze>);
47+
rerender(<Freeze frozen={false}>Globex</Freeze>);
48+
49+
expect(screen.getByText('Globex')).toBeInTheDocument();
50+
expect(screen.queryByText('Acme')).toBeNull();
51+
});
52+
53+
it('holds state updates raised from inside the frozen subtree', () => {
54+
function Counter({ count }: { count: number }) {
55+
return <span>count: {count}</span>;
56+
}
57+
58+
const { rerender } = render(
59+
<Freeze frozen={false}>
60+
<Counter count={0} />
61+
</Freeze>,
62+
);
63+
64+
rerender(
65+
<Freeze frozen>
66+
<Counter count={1} />
67+
</Freeze>,
68+
);
69+
70+
expect(screen.getByText('count: 0')).toBeInTheDocument();
71+
});
72+
});
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
'use client';
2+
3+
import * as React from 'react';
4+
5+
/**
6+
* Never settles. Throwing it suspends the enclosing boundary indefinitely: React keeps
7+
* rendering the subtree but holds the commit, so the DOM keeps painting its last frame.
8+
*/
9+
const never = new Promise<never>(() => {});
10+
11+
function Suspend(): null {
12+
// 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.
13+
throw never;
14+
}
15+
16+
export interface FreezeProps {
17+
/** While `true`, the DOM below holds whatever it last committed. */
18+
frozen: boolean;
19+
children?: React.ReactNode;
20+
}
21+
22+
/**
23+
* Holds its subtree's DOM at the last committed frame while `frozen`. Renders keep
24+
* happening, they just don't reach the DOM; the pending one commits when `frozen` flips
25+
* back to `false`.
26+
*
27+
* Use it to stop content from visibly changing under an exit animation — a popover that
28+
* closes because the thing it was showing changed would otherwise swap its contents on the
29+
* way out.
30+
*/
31+
export function Freeze({ frozen, children }: FreezeProps) {
32+
const contentRef = React.useRef<HTMLDivElement | null>(null);
33+
34+
// Hold onto the node ourselves rather than reading a plain ref: hiding a boundary's children
35+
// detaches their refs, so by the time the effect below runs a normal ref reads `null`.
36+
const setContent = React.useCallback((node: HTMLDivElement | null) => {
37+
if (node) {
38+
contentRef.current = node;
39+
}
40+
}, []);
41+
42+
// React hides a suspended boundary's host children with `display: none !important`, which is
43+
// the opposite of what this is for. Undo it on the commit that applies it: insertion effects
44+
// run after the boundary's mutation and before paint, so the held frame never blinks out.
45+
// `display: contents` is also what the wrapper renders with, so React puts it back on unfreeze
46+
// and the wrapper stays out of the layout it is spliced into.
47+
React.useInsertionEffect(() => {
48+
if (frozen) {
49+
contentRef.current?.style.setProperty('display', 'contents');
50+
}
51+
}, [frozen]);
52+
53+
return (
54+
<React.Suspense fallback={null}>
55+
{frozen ? <Suspend /> : null}
56+
<div
57+
ref={setContent}
58+
style={{ display: 'contents' }}
59+
>
60+
{children}
61+
</div>
62+
</React.Suspense>
63+
);
64+
}

packages/headless/src/utils/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
export { cssVars } from './css-vars';
2+
export { Freeze, type FreezeProps } from './freeze';
23
export { resetLayoutStyles } from './reset-layout-styles';
34
export {
45
type ComponentProps,

packages/ui/src/mosaic/components/popover/popover.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ describe('Mosaic Popover', () => {
113113
</Popover.Root>,
114114
);
115115

116-
const popup = screen.getByText('Body');
116+
const popup = document.querySelector('.cl-popover-popup');
117117
expect(popup).toHaveClass('cl-popover-popup', 'my-popup');
118118
expect(popup).toHaveStyle({ marginTop: '8px' });
119119
});
@@ -235,6 +235,6 @@ describe('Mosaic Popover', () => {
235235
</Popover.Root>,
236236
);
237237

238-
expect(ref.current).toBe(screen.getByText('Body'));
238+
expect(ref.current).toBe(document.querySelector('.cl-popover-popup'));
239239
});
240240
});

0 commit comments

Comments
 (0)