Skip to content

Commit cf6632f

Browse files
maxyingerclaude
andauthored
fix(ui): address review feedback on #9433
Scope `DialogPartNameContext` to the alert's popup, so a plain `Dialog` nested inside one no longer inherits the name, and share `Dialog`'s content resolver instead of copying it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2bab0ba commit cf6632f

2 files changed

Lines changed: 30 additions & 30 deletions

File tree

packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { useDialogContext } from '@clerk/headless/dialog';
21
import { useRender } from '@clerk/headless/utils';
32
import * as stylex from '@stylexjs/stylex';
43
import type { ReactNode } from 'react';
@@ -18,9 +17,9 @@ import type {
1817
DialogViewportProps,
1918
} from '../dialog';
2019
import { Dialog } from '../dialog';
21-
// Deep import: the part-name context is how one Mosaic component wraps another and is
22-
// deliberately absent from `../dialog`'s public surface.
23-
import { DialogPartNameContext } from '../dialog/dialog';
20+
// Deep import: the part-name context and the content resolver are how one Mosaic component wraps
21+
// another and are deliberately absent from `../dialog`'s public surface.
22+
import { DialogContent, DialogPartNameContext } from '../dialog/dialog';
2423
import { reset } from '../reset.styles';
2524
import { styles } from './alert-dialog.styles';
2625

@@ -50,16 +49,14 @@ export type AlertDialogActionsProps = MosaicComponentProps<'div'>;
5049
/** Owns the open state, and pins the three props that make a dialog an alert dialog. */
5150
function Root<Payload = unknown>({ children, ...rest }: AlertDialogRootProps<Payload>) {
5251
return (
53-
<DialogPartNameContext.Provider value='AlertDialog'>
54-
<Dialog.Root<Payload>
55-
{...rest}
56-
role='alertdialog'
57-
closedBy='closerequest'
58-
size='prompt'
59-
>
60-
{children}
61-
</Dialog.Root>
62-
</DialogPartNameContext.Provider>
52+
<Dialog.Root<Payload>
53+
{...rest}
54+
role='alertdialog'
55+
closedBy='closerequest'
56+
size='prompt'
57+
>
58+
{children}
59+
</Dialog.Root>
6360
);
6461
}
6562

@@ -88,11 +85,16 @@ const Popup = React.forwardRef<HTMLDivElement, AlertDialogPopupProps>(function A
8885
[ref],
8986
);
9087

88+
// Scoped to the popup rather than to the whole root: this is the only place the name is read,
89+
// and a plain `Dialog` nested inside an alert would otherwise inherit it and have its own
90+
// warnings name `AlertDialog` parts that do not exist at that call site.
9191
return (
92-
<Dialog.Popup
93-
ref={mergedRef}
94-
{...props}
95-
/>
92+
<DialogPartNameContext.Provider value='AlertDialog'>
93+
<Dialog.Popup
94+
ref={mergedRef}
95+
{...props}
96+
/>
97+
</DialogPartNameContext.Provider>
9698
);
9799
});
98100

@@ -139,16 +141,6 @@ export interface AlertDialogProps
139141
children: ReactNode | ((ctx: { close: () => void }) => ReactNode);
140142
}
141143

142-
function AlertDialogContent({ children }: { children: AlertDialogProps['children'] }) {
143-
const { setOpen } = useDialogContext();
144-
if (typeof children !== 'function') {
145-
return <>{children}</>;
146-
}
147-
// Routed through the primitive's close funnel, so a controlled consumer's `onOpenChange` sees
148-
// this close the same as Escape does — and can decline it.
149-
return <>{children({ close: () => setOpen(false) })}</>;
150-
}
151-
152144
/**
153145
* Mosaic `AlertDialog` — a `Dialog` that interrupts to ask for a decision, and waits for one.
154146
*
@@ -203,7 +195,7 @@ export function AlertDialog({
203195
initialFocus={initialFocus}
204196
finalFocus={finalFocus}
205197
>
206-
<AlertDialogContent>{children}</AlertDialogContent>
198+
<DialogContent>{children}</DialogContent>
207199
</Popup>
208200
</Dialog.Viewport>
209201
</Dialog.Portal>

packages/ui/src/mosaic/components/dialog/dialog.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,15 @@ export interface DialogProps extends Pick<
315315
size?: DialogSize;
316316
}
317317

318-
function DialogContent({ children }: { children: DialogProps['children'] }) {
318+
/**
319+
* Resolves the render-prop form of `children`. Shared with `AlertDialog`, which offers the same
320+
* `close` contract and would otherwise carry a second implementation of it. Not exported from the
321+
* folder's `index.ts`, for the same reason as {@link DialogPartNameContext}.
322+
*
323+
* Routed through the primitive's close funnel, so a controlled consumer's `onOpenChange` sees this
324+
* close the same as Escape does — and can decline it.
325+
*/
326+
export function DialogContent({ children }: { children: DialogProps['children'] }) {
319327
const { setOpen } = useDialogContext();
320328
if (typeof children !== 'function') {
321329
return <>{children}</>;

0 commit comments

Comments
 (0)