feat(ui): add dialog closedBy dismissal policy - #9366
Open
maxyinger wants to merge 1 commit into
Open
Conversation
Adds `closedBy: 'any' | 'closerequest' | 'none'` to the headless Dialog root, driving `escapeKey` and `outsidePress` on `useDismiss`. Defaults to `any`, so existing callers are unaffected. The five Mosaic dialogs now use `closerequest`, which stops a stray backdrop click from discarding the type-to-confirm input in `Destructive` or closing a dialog mid-request. `trigger` on the Mosaic `Dialog` becomes optional, so the machine-driven dialogs no longer render a button they don't use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 38e7c1f The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
@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: |
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 |
Contributor
📝 WalkthroughWalkthroughThe headless dialog adds Estimated code review effort: 3 (Moderate) | ~20 minutes 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: 1
🤖 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 `@packages/headless/src/primitives/dialog/dialog-root.tsx`:
- Around line 21-31: Update the shared closedBy='none' contract in
packages/headless/src/primitives/dialog/dialog-root.tsx lines 21-31,
packages/headless/src/primitives/dialog/README.md lines 74-94, and
.changeset/lucky-donuts-invite.md line 5: state that Escape and outside-press
dismissal are disabled while explicit close controls such as Dialog.Close remain
available. Use consistent wording across all three sites.
🪄 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: 16d1a9d7-2fb9-415c-8aef-4a6c79b99611
📒 Files selected for processing (14)
.changeset/lucky-donuts-invite.mdpackages/headless/src/primitives/dialog/README.mdpackages/headless/src/primitives/dialog/dialog-root.tsxpackages/headless/src/primitives/dialog/dialog.test.tsxpackages/headless/src/primitives/dialog/index.tspackages/headless/src/primitives/dialog/parts.tspackages/swingset/src/stories/dialog.mdxpackages/swingset/src/stories/dialog.stories.tsxpackages/ui/src/mosaic/block/destructive.tsxpackages/ui/src/mosaic/components/dialog.tsxpackages/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.tsx
🔗 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)
Comment on lines
+21
to
+31
| /** | ||
| * Which gestures dismiss the dialog, mirroring the native `<dialog closedby>` attribute. | ||
| * | ||
| * - `any` — Escape and outside press | ||
| * - `closerequest` — Escape only | ||
| * - `none` — neither; the dialog closes only programmatically | ||
| * | ||
| * A single ordered enum rather than two booleans, so the fourth combination — outside press | ||
| * dismisses but Escape does not — stays unrepresentable. Dismissing by pointer but not by | ||
| * keyboard is not something to offer. | ||
| */ |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the shared none dismissal description.
closedBy='none' disables Escape and outside press. It does not disable explicit close controls such as Dialog.Close.
packages/headless/src/primitives/dialog/dialog-root.tsx#L21-L31: replace “only programmatically” with wording that preserves explicit close controls.packages/headless/src/primitives/dialog/README.md#L74-L94: describe the disabled dismissal gestures and explicit close behavior..changeset/lucky-donuts-invite.md#L5-L5: use the same corrected contract in the release note.
📍 Affects 3 files
packages/headless/src/primitives/dialog/dialog-root.tsx#L21-L31(this comment)packages/headless/src/primitives/dialog/README.md#L74-L94.changeset/lucky-donuts-invite.md#L5-L5
🤖 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 21 -
31, Update the shared closedBy='none' contract in
packages/headless/src/primitives/dialog/dialog-root.tsx lines 21-31,
packages/headless/src/primitives/dialog/README.md lines 74-94, and
.changeset/lucky-donuts-invite.md line 5: state that Escape and outside-press
dismissal are disabled while explicit close controls such as Dialog.Close remain
available. Use consistent wording across all three sites.
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
1 participant
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
closedByto the Dialog, controlling which gestures dismiss it. Mirrors the native<dialog closedby>attribute:any(default)closerequestnoneA single ordered enum rather than two booleans, so the fourth combination — outside press dismisses but Escape does not — stays unrepresentable.
The five Mosaic dialogs now use
closerequest. This fixes a live bug inDestructive, where a stray backdrop click discarded the type-to-confirm input or closed the dialog mid-delete.triggeron the MosaicDialogis now optional, so the machine-driven dialogs stop rendering a button they never use.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change