Skip to content

Commit a0b22cc

Browse files
maxyingerclaude
andauthored
fix(ui): address review feedback on #9427
Publish "still covering" rather than the raw `open` flag from `DialogNestingContext`, so a stacked child keeps `data-stacked` for the length of the parent's exit instead of painting a second scrim over the parent's fading one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent b626033 commit a0b22cc

4 files changed

Lines changed: 62 additions & 7 deletions

File tree

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,9 @@ alert stack, the middle dialog is stacked on one surface while another is stacke
237237
backdrop instead of compositing a darker one per level. `data-stack-base` is for whatever the
238238
surface underneath does to signal depth.
239239

240+
`data-stacked` holds for as long as the dialog underneath is on screen, exit transition included —
241+
otherwise the one on top would paint a second scrim over the fading original.
242+
240243
The headless parts are unstyled. Target a part with your own className (or `render` prop) and combine it with the `data-*` state attributes above.
241244

242245
## Important Notes

packages/headless/src/primitives/dialog/dialog-nesting.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ import { createContext, useCallback, useContext, useLayoutEffect, useMemo, useSt
1111
* floating ancestor but sits on the bare page, and must still paint its own scrim.
1212
*/
1313
export interface DialogNestingContextValue {
14-
/** Whether the surrounding dialog is itself open. */
14+
/**
15+
* Whether the surrounding dialog is still covering the page — open, or closed but still
16+
* mounted for its exit transition. Not the raw `open` flag: a child that un-suppressed its
17+
* backdrop the instant the parent started closing would paint a second scrim over the
18+
* parent's still-fading one.
19+
*/
1520
open: boolean;
1621
/**
1722
* Called by a dialog rendered inside this one, for as long as it is open. Returns the release.
@@ -41,7 +46,7 @@ export interface DialogNesting {
4146
* Joins a dialog root to the stack it belongs to, in both directions: up, to report itself to
4247
* the dialog it renders inside, and down, to count the dialogs that render inside it.
4348
*/
44-
export function useDialogNesting(open: boolean): DialogNesting {
49+
export function useDialogNesting(open: boolean, mounted: boolean): DialogNesting {
4550
const parent = useContext(DialogNestingContext);
4651
const [stackedChildCount, setStackedChildCount] = useState(0);
4752

@@ -70,13 +75,15 @@ export function useDialogNesting(open: boolean): DialogNesting {
7075
return registerWithParent();
7176
}, [open, registerWithParent]);
7277

78+
const covering = open || mounted;
79+
7380
const context = useMemo<DialogNestingContextValue>(
74-
() => ({ open, registerStackedChild }),
75-
[open, registerStackedChild],
81+
() => ({ open: covering, registerStackedChild }),
82+
[covering, registerStackedChild],
7683
);
7784

7885
return {
79-
// A closed parent is not something to sit on top of: the child owns the scrim in that case,
86+
// A parent that is closed AND gone is not something to sit on top of: the child owns the scrim,
8087
// which is what a confirmation root mounted beside its dialog's portal relies on.
8188
isStacked: parent !== null && parent.open,
8289
stackedChildCount,

packages/headless/src/primitives/dialog/dialog-root.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,6 @@ function DialogInner<Payload>(props: DialogProps<Payload> & { isNested: boolean
8989
const [activeTriggerId, setActiveTriggerId] = useControllableState<string | null>(props.triggerId, null);
9090
const [activePayload, setActivePayload] = useState<Payload | undefined>(undefined);
9191

92-
const nesting = useDialogNesting(open);
93-
9492
const labelId = useId();
9593
const descriptionId = useId();
9694

@@ -178,6 +176,10 @@ function DialogInner<Payload>(props: DialogProps<Payload> & { isNested: boolean
178176
ref: popupRef,
179177
});
180178

179+
// Below `useTransition` because it needs `mounted`: what a stacked child has to key off is
180+
// whether this dialog is still on screen, not whether it is still open.
181+
const nesting = useDialogNesting(open, mounted);
182+
181183
const dismiss = useDismiss(floatingContext, {
182184
outsidePressEvent: 'mousedown',
183185
escapeKey: closedBy !== 'none',

packages/headless/src/primitives/dialog/dialog.test.tsx

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,49 @@ describe('Dialog', () => {
745745
expect(screen.getByTestId('outer-popup')).not.toHaveAttribute('data-stack-base');
746746
});
747747

748+
it('stays stacked while the dialog beneath is exiting', () => {
749+
// Keep an animation pending so the one beneath stays mounted for its exit instead of
750+
// unmounting in the same commit.
751+
const original = (Element.prototype as { getAnimations?: unknown }).getAnimations;
752+
(Element.prototype as { getAnimations?: unknown }).getAnimations = () => [
753+
{ finished: new Promise<void>(() => {}) },
754+
];
755+
try {
756+
const { rerender } = renderStack();
757+
758+
// The one beneath closes first. Its backdrop is still on screen for the length of the
759+
// exit, so the one on top has to keep suppressing its own scrim rather than paint a
760+
// second one over the fading original.
761+
rerender(
762+
<Dialog.Root open={false}>
763+
<Dialog.Backdrop data-testid='outer-backdrop' />
764+
<Dialog.Viewport>
765+
<Dialog.Popup data-testid='outer-popup'>
766+
<Dialog.Title>Outer</Dialog.Title>
767+
<Dialog.Root open>
768+
<Dialog.Backdrop data-testid='inner-backdrop' />
769+
<Dialog.Viewport>
770+
<Dialog.Popup data-testid='inner-popup'>
771+
<Dialog.Title>Inner</Dialog.Title>
772+
</Dialog.Popup>
773+
</Dialog.Viewport>
774+
</Dialog.Root>
775+
</Dialog.Popup>
776+
</Dialog.Viewport>
777+
</Dialog.Root>,
778+
);
779+
780+
expect(screen.getByTestId('outer-backdrop')).toBeInTheDocument();
781+
expect(screen.getByTestId('inner-backdrop')).toHaveAttribute('data-stacked', '');
782+
} finally {
783+
if (original) {
784+
(Element.prototype as { getAnimations?: unknown }).getAnimations = original;
785+
} else {
786+
delete (Element.prototype as { getAnimations?: unknown }).getAnimations;
787+
}
788+
}
789+
});
790+
748791
it('is not stacked on a dialog that is closed', () => {
749792
// A confirmation root mounted beside its dialog's portal is inside the root but outlives
750793
// the open state; on its own it owns the scrim like any root-level dialog.

0 commit comments

Comments
 (0)