feat(ui): rebuild the Mosaic Dialog on StyleX and add composition APIs - #9388
feat(ui): rebuild the Mosaic Dialog on StyleX and add composition APIs#9388maxyinger wants to merge 2 commits into
Conversation
Moves the dialog off the Emotion slot-recipe engine onto StyleX, leaving `tabs` as the last component on the old path, and reworks its sizing, motion and mobile behaviour on top of that. `size` becomes three named surfaces — `prompt`, `card`, `panel` — and moves to `Dialog.Root`, since the backdrop reads it too. The gap to the screen edge is a fixed inset at three breakpoints rather than a percentage, which is what makes the surround an even frame. A `panel` clips and carries no padding, so its scroll region is composed inside it from the ScrollArea atoms; that keeps the close button anchored and makes a sidebar a plain flex row. Below 48rem a `prompt` becomes a bottom sheet, and `Dialog.Viewport` measures the on-screen keyboard so the sheet rises above it while a card re-centres and a panel shrinks. The chrome of a mobile browser is tinted to match the scrim, derived from the backdrop rather than shipped as a colour, refcounted across stacked dialogs and reverting exactly. Adds `Dialog.CloseButton`, `data-nested` for stacked scrims, and `--cl-dialog-origin` so a dialog scales out of whatever opened it. Also fixes a transition that never ran: it was keyed to a `data-cl-starting-style` attribute the headless layer does not emit, so dialogs appeared with no animation at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Dialog.createHandle()` returns a handle passed to both a `Dialog.Trigger`
and a `Dialog.Root`, so a trigger drives a dialog it is not nested under.
The handle also exposes imperative `open()` / `close()` / `isOpen`.
Several triggers can share one dialog, each carrying an `id` and a
`payload`, with the root's children as a function of `{ payload }` so one
dialog renders per-trigger content. Everything keyed to "the trigger" now
follows the one actually used — the dialog scales out of it and returns
focus to it — and `triggerId` names the active trigger in controlled mode,
which also gives controlled, trigger-less dialogs the origin-aware open.
`initialFocus` and `finalFocus` on `Dialog.Popup` take `true`, `false`, a
ref, or a function of the interaction type behind the change. Defaults are
unchanged: first tabbable on open, the trigger on close, except after a
pointer-driven dismissal.
Also retunes the dialog for dark mode. The scrim veils rather than darkens
there — light grey at low alpha over a dark page, against black over a
light one — so the two schemes are unrelated colours rather than one
colour at two opacities. The nested scrim stays solved rather than picked,
landing two levels on the same proportional deepening as light. The popup
shadow becomes one three-layer shadow shared by both schemes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: af5f90d The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
API Changes Report
Summary
🔴 Breaking changes index (64)Every breaking change, up front. Full diffs are in the package sections below.
@clerk/sharedVersion: 4.28.1 → 4.27.1 Subpath
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (37)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughThe PR replaces the Mosaic Dialog implementation with a headless-backed compound API. It adds detached triggers, typed payloads, imperative handles, trigger metadata, dismissal modes, configurable focus targets, nested-dialog state, StyleX styling, keyboard insets, browser-chrome synchronization, and Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (10)
packages/ui/src/mosaic/components/dialog/keyboard-inset.ts (1)
75-82: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake the release function idempotent.
Each call to the returned function decrements
listeners. A second call on the same release drives the count below zero. The counter then never returns to0, and the listeners plus the--_cl-keyboard-insetproperty stay attached for the lifetime of the page.acquireBrowserChromeinbrowser-chrome.tsguards this case with areleasedflag; this module does not.♻️ Proposed guard
- return () => { - listeners--; - if (listeners === 0 && detach) { - detach(); - detach = null; - } - }; + let released = false; + return () => { + if (released) { + return; + } + released = true; + listeners--; + if (listeners === 0 && detach) { + detach(); + detach = null; + } + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/dialog/keyboard-inset.ts` around lines 75 - 82, Make the release function returned by the keyboard-inset acquisition flow idempotent by adding a per-release guard, similar to acquireBrowserChrome’s released flag. Only decrement listeners and detach the keyboard-inset listener/property when that release has not already been executed.packages/ui/src/mosaic/components/dialog/dialog.tsx (1)
205-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
renderinrestsilently replaces the styled Button.
CloseButtonsetsrenderbefore{...rest}. If a consumer passesrender, their element replaces theButtonwrapper, andstyles.closeButtonpluscloseInsets[size]are lost. The button then loses its absolute anchoring. Consider omittingrenderfromDialogCloseButtonProps, or documenting that the override must supply its own positioning.♻️ Proposed type change
-export interface DialogCloseButtonProps extends MosaicComponentProps<'button'> { +export interface DialogCloseButtonProps extends Omit<MosaicComponentProps<'button'>, 'render'> {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/dialog/dialog.tsx` around lines 205 - 229, Prevent consumers from overriding the internal render used by CloseButton: omit render from DialogCloseButtonProps and exclude it from the rest props spread, preserving the styled Button with styles.closeButton and closeInsets[size].packages/ui/src/mosaic/components/dialog/dialog.styles.ts (1)
199-232: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTrim the duplicated and oversized comments in
sizes.panel.Two blocks state the same fact. Lines 214-223 explain that the panel fills the viewport content box with
stretch, and lines 224-228 repeat it. Reduce the block to a single terse note. The same applies across this file, where multi-paragraph rationale blocks dominate the style declarations.The coding guidelines require minimal comments: "Add comments only when critical to explain why a non-obvious change was made; never restate code behavior, and keep warranted comments to one terse line rather than a verbose multi-line block."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/dialog/dialog.styles.ts` around lines 199 - 232, Trim the duplicated rationale comments in sizes.panel, especially the repeated explanation around alignSelf: 'stretch', leaving one concise line only where the non-obvious layout decision requires justification. Apply the same minimal-comment standard to nearby oversized rationale blocks in this file without changing the style declarations.Source: Coding guidelines
packages/ui/src/mosaic/components/dialog/browser-chrome.ts (2)
160-190: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe comment describes a binary search; the code performs a linear scan.
makeEasingwalks the table withwhile (lo < SAMPLES && table[lo + 1] < x) lo++. That is a linear scan, not a binary search. Correct the comment or implement the search that it describes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/dialog/browser-chrome.ts` around lines 160 - 190, Update makeEasing so its lookup uses a binary search over the monotonic table rather than incrementing lo through entries linearly; preserve the existing interpolation and axis calculation behavior.
229-257: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe
acquireBrowserChromedoc block is attached toresolveTint.Lines 229-237 document
acquireBrowserChromeand its@param backdrop. A second doc block forresolveTintfollows at lines 238-243, and thefunction resolveTintdeclaration follows that. TypeScript and editors therefore associate the first block with nothing, andacquireBrowserChromeat line 259 has no JSDoc. Move the first block directly aboveexport function acquireBrowserChrome.The coding guidelines require that "All public APIs must be documented with JSDoc".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/dialog/browser-chrome.ts` around lines 229 - 257, Move the first JSDoc block describing the refcounted backdrop behavior and its backdrop parameter from above resolveTint to directly above export function acquireBrowserChrome. Keep the separate resolveTint documentation attached to resolveTint, ensuring the public acquireBrowserChrome API retains its documentation.Source: Coding guidelines
packages/ui/src/mosaic/styles/index.ts (1)
17-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRe-export the headless dialog types that the exported prop types reference.
packages/ui/src/mosaic/components/dialog/index.tsalso exportsDialogFocusTarget,DialogHandle, andDialogOpenChangeDetails. This barrel omits them.DialogRootPropsandDialogTriggerPropscarry ahandle?: DialogHandle<Payload>member, andDialogPopupPropscarriesinitialFocus/finalFocusof typeDialogFocusTarget. A consumer of this entry point can therefore pass those props but cannot name their types.♻️ Proposed addition
export { Dialog } from '../components/dialog'; export type { DialogBackdropProps, DialogCloseButtonProps, DialogCloseProps, DialogDescriptionProps, + DialogFocusTarget, + DialogHandle, + DialogOpenChangeDetails, DialogPopupProps, DialogProps, DialogRootProps, DialogSize, DialogTitleProps, DialogTriggerProps, DialogViewportProps, } from '../components/dialog';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/styles/index.ts` around lines 17 - 30, Update the dialog type re-exports in the styles barrel to include DialogFocusTarget, DialogHandle, and DialogOpenChangeDetails from the existing dialog component exports, alongside the current DialogRootProps, DialogTriggerProps, and DialogPopupProps types.packages/headless/src/primitives/dialog/dialog.test.tsx (1)
385-455: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for a
Dialog.Triggerwith neither a root nor a handle.
dialog-trigger.tsxline 36 throws a documented error for this case. No test covers it. The test also exposes the hook-order problem flagged inpackages/headless/src/primitives/dialog/dialog-trigger.tsxlines 33-37, because React reports a hook-count error instead of the intended message once a store disappears between renders.💚 Proposed test
+ it('throws when the trigger has neither a root nor a handle', () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render(<Dialog.Trigger>Orphan</Dialog.Trigger>)).toThrow( + /must be nested in a <Dialog.Root> or given a `handle`/, + ); + consoleError.mockRestore(); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/dialog/dialog.test.tsx` around lines 385 - 455, Add a test in the detached-trigger describe block that renders Dialog.Trigger without a Dialog.Root or handle and asserts the documented error from Dialog.Trigger. Include a rerender or unmount scenario where the associated store disappears to verify stable hook ordering and ensure the intended error is reported instead of a React hook-count error.Source: Coding guidelines
packages/headless/src/primitives/dialog/dialog-root.tsx (1)
121-143: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConfirm the stability assumptions in the
setRooteffect.The effect only re-runs when
storechanges.openFromTriggerandcloseFromTriggercapturerefs,floatingContext, andsetActiveTriggerIdfrom the render that attached the controller.setActiveTriggerIdcomes fromuseControllableState, whose setter identity depends onisControlled. If a consumer switchestriggerIdbetweenundefinedand a value after mount, the captured setter becomes stale and trigger attribution stops updating.applyOpenChangealready avoids this through thelatestref; consider routingsetActiveTriggerIdandsetActivePayloadthrough the same ref.♻️ Proposed change to route trigger state through the latest ref
- const latest = useRef({ applyOpenChange, activeTriggerId }); + const latest = useRef({ applyOpenChange, activeTriggerId, setActiveTriggerId, setActivePayload }); useLayoutEffect(() => { - latest.current = { applyOpenChange, activeTriggerId }; + latest.current = { applyOpenChange, activeTriggerId, setActiveTriggerId, setActivePayload }; }); useLayoutEffect(() => { return store.setRoot({ openFromTrigger: (id, event) => { const registration = store.getTrigger(id); - setActiveTriggerId(id); - setActivePayload(registration?.payload); + latest.current.setActiveTriggerId(id); + latest.current.setActivePayload(registration?.payload);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/dialog/dialog-root.tsx` around lines 121 - 143, Update the setRoot effect callbacks in the dialog root to access setActiveTriggerId and setActivePayload through the latest ref, matching the existing latest.current.applyOpenChange pattern. Ensure openFromTrigger always uses the current controllable-state setters when triggerId control changes, while preserving the existing trigger registration, reference assignment, pending details, and open-change behavior.packages/headless/src/primitives/dialog/dialog-handle.ts (1)
59-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
getRegistryVersionmember.
dialog-root.tsxre-resolves throughstore.subscribe, and no caller readsgetRegistryVersion. Remove the member, counter, and getter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/dialog/dialog-handle.ts` around lines 59 - 60, Remove the unused getRegistryVersion() member from the dialog handle contract, along with the registry version counter and its getter implementation. Preserve the existing store.subscribe-based re-resolution in dialog-root.tsx and remove only the obsolete registry-version plumbing.packages/headless/src/primitives/dialog/use-dialog-origin.ts (1)
34-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCondense the two explanatory comment blocks.
The coding guidelines state: "keep warranted comments to one terse line rather than a verbose multi-line block". The measurement reasoning is worth recording, but six-line and four-line blocks exceed that. Reduce each to one line, or move the full rationale into the function JSDoc at Lines 8-19.
Also applies to: 50-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/dialog/use-dialog-origin.ts` around lines 34 - 39, Condense the explanatory comment blocks surrounding the measurement logic in use-dialog-origin, including the blocks near lines 34-39 and 50-53, to one terse line each. Preserve the essential rationale about scaled getBoundingClientRect values, unscaled offset dimensions, and transform-origin coordinates, without changing the implementation.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/olive-doors-tell.md:
- Around line 2-3: Update the Dialog changes in the relevant UI and headless
compatibility implementations to retain deprecated support for the removed sx
and existing size APIs, preserving consumers on `@clerk/ui`@1 during this patch
release. If compatibility cannot be preserved, change the changeset entries for
`@clerk/headless` and `@clerk/ui` from patch to major and document the migration
path.
In `@packages/headless/src/primitives/dialog/dialog-root.tsx`:
- Around line 117-178: Add an SSR-safe layout-effect helper in the dialog
primitives and use it for every useLayoutEffect shown in DialogInner, including
the latest ref, root registration, reference resolution, payload lookup, state
publication, and cleanup effects. Preserve each effect’s dependencies, cleanup
behavior, and execution order while replacing the direct layout-effect usage
with the helper.
In `@packages/headless/src/primitives/dialog/README.md`:
- Around line 151-171: Qualify the dialog dismissal documentation to reflect
that Escape and outside-press dismissal depend on closedBy: in
packages/headless/src/primitives/dialog/README.md lines 151-171, state that
Escape closes only when closedBy is not 'none'; in
packages/swingset/src/stories/dialog.component.mdx lines 54-55, replace the
unconditional “always” wording with behavior conditional on the default
closedBy='any' value.
- Around line 105-108: Update the initialFocus and finalFocus callback
documentation for Dialog.Popup to remove refs from the callback return options.
Document callback results as boolean, void, HTMLElement, or null, while keeping
refs listed only as supported direct values.
In `@packages/headless/src/primitives/dialog/use-dialog-origin.ts`:
- Around line 25-54: Reset the popup’s ORIGIN_PROPERTY to the neutral center
value before calling getBoundingClientRect() in the useLayoutEffect, ensuring
reused popups are measured without the previous transform origin. Keep the
existing open/popup/trigger guards and origin calculation unchanged, then set
the computed origin afterward.
In `@packages/swingset/src/stories/dialog.component.mdx`:
- Around line 275-286: Add the missing stylex import to the panel example before
its usage in the Dialog content, ensuring the existing stylex.props calls
resolve when the snippet is copied.
In `@packages/ui/src/mosaic/components/dialog/dialog.test.tsx`:
- Around line 541-557: Complete the test around the stacked-dialog teardown flow
by sending a second Escape after the existing inner-dialog assertion, then use
waitFor to assert that themeColor() becomes null after the outer dialog closes
and deferred fade cleanup finishes.
---
Nitpick comments:
In `@packages/headless/src/primitives/dialog/dialog-handle.ts`:
- Around line 59-60: Remove the unused getRegistryVersion() member from the
dialog handle contract, along with the registry version counter and its getter
implementation. Preserve the existing store.subscribe-based re-resolution in
dialog-root.tsx and remove only the obsolete registry-version plumbing.
In `@packages/headless/src/primitives/dialog/dialog-root.tsx`:
- Around line 121-143: Update the setRoot effect callbacks in the dialog root to
access setActiveTriggerId and setActivePayload through the latest ref, matching
the existing latest.current.applyOpenChange pattern. Ensure openFromTrigger
always uses the current controllable-state setters when triggerId control
changes, while preserving the existing trigger registration, reference
assignment, pending details, and open-change behavior.
In `@packages/headless/src/primitives/dialog/dialog.test.tsx`:
- Around line 385-455: Add a test in the detached-trigger describe block that
renders Dialog.Trigger without a Dialog.Root or handle and asserts the
documented error from Dialog.Trigger. Include a rerender or unmount scenario
where the associated store disappears to verify stable hook ordering and ensure
the intended error is reported instead of a React hook-count error.
In `@packages/headless/src/primitives/dialog/use-dialog-origin.ts`:
- Around line 34-39: Condense the explanatory comment blocks surrounding the
measurement logic in use-dialog-origin, including the blocks near lines 34-39
and 50-53, to one terse line each. Preserve the essential rationale about scaled
getBoundingClientRect values, unscaled offset dimensions, and transform-origin
coordinates, without changing the implementation.
In `@packages/ui/src/mosaic/components/dialog/browser-chrome.ts`:
- Around line 160-190: Update makeEasing so its lookup uses a binary search over
the monotonic table rather than incrementing lo through entries linearly;
preserve the existing interpolation and axis calculation behavior.
- Around line 229-257: Move the first JSDoc block describing the refcounted
backdrop behavior and its backdrop parameter from above resolveTint to directly
above export function acquireBrowserChrome. Keep the separate resolveTint
documentation attached to resolveTint, ensuring the public acquireBrowserChrome
API retains its documentation.
In `@packages/ui/src/mosaic/components/dialog/dialog.styles.ts`:
- Around line 199-232: Trim the duplicated rationale comments in sizes.panel,
especially the repeated explanation around alignSelf: 'stretch', leaving one
concise line only where the non-obvious layout decision requires justification.
Apply the same minimal-comment standard to nearby oversized rationale blocks in
this file without changing the style declarations.
In `@packages/ui/src/mosaic/components/dialog/dialog.tsx`:
- Around line 205-229: Prevent consumers from overriding the internal render
used by CloseButton: omit render from DialogCloseButtonProps and exclude it from
the rest props spread, preserving the styled Button with styles.closeButton and
closeInsets[size].
In `@packages/ui/src/mosaic/components/dialog/keyboard-inset.ts`:
- Around line 75-82: Make the release function returned by the keyboard-inset
acquisition flow idempotent by adding a per-release guard, similar to
acquireBrowserChrome’s released flag. Only decrement listeners and detach the
keyboard-inset listener/property when that release has not already been
executed.
In `@packages/ui/src/mosaic/styles/index.ts`:
- Around line 17-30: Update the dialog type re-exports in the styles barrel to
include DialogFocusTarget, DialogHandle, and DialogOpenChangeDetails from the
existing dialog component exports, alongside the current DialogRootProps,
DialogTriggerProps, and DialogPopupProps types.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e06bfa40-bf96-4071-89f5-c32a12b34f03
📒 Files selected for processing (37)
.changeset/lucky-donuts-invite.md.changeset/olive-doors-tell.md.changeset/spicy-clocks-argue.mdpackages/headless/src/primitives/dialog/README.mdpackages/headless/src/primitives/dialog/dialog-backdrop.tsxpackages/headless/src/primitives/dialog/dialog-context.tspackages/headless/src/primitives/dialog/dialog-handle.tspackages/headless/src/primitives/dialog/dialog-popup.tsxpackages/headless/src/primitives/dialog/dialog-root.tsxpackages/headless/src/primitives/dialog/dialog-trigger.tsxpackages/headless/src/primitives/dialog/dialog-viewport.tsxpackages/headless/src/primitives/dialog/dialog.test.tsxpackages/headless/src/primitives/dialog/index.tspackages/headless/src/primitives/dialog/parts.tspackages/headless/src/primitives/dialog/use-dialog-origin.tspackages/headless/src/primitives/drawer/drawer-context.tspackages/headless/src/utils/interaction-modality.tspackages/swingset/src/stories/dialog.component.mdxpackages/swingset/src/stories/dialog.component.stories.tsxpackages/swingset/src/stories/dialog.mdxpackages/swingset/src/stories/dialog.stories.tsxpackages/ui/src/mosaic/block/destructive.tsxpackages/ui/src/mosaic/components/button/button.tsxpackages/ui/src/mosaic/components/dialog.tsxpackages/ui/src/mosaic/components/dialog/browser-chrome.tspackages/ui/src/mosaic/components/dialog/dialog.styles.tspackages/ui/src/mosaic/components/dialog/dialog.test.tsxpackages/ui/src/mosaic/components/dialog/dialog.tsxpackages/ui/src/mosaic/components/dialog/index.tspackages/ui/src/mosaic/components/dialog/keyboard-inset.tspackages/ui/src/mosaic/organization/organization-profile-domains-section-add-verify.view.tsxpackages/ui/src/mosaic/organization/organization-profile-domains-section-enrollment.view.tsxpackages/ui/src/mosaic/organization/organization-profile-domains-section-remove.view.tsxpackages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsxpackages/ui/src/mosaic/primitives/dialog.tsxpackages/ui/src/mosaic/styles/index.tspackages/ui/src/mosaic/tokens.stylex.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
💤 Files with no reviewable changes (2)
- packages/ui/src/mosaic/primitives/dialog.tsx
- packages/ui/src/mosaic/components/dialog.tsx
| '@clerk/headless': patch | ||
| '@clerk/ui': patch |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve the removed Dialog APIs in this patch release.
This changeset states that consumers must replace sx and migrate existing size usage. That breaks consumers that remain on @clerk/ui@1 after a patch upgrade.
Keep deprecated compatibility paths for these APIs in a non-major release. Otherwise, publish this as a major release with a migration path.
As per coding guidelines, “Maintain backward compatibility in packages/clerk-js and packages/ui with SDK versions already in the wild.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.changeset/olive-doors-tell.md around lines 2 - 3, Update the Dialog changes
in the relevant UI and headless compatibility implementations to retain
deprecated support for the removed sx and existing size APIs, preserving
consumers on `@clerk/ui`@1 during this patch release. If compatibility cannot be
preserved, change the changeset entries for `@clerk/headless` and `@clerk/ui` from
patch to major and document the migration path.
Sources: Coding guidelines, Linked repositories
| useLayoutEffect(() => { | ||
| latest.current = { applyOpenChange, activeTriggerId }; | ||
| }); | ||
|
|
||
| useLayoutEffect(() => { | ||
| return store.setRoot({ | ||
| openFromTrigger: (id, event) => { | ||
| const registration = store.getTrigger(id); | ||
| setActiveTriggerId(id); | ||
| setActivePayload(registration?.payload); | ||
| if (registration) { | ||
| refs.setReference(registration.element); | ||
| } | ||
| pendingDetailsRef.current = { trigger: registration?.element ?? null, triggerId: id, event }; | ||
| floatingContext.onOpenChange(true, event, 'click'); | ||
| }, | ||
| closeFromTrigger: (id, event) => { | ||
| const registration = store.getTrigger(id); | ||
| pendingDetailsRef.current = { trigger: registration?.element ?? null, triggerId: id, event }; | ||
| floatingContext.onOpenChange(false, event, 'click'); | ||
| }, | ||
| setOpen: nextOpen => { | ||
| latest.current.applyOpenChange(nextOpen, { trigger: null, triggerId: null, event: undefined }); | ||
| }, | ||
| }); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps -- floatingContext.onOpenChange, setActiveTriggerId and refs are stable | ||
| }, [store]); | ||
|
|
||
| // The floating reference is the ACTIVE trigger — origin measurement, return focus, and | ||
| // outside-press exclusion all read `elements.domReference`. With no active trigger the first | ||
| // registered one stands in, preserving single-trigger behaviour for `defaultOpen` dialogs. | ||
| // | ||
| // Subscribed imperatively rather than through `useSyncExternalStore`: re-registration must not | ||
| // re-render this component, or a trigger whose `payload` is an inline object literal would | ||
| // re-register on every render of its own and the two would feed each other forever. | ||
| useLayoutEffect(() => { | ||
| const resolve = () => { | ||
| const active = activeTriggerId != null ? store.getTrigger(activeTriggerId) : undefined; | ||
| refs.setReference(active?.element ?? store.getFirstTrigger()?.element ?? null); | ||
| }; | ||
| resolve(); | ||
| return store.subscribe(resolve); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps -- refs is stable | ||
| }, [store, activeTriggerId]); | ||
|
|
||
| // For opens that arrive without a trigger activation — a controlled `open`/`triggerId` pair, | ||
| // `defaultOpen` — the payload is looked up from the registry once the dialog is open. Runs | ||
| // after the children's layout effects, so triggers rendered inside the root are registered by | ||
| // the time it reads, and the pre-paint re-render delivers their payload on the first frame. | ||
| useLayoutEffect(() => { | ||
| if (open) { | ||
| setActivePayload(activeTriggerId != null ? store.getTrigger(activeTriggerId)?.payload : undefined); | ||
| } | ||
| }, [store, open, activeTriggerId]); | ||
|
|
||
| // What detached triggers render their open state and ARIA wiring from. | ||
| useLayoutEffect(() => { | ||
| store.publishState({ open, triggerId: activeTriggerId, popupId: floatingContext.floatingId }); | ||
| }, [store, open, activeTriggerId, floatingContext.floatingId]); | ||
| useLayoutEffect(() => { | ||
| return () => store.publishState({ open: false, triggerId: null, popupId: undefined }); | ||
| }, [store]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for an existing isomorphic layout effect helper and how other primitives handle SSR.
rg -nP -g 'packages/headless/**/*.{ts,tsx}' -C2 'useIsomorphicLayoutEffect|useIsoLayoutEffect|typeof (window|document) !== .undefined.'
# Check how many primitives call useLayoutEffect directly.
rg -nc -g 'packages/headless/src/**/*.tsx' 'useLayoutEffect'Repository: clerk/javascript
Length of output: 154
🏁 Script executed:
#!/bin/bash
set +e
printf '%s\n' '--- candidate files ---'
git ls-files 'packages/headless/**' | sed -n '1,160p'
printf '%s\n' '--- reviewed file imports and effects ---'
sed -n '1,230p' packages/headless/src/primitives/dialog/dialog-root.tsx
printf '%s\n' '--- all effect/helper references in headless ---'
rg -n -S -g 'packages/headless/**/*.{ts,tsx,js,jsx}' \
'use(Layout|Effect)|use(Isomorphic|Iso).*Effect|isomorphic|typeof[[:space:]]*\((window|document)\)|typeof[[:space:]]+(window|document)' \
|| true
printf '%s\n' '--- package metadata and SSR references ---'
rg -n -S -g 'packages/headless/**' -g 'packages/*/package.json' \
'SSR|server rendering|server-side|useLayoutEffect|react-dom/server|renderToString|renderToPipeableStream|react-server' \
|| trueRepository: clerk/javascript
Length of output: 18699
🏁 Script executed:
#!/bin/bash
set +e
printf '%s\n' '--- direct layout-effect usage ---'
rg -n -S 'useLayoutEffect|useIsomorphicLayoutEffect|useIsoLayoutEffect' packages/headless/src || true
printf '%s\n' '--- SSR test ---'
sed -n '1,240p' packages/headless/src/primitives/drawer/drawer.ssr.test.tsx
printf '%s\n' '--- package metadata ---'
cat packages/headless/package.json
printf '%s\n' '--- package README SSR references ---'
rg -n -i -C3 'ssr|server|render' packages/headless/README.md packages/headless/src packages/headless/package.json || true
printf '%s\n' '--- repository-wide effect helper definitions ---'
rg -n -S 'use(Isomorphic|Iso).*Effect|isomorphic.*effect|useLayoutEffect' packages | sed -n '1,240p'Repository: clerk/javascript
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set +e
printf '%s\n' '--- exact layout-effect references in source ---'
rg -n -S '\buseLayoutEffect\b|\buseIsomorphicLayoutEffect\b|\buseIsoLayoutEffect\b' packages/headless/src --glob '!**/*.test.*' || true
printf '%s\n' '--- exact layout-effect references in tests ---'
rg -n -S '\buseLayoutEffect\b|\buseIsomorphicLayoutEffect\b|\buseIsoLayoutEffect\b' packages/headless/src --glob '**/*.test.*' || true
printf '%s\n' '--- drawer SSR test ---'
cat -n packages/headless/src/primitives/drawer/drawer.ssr.test.tsx
printf '%s\n' '--- package metadata ---'
cat packages/headless/package.json
printf '%s\n' '--- SSR-related package files ---'
rg -n -i -S 'SSR|server rendering|server-side|react-dom/server|renderToString|renderToPipeableStream' packages/headless --glob '!**/*.test.*' || trueRepository: clerk/javascript
Length of output: 8072
🌐 Web query:
React useLayoutEffect server rendering warning official documentation
💡 Result:
When using useLayoutEffect in a server-side rendering (SSR) environment, React issues a warning because the hook is designed to perform DOM operations or measurements that rely on layout information which does not exist on the server [1][2][3]. Since useLayoutEffect only runs on the client, it cannot affect the initial HTML generated during server rendering, potentially leading to a "flash" or a mismatch between the server-rendered UI and the intended client-rendered UI [3][4][5]. According to official React documentation and guidance, there are several recommended approaches to resolve this warning [1][2]: 1. Replace with useEffect: If the logic does not strictly need to block the browser from repainting the screen (e.g., it is not measuring layout for immediate positioning), replace useLayoutEffect with useEffect [1][2]. This will suppress the warning because useEffect is intended to run after the paint [1][3]. 2. Defer rendering until client-side hydration: If the component's UI depends on layout measurements and would appear broken without them, delay its rendering until after the component has mounted on the client [1][2]. You can achieve this by using a state variable (e.g., isMounted) initialized to false, and setting it to true within a useEffect hook [1][2][6]. The component will then only render its layout-dependent content after hydration is complete [1]. 3. Use useSyncExternalStore: If you are synchronizing your component with an external data store rather than performing DOM measurements, consider using the useSyncExternalStore hook, which is explicitly designed to support server rendering [1][2]. Attempting to create a custom "isomorphic" layout effect hook by simply suppressing the warning does not make the hook work on the server; it only hides the diagnostic message while the underlying issue—that the effect cannot execute on the server—remains [5].
Citations:
- 1: https://react.dev/reference/react/useLayoutEffect
- 2: https://uk.react.dev/reference/react/useLayoutEffect
- 3: https://kevinyank.com/posts/useeffect-vs-uselayouteffect-and-ssr/
- 4: useLayoutEffect in ssr react/react#14927
- 5: https://handlewithcare.dev/blog/no_such_thing_isomorphic_layout_effect/
- 6: https://www.hendriklammers.com/notes/uselayouteffect-and-ssr
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from pathlib import Path
import re
root = Path("packages/headless/src")
source_files = [
p for p in root.rglob("*")
if p.suffix in {".ts", ".tsx"} and ".test." not in p.name
]
direct = []
for path in source_files:
text = path.read_text()
count = len(re.findall(r"\buseLayoutEffect\b", text))
if count:
direct.append((str(path), count))
print("direct_useLayoutEffect_files=", len(direct))
print("direct_useLayoutEffect_occurrences=", sum(count for _, count in direct))
for path, count in direct:
print(f"{path}: {count}")
print("isomorphic_helper_names_found=", any(
re.search(r"\buse(?:Isomorphic|Iso)LayoutEffect\b", p.read_text())
for p in source_files
))
print("ssr_test_files=", [
str(p) for p in root.rglob("*.ssr.test.*")
])
PYRepository: clerk/javascript
Length of output: 895
Add an SSR-safe layout-effect helper for DialogInner
@clerk/headless supports SSR but has no such helper. Add one and use it for these effects to prevent React’s server-rendering warning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/headless/src/primitives/dialog/dialog-root.tsx` around lines 117 -
178, Add an SSR-safe layout-effect helper in the dialog primitives and use it
for every useLayoutEffect shown in DialogInner, including the latest ref, root
registration, reference resolution, payload lookup, state publication, and
cleanup effects. Preserve each effect’s dependencies, cleanup behavior, and
execution order while replacing the direct layout-effect usage with the helper.
Source: Coding guidelines
| `initialFocus` and `finalFocus` on `Dialog.Popup` control where focus moves on open and close. | ||
| Each accepts `true` (the default behaviour), `false` (do not move focus), a ref, or a function of | ||
| the interaction type behind the open/close (`'mouse' | 'touch' | 'pen' | 'keyboard' | ''`, empty | ||
| for programmatic) returning any of those: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline packages/headless/src/primitives/dialog --items all --match 'DialogFocusTarget|initialFocus|finalFocus'
rg -n -C 4 '\b(DialogFocusTarget|initialFocus|finalFocus)\b' packages/headless/src/primitives/dialog -g '*.ts' -g '*.tsx'Repository: clerk/javascript
Length of output: 22452
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- README focus documentation ---'
cat -n packages/headless/src/primitives/dialog/README.md | sed -n '95,115p;190,210p'
printf '%s\n' '--- exported focus type and implementation ---'
cat -n packages/headless/src/primitives/dialog/dialog-popup.tsx | sed -n '10,32p;60,86p;86,116p'
cat -n packages/headless/src/primitives/dialog/index.ts | sed -n '1,25p'
printf '%s\n' '--- focus-related README references ---'
rg -n -C 3 'RefObject|callback|function|initialFocus|finalFocus|focus target' packages/headless/src/primitives/dialog/README.mdRepository: clerk/javascript
Length of output: 10340
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
source = Path("packages/headless/src/primitives/dialog/dialog-popup.tsx").read_text()
readme = Path("packages/headless/src/primitives/dialog/README.md").read_text()
type_match = re.search(
r"export type DialogFocusTarget\s*=\s*(.*?);",
source,
re.S,
)
assert type_match, "DialogFocusTarget declaration not found"
type_text = " ".join(type_match.group(1).split())
callback_match = re.search(r"\(\(interactionType: InteractionType\) => ([^)]+)\)", type_text)
assert callback_match, "DialogFocusTarget callback return type not found"
callback_return = callback_match.group(1)
claim_match = re.search(
r"Each accepts .*?a ref, or a function .*?returning any of those:",
readme,
re.S,
)
assert claim_match, "README focus-target claim not found"
print("DialogFocusTarget:", type_text)
print("Callback return type:", callback_return)
print("Callback return includes RefObject:", "RefObject" in callback_return)
print("README says callback returns the ref option:", "returning any of those" in claim_match.group(0))
PYRepository: clerk/javascript
Length of output: 449
Correct the focus callback documentation.
DialogFocusTarget allows refs only as direct values. Callback results allow boolean, void, HTMLElement, or null. Remove refs from the callback description.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/headless/src/primitives/dialog/README.md` around lines 105 - 108,
Update the initialFocus and finalFocus callback documentation for Dialog.Popup
to remove refs from the callback return options. Document callback results as
boolean, void, HTMLElement, or null, while keeping refs listed only as supported
direct values.
Source: Coding guidelines
| useLayoutEffect(() => { | ||
| const popup = popupRef.current; | ||
| if (!open || !popup || !trigger) { | ||
| return; | ||
| } | ||
|
|
||
| const triggerRect = trigger.getBoundingClientRect(); | ||
| const popupRect = popup.getBoundingClientRect(); | ||
|
|
||
| // `getBoundingClientRect` reports the SCALED box, and the entering frame is already at | ||
| // `scale(0.98)`. Its CENTRE is not affected, though — the property is still unset at this | ||
| // point, so that scale is about `center` — and `offsetWidth`/`offsetHeight` are the | ||
| // unscaled layout dimensions. Together they recover the untransformed box, which is what | ||
| // `transform-origin`'s coordinates are relative to. Measuring the scaled edges instead | ||
| // would offset the origin by half the scale delta on each axis. | ||
| const centerX = popupRect.left + popupRect.width / 2; | ||
| const centerY = popupRect.top + popupRect.height / 2; | ||
| const layoutLeft = centerX - popup.offsetWidth / 2; | ||
| const layoutTop = centerY - popup.offsetHeight / 2; | ||
|
|
||
| const originX = triggerRect.left + triggerRect.width / 2 - layoutLeft; | ||
| const originY = triggerRect.top + triggerRect.height / 2 - layoutTop; | ||
|
|
||
| popup.style.setProperty(ORIGIN_PROPERTY, `${originX}px ${originY}px`); | ||
|
|
||
| // Runs in a layout effect, so this lands before paint on the frame that still carries | ||
| // `data-starting-style` — the frame pinned at `opacity: 0` with `transition: none`. Moving | ||
| // the origin repositions the scaled box, and that reflow is invisible for the same reason | ||
| // the popover's is: nothing is painted yet, and the transition arms a frame later. | ||
| }, [popupRef, trigger, open]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Determine whether the dialog popup element can survive a close/reopen cycle.
set -euo pipefail
fd . packages/headless/src/primitives/dialog --type f --extension ts --extension tsx --exec cat -n {}
# Look for keepMounted / persistence options that would retain the popup node.
rg -n -C4 'keepMounted|unmount|mounted' packages/headless/src/primitives/dialog packages/headless/src/utilsRepository: clerk/javascript
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- use-dialog-origin.ts ---'
cat -n packages/headless/src/primitives/dialog/use-dialog-origin.ts
printf '%s\n' '--- use-transition implementation and usages ---'
rg -n -C5 'export function useTransition|function useTransition|useTransition\(' packages/headless/src/hooks packages/headless/src/primitives/dialog
printf '%s\n' '--- persistence-related dialog APIs ---'
rg -n -C3 'keepMounted|unmount|mounted|presence|exit|transition' packages/headless/src packages/headless/test packages/headless 2>/dev/null | head -240Repository: clerk/javascript
Length of output: 34603
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- use-transition-status.ts ---'
cat -n packages/headless/src/hooks/use-transition-status.ts
printf '%s\n' '--- use-transition.ts ---'
cat -n packages/headless/src/hooks/use-transition.ts
printf '%s\n' '--- animation-finish helper ---'
cat -n packages/headless/src/hooks/use-animations-finished.ts
printf '%s\n' '--- rapid close/open transition tests ---'
sed -n '145,220p' packages/headless/src/hooks/use-transition.test.tsRepository: clerk/javascript
Length of output: 10946
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C5 -- '--cl-dialog-origin|data-starting-style|data-ending-style' packages | head -300Repository: clerk/javascript
Length of output: 29599
Reset --cl-dialog-origin before measuring a reused popup.
useTransition keeps the popup mounted during an exit animation and supports rapid close-to-open cancellation. The next layout effect can measure the same element with the previous --cl-dialog-origin. Clear the property or set it to center before reading getBoundingClientRect().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/headless/src/primitives/dialog/use-dialog-origin.ts` around lines 25
- 54, Reset the popup’s ORIGIN_PROPERTY to the neutral center value before
calling getBoundingClientRect() in the useLayoutEffect, ensuring reused popups
are measured without the previous transform origin. Keep the existing
open/popup/trigger guards and origin calculation unchanged, then set the
computed origin afterward.
Source: Coding guidelines
| ```tsx | ||
| import { scrollAreaRoot, scrollAreaViewport } from '@clerk/ui/mosaic/components/scroll-area'; | ||
|
|
||
| <Dialog size='panel' trigger={props => <Button {...props}>Open settings</Button>}> | ||
| <Dialog.CloseButton /> | ||
| <Dialog.Title render={<Heading size='lg' />}>Settings</Dialog.Title> | ||
|
|
||
| <div style={{ display: 'flex', flex: 1, gap: '1.5rem', minHeight: 0 }}> | ||
| <nav style={{ flex: 'none', width: '12rem' }}>…</nav> | ||
|
|
||
| <div {...stylex.props(scrollAreaRoot)} style={{ flex: 1, minWidth: 0 }}> | ||
| <div {...stylex.props(...scrollAreaViewport())}> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 "import \\* as stylex from '`@stylexjs/stylex`'|stylex\\.props" packages/ui packages/swingset \
-g '*.ts' -g '*.tsx' -g '*.mdx'Repository: clerk/javascript
Length of output: 46756
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '260,295p' packages/swingset/src/stories/dialog.component.mdx
printf '\n--- stylex references and imports in this story ---\n'
rg -n "stylex|scrollAreaRoot|scrollAreaViewport" packages/swingset/src/stories/dialog.component.mdxRepository: clerk/javascript
Length of output: 2119
Import stylex in the panel example.
The example calls stylex.props without defining stylex, so copied code fails with an unresolved identifier.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/swingset/src/stories/dialog.component.mdx` around lines 275 - 286,
Add the missing stylex import to the panel example before its usage in the
Dialog content, ensuring the existing stylex.props calls resolve when the
snippet is copied.
Source: Coding guidelines
| it('keeps one meta for stacked dialogs and removes it only with the last', async () => { | ||
| const user = userEvent.setup(); | ||
| render( | ||
| <Dialog defaultOpen> | ||
| <div>Outer</div> | ||
| <Dialog trigger={addEmailTriggerShared}> | ||
| <div>Inner</div> | ||
| </Dialog> | ||
| </Dialog>, | ||
| ); | ||
|
|
||
| await user.click(screen.getByRole('button', { name: 'Add email' })); | ||
| expect(document.head.querySelectorAll('meta[name="theme-color"]')).toHaveLength(1); | ||
|
|
||
| await user.keyboard('{Escape}'); | ||
| expect(themeColor()).not.toBeNull(); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Complete the stacked-dialog teardown assertion.
The test name states that the meta is removed only with the last dialog. The test closes the inner dialog and asserts the meta still exists. It never closes the outer dialog, so the "removes it only with the last" half is unverified. Add a second Escape and assert removal. Use waitFor, because removal is deferred until after the fade.
💚 Proposed addition
await user.keyboard('{Escape}');
expect(themeColor()).not.toBeNull();
+
+ await user.keyboard('{Escape}');
+ await waitFor(() => expect(themeColor()).toBeNull());
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('keeps one meta for stacked dialogs and removes it only with the last', async () => { | |
| const user = userEvent.setup(); | |
| render( | |
| <Dialog defaultOpen> | |
| <div>Outer</div> | |
| <Dialog trigger={addEmailTriggerShared}> | |
| <div>Inner</div> | |
| </Dialog> | |
| </Dialog>, | |
| ); | |
| await user.click(screen.getByRole('button', { name: 'Add email' })); | |
| expect(document.head.querySelectorAll('meta[name="theme-color"]')).toHaveLength(1); | |
| await user.keyboard('{Escape}'); | |
| expect(themeColor()).not.toBeNull(); | |
| }); | |
| it('keeps one meta for stacked dialogs and removes it only with the last', async () => { | |
| const user = userEvent.setup(); | |
| render( | |
| <Dialog defaultOpen> | |
| <div>Outer</div> | |
| <Dialog trigger={addEmailTriggerShared}> | |
| <div>Inner</div> | |
| </Dialog> | |
| </Dialog>, | |
| ); | |
| await user.click(screen.getByRole('button', { name: 'Add email' })); | |
| expect(document.head.querySelectorAll('meta[name="theme-color"]')).toHaveLength(1); | |
| await user.keyboard('{Escape}'); | |
| expect(themeColor()).not.toBeNull(); | |
| await user.keyboard('{Escape}'); | |
| await waitFor(() => expect(themeColor()).toBeNull()); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/mosaic/components/dialog/dialog.test.tsx` around lines 541 -
557, Complete the test around the stacked-dialog teardown flow by sending a
second Escape after the existing inner-dialog assertion, then use waitFor to
assert that themeColor() becomes null after the outer dialog closes and deferred
fade cleanup finishes.
Source: Coding guidelines
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 7
🧹 Nitpick comments (10)
packages/ui/src/mosaic/components/dialog/keyboard-inset.ts (1)
75-82: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake the release function idempotent.
Each call to the returned function decrements
listeners. A second call on the same release drives the count below zero. The counter then never returns to0, and the listeners plus the--_cl-keyboard-insetproperty stay attached for the lifetime of the page.acquireBrowserChromeinbrowser-chrome.tsguards this case with areleasedflag; this module does not.♻️ Proposed guard
- return () => { - listeners--; - if (listeners === 0 && detach) { - detach(); - detach = null; - } - }; + let released = false; + return () => { + if (released) { + return; + } + released = true; + listeners--; + if (listeners === 0 && detach) { + detach(); + detach = null; + } + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/dialog/keyboard-inset.ts` around lines 75 - 82, Make the release function returned by the keyboard-inset acquisition flow idempotent by adding a per-release guard, similar to acquireBrowserChrome’s released flag. Only decrement listeners and detach the keyboard-inset listener/property when that release has not already been executed.packages/ui/src/mosaic/components/dialog/dialog.tsx (1)
205-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
renderinrestsilently replaces the styled Button.
CloseButtonsetsrenderbefore{...rest}. If a consumer passesrender, their element replaces theButtonwrapper, andstyles.closeButtonpluscloseInsets[size]are lost. The button then loses its absolute anchoring. Consider omittingrenderfromDialogCloseButtonProps, or documenting that the override must supply its own positioning.♻️ Proposed type change
-export interface DialogCloseButtonProps extends MosaicComponentProps<'button'> { +export interface DialogCloseButtonProps extends Omit<MosaicComponentProps<'button'>, 'render'> {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/dialog/dialog.tsx` around lines 205 - 229, Prevent consumers from overriding the internal render used by CloseButton: omit render from DialogCloseButtonProps and exclude it from the rest props spread, preserving the styled Button with styles.closeButton and closeInsets[size].packages/ui/src/mosaic/components/dialog/dialog.styles.ts (1)
199-232: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTrim the duplicated and oversized comments in
sizes.panel.Two blocks state the same fact. Lines 214-223 explain that the panel fills the viewport content box with
stretch, and lines 224-228 repeat it. Reduce the block to a single terse note. The same applies across this file, where multi-paragraph rationale blocks dominate the style declarations.The coding guidelines require minimal comments: "Add comments only when critical to explain why a non-obvious change was made; never restate code behavior, and keep warranted comments to one terse line rather than a verbose multi-line block."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/dialog/dialog.styles.ts` around lines 199 - 232, Trim the duplicated rationale comments in sizes.panel, especially the repeated explanation around alignSelf: 'stretch', leaving one concise line only where the non-obvious layout decision requires justification. Apply the same minimal-comment standard to nearby oversized rationale blocks in this file without changing the style declarations.Source: Coding guidelines
packages/ui/src/mosaic/components/dialog/browser-chrome.ts (2)
160-190: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe comment describes a binary search; the code performs a linear scan.
makeEasingwalks the table withwhile (lo < SAMPLES && table[lo + 1] < x) lo++. That is a linear scan, not a binary search. Correct the comment or implement the search that it describes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/dialog/browser-chrome.ts` around lines 160 - 190, Update makeEasing so its lookup uses a binary search over the monotonic table rather than incrementing lo through entries linearly; preserve the existing interpolation and axis calculation behavior.
229-257: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe
acquireBrowserChromedoc block is attached toresolveTint.Lines 229-237 document
acquireBrowserChromeand its@param backdrop. A second doc block forresolveTintfollows at lines 238-243, and thefunction resolveTintdeclaration follows that. TypeScript and editors therefore associate the first block with nothing, andacquireBrowserChromeat line 259 has no JSDoc. Move the first block directly aboveexport function acquireBrowserChrome.The coding guidelines require that "All public APIs must be documented with JSDoc".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/dialog/browser-chrome.ts` around lines 229 - 257, Move the first JSDoc block describing the refcounted backdrop behavior and its backdrop parameter from above resolveTint to directly above export function acquireBrowserChrome. Keep the separate resolveTint documentation attached to resolveTint, ensuring the public acquireBrowserChrome API retains its documentation.Source: Coding guidelines
packages/ui/src/mosaic/styles/index.ts (1)
17-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRe-export the headless dialog types that the exported prop types reference.
packages/ui/src/mosaic/components/dialog/index.tsalso exportsDialogFocusTarget,DialogHandle, andDialogOpenChangeDetails. This barrel omits them.DialogRootPropsandDialogTriggerPropscarry ahandle?: DialogHandle<Payload>member, andDialogPopupPropscarriesinitialFocus/finalFocusof typeDialogFocusTarget. A consumer of this entry point can therefore pass those props but cannot name their types.♻️ Proposed addition
export { Dialog } from '../components/dialog'; export type { DialogBackdropProps, DialogCloseButtonProps, DialogCloseProps, DialogDescriptionProps, + DialogFocusTarget, + DialogHandle, + DialogOpenChangeDetails, DialogPopupProps, DialogProps, DialogRootProps, DialogSize, DialogTitleProps, DialogTriggerProps, DialogViewportProps, } from '../components/dialog';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/styles/index.ts` around lines 17 - 30, Update the dialog type re-exports in the styles barrel to include DialogFocusTarget, DialogHandle, and DialogOpenChangeDetails from the existing dialog component exports, alongside the current DialogRootProps, DialogTriggerProps, and DialogPopupProps types.packages/headless/src/primitives/dialog/dialog.test.tsx (1)
385-455: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for a
Dialog.Triggerwith neither a root nor a handle.
dialog-trigger.tsxline 36 throws a documented error for this case. No test covers it. The test also exposes the hook-order problem flagged inpackages/headless/src/primitives/dialog/dialog-trigger.tsxlines 33-37, because React reports a hook-count error instead of the intended message once a store disappears between renders.💚 Proposed test
+ it('throws when the trigger has neither a root nor a handle', () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render(<Dialog.Trigger>Orphan</Dialog.Trigger>)).toThrow( + /must be nested in a <Dialog.Root> or given a `handle`/, + ); + consoleError.mockRestore(); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/dialog/dialog.test.tsx` around lines 385 - 455, Add a test in the detached-trigger describe block that renders Dialog.Trigger without a Dialog.Root or handle and asserts the documented error from Dialog.Trigger. Include a rerender or unmount scenario where the associated store disappears to verify stable hook ordering and ensure the intended error is reported instead of a React hook-count error.Source: Coding guidelines
packages/headless/src/primitives/dialog/dialog-root.tsx (1)
121-143: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConfirm the stability assumptions in the
setRooteffect.The effect only re-runs when
storechanges.openFromTriggerandcloseFromTriggercapturerefs,floatingContext, andsetActiveTriggerIdfrom the render that attached the controller.setActiveTriggerIdcomes fromuseControllableState, whose setter identity depends onisControlled. If a consumer switchestriggerIdbetweenundefinedand a value after mount, the captured setter becomes stale and trigger attribution stops updating.applyOpenChangealready avoids this through thelatestref; consider routingsetActiveTriggerIdandsetActivePayloadthrough the same ref.♻️ Proposed change to route trigger state through the latest ref
- const latest = useRef({ applyOpenChange, activeTriggerId }); + const latest = useRef({ applyOpenChange, activeTriggerId, setActiveTriggerId, setActivePayload }); useLayoutEffect(() => { - latest.current = { applyOpenChange, activeTriggerId }; + latest.current = { applyOpenChange, activeTriggerId, setActiveTriggerId, setActivePayload }; }); useLayoutEffect(() => { return store.setRoot({ openFromTrigger: (id, event) => { const registration = store.getTrigger(id); - setActiveTriggerId(id); - setActivePayload(registration?.payload); + latest.current.setActiveTriggerId(id); + latest.current.setActivePayload(registration?.payload);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/dialog/dialog-root.tsx` around lines 121 - 143, Update the setRoot effect callbacks in the dialog root to access setActiveTriggerId and setActivePayload through the latest ref, matching the existing latest.current.applyOpenChange pattern. Ensure openFromTrigger always uses the current controllable-state setters when triggerId control changes, while preserving the existing trigger registration, reference assignment, pending details, and open-change behavior.packages/headless/src/primitives/dialog/dialog-handle.ts (1)
59-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
getRegistryVersionmember.
dialog-root.tsxre-resolves throughstore.subscribe, and no caller readsgetRegistryVersion. Remove the member, counter, and getter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/dialog/dialog-handle.ts` around lines 59 - 60, Remove the unused getRegistryVersion() member from the dialog handle contract, along with the registry version counter and its getter implementation. Preserve the existing store.subscribe-based re-resolution in dialog-root.tsx and remove only the obsolete registry-version plumbing.packages/headless/src/primitives/dialog/use-dialog-origin.ts (1)
34-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCondense the two explanatory comment blocks.
The coding guidelines state: "keep warranted comments to one terse line rather than a verbose multi-line block". The measurement reasoning is worth recording, but six-line and four-line blocks exceed that. Reduce each to one line, or move the full rationale into the function JSDoc at Lines 8-19.
Also applies to: 50-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/dialog/use-dialog-origin.ts` around lines 34 - 39, Condense the explanatory comment blocks surrounding the measurement logic in use-dialog-origin, including the blocks near lines 34-39 and 50-53, to one terse line each. Preserve the essential rationale about scaled getBoundingClientRect values, unscaled offset dimensions, and transform-origin coordinates, without changing the implementation.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/olive-doors-tell.md:
- Around line 2-3: Update the Dialog changes in the relevant UI and headless
compatibility implementations to retain deprecated support for the removed sx
and existing size APIs, preserving consumers on `@clerk/ui`@1 during this patch
release. If compatibility cannot be preserved, change the changeset entries for
`@clerk/headless` and `@clerk/ui` from patch to major and document the migration
path.
In `@packages/headless/src/primitives/dialog/dialog-root.tsx`:
- Around line 117-178: Add an SSR-safe layout-effect helper in the dialog
primitives and use it for every useLayoutEffect shown in DialogInner, including
the latest ref, root registration, reference resolution, payload lookup, state
publication, and cleanup effects. Preserve each effect’s dependencies, cleanup
behavior, and execution order while replacing the direct layout-effect usage
with the helper.
In `@packages/headless/src/primitives/dialog/README.md`:
- Around line 151-171: Qualify the dialog dismissal documentation to reflect
that Escape and outside-press dismissal depend on closedBy: in
packages/headless/src/primitives/dialog/README.md lines 151-171, state that
Escape closes only when closedBy is not 'none'; in
packages/swingset/src/stories/dialog.component.mdx lines 54-55, replace the
unconditional “always” wording with behavior conditional on the default
closedBy='any' value.
- Around line 105-108: Update the initialFocus and finalFocus callback
documentation for Dialog.Popup to remove refs from the callback return options.
Document callback results as boolean, void, HTMLElement, or null, while keeping
refs listed only as supported direct values.
In `@packages/headless/src/primitives/dialog/use-dialog-origin.ts`:
- Around line 25-54: Reset the popup’s ORIGIN_PROPERTY to the neutral center
value before calling getBoundingClientRect() in the useLayoutEffect, ensuring
reused popups are measured without the previous transform origin. Keep the
existing open/popup/trigger guards and origin calculation unchanged, then set
the computed origin afterward.
In `@packages/swingset/src/stories/dialog.component.mdx`:
- Around line 275-286: Add the missing stylex import to the panel example before
its usage in the Dialog content, ensuring the existing stylex.props calls
resolve when the snippet is copied.
In `@packages/ui/src/mosaic/components/dialog/dialog.test.tsx`:
- Around line 541-557: Complete the test around the stacked-dialog teardown flow
by sending a second Escape after the existing inner-dialog assertion, then use
waitFor to assert that themeColor() becomes null after the outer dialog closes
and deferred fade cleanup finishes.
---
Nitpick comments:
In `@packages/headless/src/primitives/dialog/dialog-handle.ts`:
- Around line 59-60: Remove the unused getRegistryVersion() member from the
dialog handle contract, along with the registry version counter and its getter
implementation. Preserve the existing store.subscribe-based re-resolution in
dialog-root.tsx and remove only the obsolete registry-version plumbing.
In `@packages/headless/src/primitives/dialog/dialog-root.tsx`:
- Around line 121-143: Update the setRoot effect callbacks in the dialog root to
access setActiveTriggerId and setActivePayload through the latest ref, matching
the existing latest.current.applyOpenChange pattern. Ensure openFromTrigger
always uses the current controllable-state setters when triggerId control
changes, while preserving the existing trigger registration, reference
assignment, pending details, and open-change behavior.
In `@packages/headless/src/primitives/dialog/dialog.test.tsx`:
- Around line 385-455: Add a test in the detached-trigger describe block that
renders Dialog.Trigger without a Dialog.Root or handle and asserts the
documented error from Dialog.Trigger. Include a rerender or unmount scenario
where the associated store disappears to verify stable hook ordering and ensure
the intended error is reported instead of a React hook-count error.
In `@packages/headless/src/primitives/dialog/use-dialog-origin.ts`:
- Around line 34-39: Condense the explanatory comment blocks surrounding the
measurement logic in use-dialog-origin, including the blocks near lines 34-39
and 50-53, to one terse line each. Preserve the essential rationale about scaled
getBoundingClientRect values, unscaled offset dimensions, and transform-origin
coordinates, without changing the implementation.
In `@packages/ui/src/mosaic/components/dialog/browser-chrome.ts`:
- Around line 160-190: Update makeEasing so its lookup uses a binary search over
the monotonic table rather than incrementing lo through entries linearly;
preserve the existing interpolation and axis calculation behavior.
- Around line 229-257: Move the first JSDoc block describing the refcounted
backdrop behavior and its backdrop parameter from above resolveTint to directly
above export function acquireBrowserChrome. Keep the separate resolveTint
documentation attached to resolveTint, ensuring the public acquireBrowserChrome
API retains its documentation.
In `@packages/ui/src/mosaic/components/dialog/dialog.styles.ts`:
- Around line 199-232: Trim the duplicated rationale comments in sizes.panel,
especially the repeated explanation around alignSelf: 'stretch', leaving one
concise line only where the non-obvious layout decision requires justification.
Apply the same minimal-comment standard to nearby oversized rationale blocks in
this file without changing the style declarations.
In `@packages/ui/src/mosaic/components/dialog/dialog.tsx`:
- Around line 205-229: Prevent consumers from overriding the internal render
used by CloseButton: omit render from DialogCloseButtonProps and exclude it from
the rest props spread, preserving the styled Button with styles.closeButton and
closeInsets[size].
In `@packages/ui/src/mosaic/components/dialog/keyboard-inset.ts`:
- Around line 75-82: Make the release function returned by the keyboard-inset
acquisition flow idempotent by adding a per-release guard, similar to
acquireBrowserChrome’s released flag. Only decrement listeners and detach the
keyboard-inset listener/property when that release has not already been
executed.
In `@packages/ui/src/mosaic/styles/index.ts`:
- Around line 17-30: Update the dialog type re-exports in the styles barrel to
include DialogFocusTarget, DialogHandle, and DialogOpenChangeDetails from the
existing dialog component exports, alongside the current DialogRootProps,
DialogTriggerProps, and DialogPopupProps types.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e06bfa40-bf96-4071-89f5-c32a12b34f03
📒 Files selected for processing (37)
.changeset/lucky-donuts-invite.md.changeset/olive-doors-tell.md.changeset/spicy-clocks-argue.mdpackages/headless/src/primitives/dialog/README.mdpackages/headless/src/primitives/dialog/dialog-backdrop.tsxpackages/headless/src/primitives/dialog/dialog-context.tspackages/headless/src/primitives/dialog/dialog-handle.tspackages/headless/src/primitives/dialog/dialog-popup.tsxpackages/headless/src/primitives/dialog/dialog-root.tsxpackages/headless/src/primitives/dialog/dialog-trigger.tsxpackages/headless/src/primitives/dialog/dialog-viewport.tsxpackages/headless/src/primitives/dialog/dialog.test.tsxpackages/headless/src/primitives/dialog/index.tspackages/headless/src/primitives/dialog/parts.tspackages/headless/src/primitives/dialog/use-dialog-origin.tspackages/headless/src/primitives/drawer/drawer-context.tspackages/headless/src/utils/interaction-modality.tspackages/swingset/src/stories/dialog.component.mdxpackages/swingset/src/stories/dialog.component.stories.tsxpackages/swingset/src/stories/dialog.mdxpackages/swingset/src/stories/dialog.stories.tsxpackages/ui/src/mosaic/block/destructive.tsxpackages/ui/src/mosaic/components/button/button.tsxpackages/ui/src/mosaic/components/dialog.tsxpackages/ui/src/mosaic/components/dialog/browser-chrome.tspackages/ui/src/mosaic/components/dialog/dialog.styles.tspackages/ui/src/mosaic/components/dialog/dialog.test.tsxpackages/ui/src/mosaic/components/dialog/dialog.tsxpackages/ui/src/mosaic/components/dialog/index.tspackages/ui/src/mosaic/components/dialog/keyboard-inset.tspackages/ui/src/mosaic/organization/organization-profile-domains-section-add-verify.view.tsxpackages/ui/src/mosaic/organization/organization-profile-domains-section-enrollment.view.tsxpackages/ui/src/mosaic/organization/organization-profile-domains-section-remove.view.tsxpackages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsxpackages/ui/src/mosaic/primitives/dialog.tsxpackages/ui/src/mosaic/styles/index.tspackages/ui/src/mosaic/tokens.stylex.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
💤 Files with no reviewable changes (2)
- packages/ui/src/mosaic/primitives/dialog.tsx
- packages/ui/src/mosaic/components/dialog.tsx
🛑 Comments failed to post (1)
packages/headless/src/primitives/dialog/README.md (1)
151-171: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document dismissal as conditional on
closedBy.
closedBy='none'disables both Escape and outside-press dismissal. The current documentation makes unconditional dismissal claims.
packages/headless/src/primitives/dialog/README.md#L151-L171: qualify the Keyboard section so Escape closes only whenclosedByis not'none'.packages/swingset/src/stories/dialog.component.mdx#L54-L55: replace “always” with behavior conditional on the defaultclosedBy='any'value.📍 Affects 2 files
packages/headless/src/primitives/dialog/README.md#L151-L171(this comment)packages/swingset/src/stories/dialog.component.mdx#L54-L55🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/dialog/README.md` around lines 151 - 171, Qualify the dialog dismissal documentation to reflect that Escape and outside-press dismissal depend on closedBy: in packages/headless/src/primitives/dialog/README.md lines 151-171, state that Escape closes only when closedBy is not 'none'; in packages/swingset/src/stories/dialog.component.mdx lines 54-55, replace the unconditional “always” wording with behavior conditional on the default closedBy='any' value.
1 participant
Description
Rebuilds the Mosaic
Dialogon StyleX — leavingTabsas the last component on theEmotion slot-recipe path — and adds Base UI–style composition APIs to both the headless
primitive and the styled component.
Styling and layout. The dialog's rules ship in
@clerk/ui/styles.css; style it via the.cl-dialog-*slot classes or per-partclassName/stylein place ofsx.sizebecomes
prompt/card/paneland moves toDialog.Root, since the backdrop reads ittoo. The gap to the screen edge is a fixed inset at three breakpoints rather than a
percentage of the viewport. A
panelclips and carries no padding — its scroll region iscomposed inside it from the ScrollArea atoms, which keeps anything anchored to the popup
from scrolling away and makes a sidebar a plain flex row.
Mobile. Below
48remapromptbecomes a bottom sheet. `Diaon-screen keyboard, so the sheet rises above it while a card re-centres and a panel shrinks.
The mobile browser's own chrome is tinted to match the scrim — de
rather than shipped as a colour, and reverting exactly on close.
Composition.
Dialog.createHandle()drives a dialog from a trigger it is not nestedunder. Multiple triggers can share one dialog through
id/pay the root's children as a function of the active trigger's payload.initialFocusandfinalFocusonDialog.Popup` take a bool, a ref, or a functionAlso. New
Dialog.CloseButton;data-nestedso stacked scri--cl-dialog-originso a dialog scales out of whatever opened it;Buttongainsxstylefor composing StyleX styles into its own. Fixes an enter/exit trat
was keyed to a
data-cl-starting-styleattribute the headless layer does not emit.TODO:
position: fixed, whichno longer covers the region beneath translucent chrome.
AlertDialogpreset, and close-confirmation built on top of it.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change