Skip to content

Commit 094939a

Browse files
wobsorianowobsoriano
authored
Merge branch 'main' into mike/expo-native-package
2 parents 12061bb + 3b4ceb6 commit 094939a

11 files changed

Lines changed: 272 additions & 46 deletions

File tree

‎.changeset/localization-modifier-quoted-params.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/ui': patch
3+
---
4+
5+
Fix localization strings whose `link("…")` labels contain apostrophes, quotes, commas or parentheses. Previously, a custom label such as `link("Conditions d'utilisation")` rendered as "Conditions dutilisation"; quoted labels now render exactly as written.

‎.changeset/mosaic-dialog-outside-press.md‎

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

‎packages/mosaic/src/components/card/card.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,11 @@ const Header = React.forwardRef<HTMLDivElement, MosaicComponentProps<'div'>>(fun
124124
<>
125125
{/* First in the DOM, so it is the first tabbable element and takes the dialog's opening
126126
focus — the same reason `Dialog.CloseButton` is a part rather than a popup flag.
127-
Not outside a dialog, where there is nothing to close, and not in an alert dialog. */}
127+
Not outside a dialog, where there is nothing to close, and not where the dialog's
128+
`showCloseButton` is off, which is the default for an alert dialog. */}
128129
{/* An alert dialog interrupts to ask for a decision, and a corner X is a way out
129130
without answering one. The cancel action in the footer is the way out. */}
130-
{isInDialog(dialog) && dialog.role !== 'alertdialog' ? <HeaderCloseButton /> : null}
131+
{isInDialog(dialog) && dialog.showCloseButton ? <HeaderCloseButton /> : null}
131132
<div {...mergeStyleProps(themeProps('card-header-content'), stylex.props(reset.base, slots.header.content))}>
132133
{children}
133134
</div>

‎packages/mosaic/src/components/dialog/__tests__/alert-dialog.test.tsx‎

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,10 @@ const settle = () =>
1515
await new Promise(resolve => setTimeout(resolve, 0));
1616
});
1717

18-
function Confirm({ onOpenChange, ...rest }: Partial<DialogRootProps> = {}) {
18+
function Confirm({ onOpenChange }: Pick<DialogRootProps, 'onOpenChange'> = {}) {
1919
return (
2020
<Dialog.Root
2121
defaultOpen
22-
{...rest}
2322
role='alertdialog'
2423
onOpenChange={onOpenChange}
2524
>
@@ -375,3 +374,86 @@ describe('stacked on another dialog', () => {
375374
warn.mockRestore();
376375
});
377376
});
377+
378+
describe('showCloseButton', () => {
379+
function Verify({ showCloseButton }: { showCloseButton?: boolean }) {
380+
return (
381+
<Dialog.Root
382+
defaultOpen
383+
role='alertdialog'
384+
showCloseButton={showCloseButton}
385+
>
386+
<Dialog.Popup>
387+
<Card.Root elevation='overlay'>
388+
<Card.Header>
389+
<Card.Title>Verify it is you</Card.Title>
390+
<Card.Description>Enter the code sent to your email.</Card.Description>
391+
</Card.Header>
392+
</Card.Root>
393+
</Dialog.Popup>
394+
</Dialog.Root>
395+
);
396+
}
397+
398+
it('renders the card header dismiss in an alert dialog that opts in', async () => {
399+
const user = userEvent.setup();
400+
render(<Verify showCloseButton />);
401+
402+
await user.click(screen.getByRole('button', { name: 'Close' }));
403+
404+
expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument();
405+
});
406+
407+
it('shows and hides the dismiss as the prop changes while open', () => {
408+
const { rerender } = render(<Verify />);
409+
expect(screen.queryByRole('button', { name: 'Close' })).not.toBeInTheDocument();
410+
411+
rerender(<Verify showCloseButton />);
412+
expect(screen.getByRole('button', { name: 'Close' })).toBeInTheDocument();
413+
414+
rerender(<Verify showCloseButton={false} />);
415+
expect(screen.queryByRole('button', { name: 'Close' })).not.toBeInTheDocument();
416+
});
417+
418+
it('hides the card header dismiss in a plain dialog that opts out', () => {
419+
render(
420+
<Dialog.Root
421+
defaultOpen
422+
showCloseButton={false}
423+
>
424+
<Dialog.Popup>
425+
<Card.Root elevation='overlay'>
426+
<Card.Header>
427+
<Card.Title>Add email address</Card.Title>
428+
</Card.Header>
429+
</Card.Root>
430+
</Dialog.Popup>
431+
</Dialog.Root>,
432+
);
433+
434+
expect(screen.queryByRole('button', { name: 'Close' })).not.toBeInTheDocument();
435+
});
436+
437+
it('does not warn about Dialog.CloseButton in an alert dialog that opts in', async () => {
438+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
439+
render(
440+
<Dialog.Root
441+
defaultOpen
442+
role='alertdialog'
443+
showCloseButton
444+
>
445+
<Dialog.Popup>
446+
<Dialog.CloseButton />
447+
<Card.Root elevation='overlay'>
448+
<Card.Title>Verify it is you</Card.Title>
449+
<Card.Description>Enter the code sent to your email.</Card.Description>
450+
</Card.Root>
451+
</Dialog.Popup>
452+
</Dialog.Root>,
453+
);
454+
await settle();
455+
456+
expect(warn).not.toHaveBeenCalled();
457+
warn.mockRestore();
458+
});
459+
});

‎packages/mosaic/src/components/dialog/__tests__/dialog.test.tsx‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,57 @@ describe('Mosaic Dialog', () => {
164164
});
165165
});
166166

167+
describe('shouldCloseOnInteractOutside', () => {
168+
function Verify({ shouldCloseOnInteractOutside }: { shouldCloseOnInteractOutside?: boolean }) {
169+
return (
170+
<Dialog.Root
171+
defaultOpen
172+
shouldCloseOnInteractOutside={shouldCloseOnInteractOutside}
173+
>
174+
<Dialog.Popup>
175+
<Surface title='Verify it is you' />
176+
</Dialog.Popup>
177+
</Dialog.Root>
178+
);
179+
}
180+
181+
it('closes on an outside press by default', async () => {
182+
const user = userEvent.setup();
183+
render(<Verify />);
184+
185+
await user.click(document.querySelector('.cl-dialog-backdrop') as HTMLElement);
186+
187+
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
188+
});
189+
190+
it('stays open on an outside press when false', async () => {
191+
const user = userEvent.setup();
192+
render(<Verify shouldCloseOnInteractOutside={false} />);
193+
194+
await user.click(document.querySelector('.cl-dialog-backdrop') as HTMLElement);
195+
196+
expect(screen.getByRole('dialog', { name: 'Verify it is you' })).toBeInTheDocument();
197+
});
198+
199+
it('still closes on Escape when false', async () => {
200+
const user = userEvent.setup();
201+
render(<Verify shouldCloseOnInteractOutside={false} />);
202+
203+
await user.keyboard('{Escape}');
204+
205+
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
206+
});
207+
208+
it('keeps the corner dismiss when false', async () => {
209+
const user = userEvent.setup();
210+
render(<Verify shouldCloseOnInteractOutside={false} />);
211+
212+
await user.click(screen.getByRole('button', { name: 'Close' }));
213+
214+
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
215+
});
216+
});
217+
167218
// A `profile` dialog (account profile) opening a `card` dialog (add an email address) is a real
168219
// shape, so the `FloatingTree` nesting the headless README claims is exercised here rather than
169220
// assumed. Dismissal must reach the topmost dialog only, and the body must stay locked until the

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

Lines changed: 52 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,14 @@ export interface DialogContextValue {
7474
* the one way out an alert must not offer.
7575
*/
7676
role: DialogRole;
77+
/** Whether `Card.Header` renders its dismiss. From `showCloseButton` on the root. */
78+
showCloseButton: boolean;
7779
}
7880

7981
export const DialogContext = React.createContext<DialogContextValue | null>(null);
8082

83+
const ShowCloseButtonContext = React.createContext(true);
84+
8185
/**
8286
* In a dialog, as opposed to standalone. A surface that can be rendered either way — `Card`,
8387
* `Profile` — reads this to take the dialog's dismiss and its overlay treatment, without knowing
@@ -136,32 +140,55 @@ export interface DialogPopupProps extends MosaicComponentProps<'div'> {
136140
type DialogRootBaseProps<Payload> = Omit<HeadlessDialogProps<Payload>, 'role' | 'closedBy'>;
137141

138142
/**
139-
* `role` is the whole dismissal API: it decides how the dialog announces itself AND what closes it,
140-
* with no prop to contradict either.
143+
* `role` decides how the dialog announces itself; `shouldCloseOnInteractOutside` decides whether
144+
* an outside press closes it; `showCloseButton` decides whether the surface renders its dismiss.
145+
* Escape always closes.
141146
*
142147
* - `alertdialog` announces as an interruption rather than as a surface the user navigated to, and
143-
* cannot be dismissed by an outside press. A dialog asking a question it needs an answer to must
144-
* not be answerable by clicking next to it. Escape still closes, which is the keyboard's
145-
* equivalent of the cancel button that is always present;
146-
* - `dialog` dismisses on an outside press as well.
148+
* never closes on an outside press. A dialog asking a question it needs an answer to must not be
149+
* answerable by clicking next to it. Escape is the keyboard's equivalent of the cancel button
150+
* that is always present;
151+
* - `dialog` closes on an outside press unless `shouldCloseOnInteractOutside` is `false`, for a
152+
* task an accidental press would throw away, such as entering a verification code.
147153
*
148154
* It says nothing about the variant: an alert is a `Card` like every other dialog.
149155
*/
150-
export type DialogRootProps<Payload = unknown> = DialogRootBaseProps<Payload> & {
151-
/** The popup's ARIA role, and with it what dismisses the dialog. @default 'dialog' */
152-
role?: DialogRole;
153-
};
156+
export type DialogRootProps<Payload = unknown> = DialogRootBaseProps<Payload> &
157+
(
158+
| {
159+
/** The popup's ARIA role. @default 'dialog' */
160+
role?: 'dialog';
161+
/** Whether an outside press closes the dialog. @default true */
162+
shouldCloseOnInteractOutside?: boolean;
163+
/** Whether the surface renders its dismiss. @default true */
164+
showCloseButton?: boolean;
165+
}
166+
| {
167+
role: 'alertdialog';
168+
shouldCloseOnInteractOutside?: false;
169+
/** Whether the surface renders its dismiss, for a step past the question. @default false */
170+
showCloseButton?: boolean;
171+
}
172+
);
154173

155174
/** Owns the open state and the decisions — role, dismissal — every part reads. */
156-
function Root<Payload = unknown>({ role = 'dialog', children, ...rest }: DialogRootProps<Payload>) {
175+
function Root<Payload = unknown>({
176+
role = 'dialog',
177+
shouldCloseOnInteractOutside = role === 'dialog',
178+
showCloseButton = role === 'dialog',
179+
children,
180+
...rest
181+
}: DialogRootProps<Payload>) {
157182
return (
158-
<Primitive.Root<Payload>
159-
{...rest}
160-
role={role}
161-
closedBy={role === 'alertdialog' ? 'closerequest' : 'any'}
162-
>
163-
{children}
164-
</Primitive.Root>
183+
<ShowCloseButtonContext.Provider value={showCloseButton}>
184+
<Primitive.Root<Payload>
185+
{...rest}
186+
role={role}
187+
closedBy={shouldCloseOnInteractOutside ? 'any' : 'closerequest'}
188+
>
189+
{children}
190+
</Primitive.Root>
191+
</ShowCloseButtonContext.Provider>
165192
);
166193
}
167194

@@ -191,8 +218,8 @@ const Close = React.forwardRef<HTMLButtonElement, DialogCloseProps>(function Dia
191218
});
192219

193220
/**
194-
* Warns when the corner dismiss is rendered inside an alert dialog, where a corner X is a way out
195-
* without answering the question.
221+
* Warns when the corner dismiss is rendered inside an alert dialog that has not opted in with
222+
* `showCloseButton`, where a corner X is a way out without answering the question.
196223
*/
197224
function useCloseButtonWarning(isAlert: boolean) {
198225
React.useEffect(() => {
@@ -219,9 +246,10 @@ const CloseButton = React.forwardRef<HTMLButtonElement, DialogCloseButtonProps>(
219246
ref,
220247
) {
221248
const surface = React.useContext(DialogContext);
249+
const showCloseButton = React.useContext(ShowCloseButtonContext);
222250
const { role } = useHeadlessDialogContext();
223251
const variant = surface?.variant ?? 'card';
224-
useCloseButtonWarning(role === 'alertdialog');
252+
useCloseButtonWarning(role === 'alertdialog' && !showCloseButton);
225253
return (
226254
<span {...stylex.props(styles.closeButton, closeInsets[variant])}>
227255
<Primitive.Close
@@ -362,15 +390,16 @@ const Popup = React.forwardRef<HTMLDivElement, DialogPopupProps>(function Dialog
362390
// The headless flag, not the stack check below — the variant rule is about opening a dialog inside
363391
// ANY dialog, which is broader than the card-on-card case the stacking styles cover.
364392
const { role, isStacked: isNestedInDialog, labelId, descriptionId } = useHeadlessDialogContext();
393+
const showCloseButton = React.useContext(ShowCloseButtonContext);
365394
const isAlert = role === 'alertdialog';
366395
// A profile has its own compact-band treatment and takes no placement; the warning says so.
367396
const compactPlacement: DialogCompactPlacement = variant === 'profile' ? 'center' : (compactPlacementProp ?? 'sheet');
368397
useCompactPlacementWarning(variant, compactPlacementProp);
369398
useNestedVariantWarning(isNestedInDialog, variant);
370399

371400
const surface = React.useMemo(
372-
() => ({ labelId, descriptionId, variant, role }),
373-
[labelId, descriptionId, variant, role],
401+
() => ({ labelId, descriptionId, variant, role, showCloseButton }),
402+
[labelId, descriptionId, variant, role, showCloseButton],
374403
);
375404
// Observed through state rather than a plain ref, because the warnings have to re-run when the
376405
// node arrives and a ref mutation does not re-render.

‎packages/mosaic/src/components/profile/profile.test.tsx‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,23 @@ describe('Profile', () => {
596596
expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument();
597597
});
598598

599+
it('follows showCloseButton on the dialog', () => {
600+
render(
601+
<MosaicProvider>
602+
<Dialog.Root
603+
defaultOpen
604+
showCloseButton={false}
605+
>
606+
<Dialog.Popup variant='profile'>
607+
<Surface />
608+
</Dialog.Popup>
609+
</Dialog.Root>
610+
</MosaicProvider>,
611+
);
612+
613+
expect(screen.queryByRole('button', { name: 'Close', exact: true })).not.toBeInTheDocument();
614+
});
615+
599616
it('carries no dismiss standalone', () => {
600617
renderSurface();
601618

‎packages/mosaic/src/components/profile/profile.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ const Root = React.forwardRef<HTMLDivElement, ProfileRootProps>(function Profile
150150
{...mergeStyleProps(themeProps('profile-sentinel'), stylex.props(reset.base, styles.sentinel))}
151151
/>
152152
{/* First in the DOM so it takes the dialog's opening focus, as in `Card.Header`. */}
153-
{isInDialog(dialog) && dialog.role !== 'alertdialog' ? <Dialog.CloseButton /> : null}
153+
{isInDialog(dialog) && dialog.showCloseButton ? <Dialog.CloseButton /> : null}
154154
<div
155155
{...mergeStyleProps(
156156
themeProps('profile-layout'),

‎packages/swingset/src/stories/dialog.component.mdx‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import * as DialogStories from './dialog.component.stories';
1313
meta={DialogStories.meta}
1414
extra={[
1515
{ name: 'role', type: "'dialog' | 'alertdialog'", default: "'dialog'" },
16+
{ name: 'shouldCloseOnInteractOutside', type: 'boolean', default: "true (always false for 'alertdialog')" },
17+
{ name: 'showCloseButton', type: 'boolean', default: "true for 'dialog', false for 'alertdialog'" },
1618
{ name: 'open', type: 'boolean' },
1719
{ name: 'defaultOpen', type: 'boolean', default: 'false' },
1820
{ name: 'onOpenChange', type: '(open: boolean, details: DialogOpenChangeDetails) => void' },
@@ -138,7 +140,8 @@ What follows from the role:
138140
- No outside-press dismissal; Escape still cancels.
139141
- A name **and** a description are required — `Card.Title` and `Card.Description`; both warn in dev
140142
when missing.
141-
- `Card.Header` withholds its corner dismiss, and `Dialog.CloseButton` warns.
143+
- `Card.Header` withholds its corner dismiss, and `Dialog.CloseButton` warns, unless the root sets
144+
`showCloseButton`.
142145

143146
### Confirming a discard
144147

@@ -173,14 +176,15 @@ the DOM. Point `initialFocus` on `Dialog.Popup` at the field that should have it
173176

174177
### Dismissal
175178

176-
`role` decides what closes the dialog. There is no prop for it.
179+
Escape always closes. `shouldCloseOnInteractOutside={false}` keeps a `dialog` open on an outside
180+
press, for a task an accidental press would throw away, such as entering a verification code. An
181+
`alertdialog` never closes on an outside press.
177182

178-
| Role | Escape | Outside press | Programmatic |
179-
| ------------- | ------ | ------------- | ------------ |
180-
| `dialog` | ✅ | ✅ | ✅ |
181-
| `alertdialog` | ✅ | ❌ | ✅ |
182-
183-
The two map to the native [`<dialog closedby>`](https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/closedBy) values `any` and `closerequest`.
183+
| Root | Escape | Outside press | Programmatic |
184+
| ---------------------------------------------------- | ------ | ------------- | ------------ |
185+
| `role='dialog'` | ✅ | ✅ | ✅ |
186+
| `role='dialog' shouldCloseOnInteractOutside={false}` | ✅ | ❌ | ✅ |
187+
| `role='alertdialog'` | ✅ | ❌ | ✅ |
184188

185189
### Keyboard
186190

0 commit comments

Comments
 (0)