feat(ui): add Base UI-style composition APIs to the Dialog - #9419
Merged
Conversation
🦋 Changeset detectedLatest commit: 11fe160 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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.
|
11 tasks
maxyinger
force-pushed
the
max/dialog-composition-apis
branch
from
August 12, 2026 19:26
f563af4 to
f2bf54f
Compare
@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: |
maxyinger
force-pushed
the
max/dialog-composition-apis
branch
from
August 12, 2026 19:31
f2bf54f to
4fad362
Compare
Contributor
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
maxyinger
force-pushed
the
max/dialog-composition-apis
branch
from
August 12, 2026 19:44
4fad362 to
42de446
Compare
maxyinger
force-pushed
the
max/dialog-composition-apis
branch
from
August 12, 2026 20:36
42de446 to
e67f1fc
Compare
maxyinger
force-pushed
the
max/dialog-composition-apis
branch
from
August 12, 2026 20:45
e67f1fc to
8e7129c
Compare
alexcarpenter
approved these changes
Aug 12, 2026
maxyinger
added a commit
that referenced
this pull request
Aug 12, 2026
`Dialog.Root` does not take a `handle` — that arrives with the composition APIs in #9419, not here. `data-size` lands on the Viewport as well as the Popup: `themeProps` emits `data-<axis>` for every variant it is handed, and `size` is passed to both.
maxyinger
force-pushed
the
max/dialog-composition-apis
branch
from
August 12, 2026 20:59
8e7129c to
9b86dcd
Compare
maxyinger
added a commit
that referenced
this pull request
Aug 12, 2026
`Dialog.Root` does not take a `handle` — that arrives with the composition APIs in #9419, not here. `data-size` lands on the Viewport as well as the Popup: `themeProps` emits `data-<axis>` for every variant it is handed, and `size` is passed to both.
maxyinger
force-pushed
the
max/dialog-composition-apis
branch
from
August 12, 2026 21:04
9b86dcd to
0385d4c
Compare
maxyinger
force-pushed
the
max/dialog-composition-apis
branch
from
August 12, 2026 21:17
0385d4c to
4a1e4b1
Compare
maxyinger
force-pushed
the
max/dialog-composition-apis
branch
from
August 12, 2026 21:28
4a1e4b1 to
df51920
Compare
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughThe dialog API adds detached trigger handles, typed trigger payloads, trigger attribution in open-change details, and configurable initial and final focus targets. Headless dialog state now uses a trigger registry and centralized open-change handling. Mosaic Dialog propagates the new types and handle API. Tests, documentation, stories, and a Changesets entry were updated. Estimated code review effort: 4 (Complex) | ~60 minutes Mergeability Score: 🟡 Moderate · up to The new Dialog composition APIs change trigger attribution, payloads, and focus restoration; a controlled close can leave function-based final focus unresolved, potentially stranding keyboard focus. The release entry and API reference also need follow-up, so merge should wait for the focus issue and release/documentation readiness to be addressed or explicitly accepted. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/swingset/src/stories/dialog.component.mdx (1)
19-29: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the complete Mosaic Dialog composition API.
Lines 19-29 omit
handleandtriggerId. Thechildrentype also omits the active triggerpayload. Lines 186-191 omittriggerIdfromDialog.RootandidfromDialog.Trigger.Add these entries so consumers can configure detached triggers and controlled attribution from this API reference.
As per coding guidelines, “Update documentation for API changes.”
Also applies to: 184-192
🤖 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 19 - 29, Update the Dialog composition API documentation in the PropTable and the Dialog.Root/Dialog.Trigger sections. Add entries for handle and triggerId, include the active trigger payload in the children callback type, and document triggerId on Dialog.Root and id on Dialog.Trigger, preserving the existing formatting and defaults.Source: Coding guidelines
🧹 Nitpick comments (1)
packages/headless/src/primitives/dialog/dialog-root.tsx (1)
89-95: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCondense the rationale comment blocks.
The coding guidelines limit warranted comments to one terse line. These three blocks span 4 to 7 lines each. Keep the non-obvious reason and drop the narrative.
Example for Line 145-151: state only that the reference must be the active trigger and that the subscription is imperative to avoid a re-register/re-render loop.
As per coding guidelines: "Keep code comments minimal. 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."
Also applies to: 145-151, 162-165
🤖 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 89 - 95, Condense the rationale comments near pendingDetailsRef, the active-trigger subscription, and the open/close funnel to one terse line each. Preserve only the non-obvious reasons: retain the reference to the active trigger, keep the subscription imperative to avoid re-register/re-render loops, and note any necessary synchronous detail/focus ordering; remove narrative descriptions of routine behavior.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/spicy-clocks-argue.md:
- Around line 1-2: Add a non-empty changeset release entry in the empty front
matter of spicy-clocks-argue.md, assigning the appropriate version bump for
`@clerk/ui` and briefly describing the public Dialog API additions, including
handles, payloads, trigger attribution, and focus-target APIs.
In `@packages/headless/src/primitives/dialog/dialog-popup.tsx`:
- Around line 78-107: Update the function-valued finalFocus handling around
resolvedFinalFocusRef and finalFocusResolverRef to track whether resolution has
run, defaulting resolvedReturnFocus to returnFocusRef until it has. Reset that
resolution state whenever the dialog opens so controlled closes restore trigger
focus, while preserving a resolved null when the callback explicitly returns
false.
In `@packages/headless/src/primitives/dialog/dialog-trigger.tsx`:
- Around line 56-63: The registration effect in DialogTrigger must not
unregister and re-register when only payload changes. Keep the existing
store.registerTrigger subscription stable for the element and triggerId, and
update the registered payload through the store’s existing registration/update
mechanism without emitting transient unregister notifications or changing the
root reference to null.
In `@packages/headless/src/primitives/dialog/dialog.test.tsx`:
- Around line 641-650: Update Dialog.Close’s close path to pass the triggering
event’s nativeEvent into setOpen so programmatic pointer closes resolve
finalFocus with the native event type. In the test around the function-form
close case, rename the test to reflect native-event forwarding and change the
expected finalFocus argument from an empty string to 'mouse'.
In `@packages/swingset/src/stories/dialog.component.stories.tsx`:
- Around line 518-522: Add accessible labeling to both Input controls in the
dialog story: associate a visible form label or provide an aria-label
identifying the first field as Name and the feedbackRef field as Feedback, while
preserving their existing placeholders.
---
Outside diff comments:
In `@packages/swingset/src/stories/dialog.component.mdx`:
- Around line 19-29: Update the Dialog composition API documentation in the
PropTable and the Dialog.Root/Dialog.Trigger sections. Add entries for handle
and triggerId, include the active trigger payload in the children callback type,
and document triggerId on Dialog.Root and id on Dialog.Trigger, preserving the
existing formatting and defaults.
---
Nitpick comments:
In `@packages/headless/src/primitives/dialog/dialog-root.tsx`:
- Around line 89-95: Condense the rationale comments near pendingDetailsRef, the
active-trigger subscription, and the open/close funnel to one terse line each.
Preserve only the non-obvious reasons: retain the reference to the active
trigger, keep the subscription imperative to avoid re-register/re-render loops,
and note any necessary synchronous detail/focus ordering; remove narrative
descriptions of routine behavior.
🪄 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: 8f4703a4-ee88-4a51-b948-a3d2f067f0d1
📒 Files selected for processing (19)
.changeset/spicy-clocks-argue.mdpackages/headless/src/primitives/dialog/README.mdpackages/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.test.tsxpackages/headless/src/primitives/dialog/index.tspackages/headless/src/primitives/dialog/parts.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/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.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/clerk-ios(auto-detected)clerk/cli(auto-detected)clerk/clerk-android(auto-detected)
`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 replaces the popup shadow with one three-layer shadow shared by both
schemes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s openchange Drops `finalFocusResolverRef` from the dialog context. The popup now subscribes to floating-ui's `openchange` itself, which fires synchronously before the state commit, so the root no longer has to reach into the popup on every close. That lets programmatic `setOpen` route through `floatingContext.onOpenChange` like trigger activations and dismissals already did, which removes `applyOpenChange` and the ref-repointing dance that kept its closures fresh. Before, a programmatic close bypassed floating-ui's emit entirely, so no listener saw it. The resolved target is now materialised lazily, when the focus manager reads the ref at restore time, rather than eagerly at close. Ordering between this and `useReturnFocus` therefore stops mattering: the pointer-close downgrade has landed on the default by the time it is read. Also extracts `useInitialFocus` / `useFinalFocus` from the popup body, and drops `getRegistryVersion`, which nothing consumed.
…e bus Dialog's programmatic closes route through `floatingContext.onOpenChange`, so they do reach this listener — they just carry no event. Menu and Popover are the case the note described: their closes go through the consumer's own state setter and never reach the bus at all.
`useReturnFocus` now requires a floating-ui `reason` alongside the event before it drops the trigger. A reason marks a close its interaction hooks drove — an outside press, a trigger press — which is the case that should leave focus where the pointer put it. A `Dialog.Close` press is not that: focus is inside the dialog and about to be destroyed, so it belongs back on the trigger. `Dialog.Close` forwards its click through `setOpen`, which now takes an optional event, so `finalFocus` can still resolve the interaction type behind the close while the default keeps the trigger. Programmatic closes carry neither event nor reason and resolve to the empty type, as before. Trigger registrations hand out a `getPayload()` getter rather than a payload snapshot, so an inline-literal `payload` no longer re-registers the trigger on every render — which is what the root's imperative subscription was working around. That also lets the `exhaustive-deps` disables in the root go: re-running `setRoot` is an idempotent swap.
maxyinger
force-pushed
the
max/dialog-composition-apis
branch
from
August 12, 2026 23:01
9cee303 to
11fe160
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds Base UI-style composition APIs to the headless
Dialog, split out of #9388 so the runtimechanges are reviewed on their own rather than alongside that PR's styling work.
Detached triggers.
Dialog.createHandle()returns a handle; pass the same one to aDialog.Triggerand aDialog.Rootand the trigger drives the dialog from anywhere in the tree,with no JSX nesting. The handle also carries imperative
open()/close()/isOpenfor openswith no trigger element at all. A root with no
handlecreates a private one, so nested anddetached triggers share a single registration and open/close path.
Multiple triggers and payloads. Each trigger can carry an
idand apayload, and the root'schildren can be a function of the active trigger's payload — one dialog, per-trigger content.
onOpenChangegains a details argument naming the trigger behind each change, andtriggerIdonthe root drives the attribution in controlled mode.
Custom focus management.
initialFocusandfinalFocusonDialog.Popuptaketrue,false,a ref, or a function of the interaction type behind the open/close.
Notable internals: trigger activation moves off floating-ui's
useClick, since a detached triggercannot collect reference props from context. Every open and close — trigger, dismissal, programmatic
— now funnels through one path so the
onOpenChangedetails and thefinalFocusresolution eachhappen exactly once, synchronously, before focus restoration can run. The trigger registry is
subscribed to imperatively rather than via
useSyncExternalStore, so a trigger with an inline-objectpayloadcannot loop with the root re-registering it.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change