docs(swingset): align dialog docs with archetype guidance#8892
Open
kylemac wants to merge 1 commit into
Open
Conversation
Restructure the Mosaic Dialog component page (/components/dialog) to the knob-driven archetype: live Preview playground with an interactive PropTable above the examples. Correct the headless Dialog primitive page (/primitives/dialog) to document Dialog.Viewport, fix scroll-lock ownership, and clarify that data-cl-slot is applied by the styled layer.
🦋 Changeset detectedLatest commit: 05ac390 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.
|
Contributor
📝 WalkthroughWalkthroughDocumentation-only update to the Dialog headless primitive: restructures the usage example to nest ChangesDialog Documentation Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/expo
@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: |
![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: 3
🤖 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/swingset/src/stories/dialog.component.mdx`:
- Around line 10-31: The dialog.component.mdx file is missing the required
Examples section which is mandatory for Archetype A documentation. The file
currently contains Playground, Props, and Usage sections, but according to
guidelines, the Examples section must be included after Usage. Add an Examples
section after the Usage section heading with relevant code examples
demonstrating how to use the Dialog component in different scenarios.
- Around line 19-28: The PropTable component in the dialog.component.mdx file
has an extra array with prop definitions where some props are missing the
default field. Add a default property to each prop object in the extra array
that currently lacks one (trigger, children, open, and onOpenChange). For props
without a meaningful default value, use "—" as the default to comply with the
props table documentation standards. Ensure all prop objects in the extra array
now include an explicit default field.
In `@packages/swingset/src/stories/dialog.mdx`:
- Around line 5-6: The documentation in the Dialog component description
incorrectly presents focus trapping as unconditional behavior, but the component
supports a modal={false} option where focus is not trapped. Locate the
introductory description around line 5-6 that states the component "traps focus
until dismissed" and the Dialog.Popup description around line 69, then add
qualifying language such as "in modal mode" or "by default" to both locations to
clarify that focus trapping only occurs when the modal prop is enabled. This
ensures the documentation accurately reflects that focus trapping is conditional
behavior rather than a default always-on feature.
🪄 Autofix (Beta)
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), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 3edb52ee-d884-4250-bbd1-378170795c4b
📒 Files selected for processing (3)
.changeset/bold-horses-rhyme.mdpackages/swingset/src/stories/dialog.component.mdxpackages/swingset/src/stories/dialog.mdx
Comment on lines
+10
to
31
| ## Playground | ||
|
|
||
| <Story | ||
| <Preview | ||
| name='Default' | ||
| storyModule={DialogStories} | ||
| /> | ||
|
|
||
| ## Props | ||
|
|
||
| <PropTable | ||
| meta={DialogStories.meta} | ||
| extra={[ | ||
| { name: 'trigger', type: '(props: HTMLAttributes<HTMLElement>) => ReactElement' }, | ||
| { name: 'children', type: 'ReactNode | ((ctx: { close: () => void }) => ReactNode)' }, | ||
| { name: 'open', type: 'boolean' }, | ||
| { name: 'defaultOpen', type: 'boolean', default: 'false' }, | ||
| { name: 'onOpenChange', type: '(open: boolean) => void' }, | ||
| { name: 'modal', type: 'boolean', default: 'true' }, | ||
| ]} | ||
| /> | ||
|
|
||
| ## Usage |
Contributor
There was a problem hiding this comment.
Add the required Examples section for Archetype A docs.
This page currently has Playground → Props → Usage but omits Examples, which is required for Archetype A MDX docs.
As per coding guidelines: “For Archetype A (simple CVA) MDX files, required sections in order: Playground, Props, Usage, then Examples. Never reorder or omit these sections.”
🤖 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 10 - 31, The
dialog.component.mdx file is missing the required Examples section which is
mandatory for Archetype A documentation. The file currently contains Playground,
Props, and Usage sections, but according to guidelines, the Examples section
must be included after Usage. Add an Examples section after the Usage section
heading with relevant code examples demonstrating how to use the Dialog
component in different scenarios.
Source: Coding guidelines
Comment on lines
+19
to
+28
| <PropTable | ||
| meta={DialogStories.meta} | ||
| extra={[ | ||
| { name: 'trigger', type: '(props: HTMLAttributes<HTMLElement>) => ReactElement' }, | ||
| { name: 'children', type: 'ReactNode | ((ctx: { close: () => void }) => ReactNode)' }, | ||
| { name: 'open', type: 'boolean' }, | ||
| { name: 'defaultOpen', type: 'boolean', default: 'false' }, | ||
| { name: 'onOpenChange', type: '(open: boolean) => void' }, | ||
| { name: 'modal', type: 'boolean', default: 'true' }, | ||
| ]} |
Contributor
There was a problem hiding this comment.
Provide defaults for every prop in the generated props table input.
In extra, several props (trigger, children, open, onOpenChange) do not declare a default. Please set explicit defaults (typically —) so the rendered table stays compliant and unambiguous.
As per coding guidelines: “In all props tables, document the default value for every prop in a dedicated Default column … use — when there is no default.”
🤖 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 - 28, The
PropTable component in the dialog.component.mdx file has an extra array with
prop definitions where some props are missing the default field. Add a default
property to each prop object in the extra array that currently lacks one
(trigger, children, open, and onOpenChange). For props without a meaningful
default value, use "—" as the default to comply with the props table
documentation standards. Ensure all prop objects in the extra array now include
an explicit default field.
Source: Coding guidelines
Comment on lines
5
to
6
| A modal window that overlays the page and traps focus until dismissed, from | ||
| `@clerk/headless`. It is a **headless** primitive: it supplies open state, portalling, an |
Contributor
There was a problem hiding this comment.
Clarify focus-trap wording as modal-only behavior.
The intro and Dialog.Popup description read as unconditional focus trapping, but this page also documents modal={false} where focus is not trapped. Please qualify these lines as “in modal mode” (or “by default”).
Also applies to: 69-69
🤖 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.mdx` around lines 5 - 6, The
documentation in the Dialog component description incorrectly presents focus
trapping as unconditional behavior, but the component supports a modal={false}
option where focus is not trapped. Locate the introductory description around
line 5-6 that states the component "traps focus until dismissed" and the
Dialog.Popup description around line 69, then add qualifying language such as
"in modal mode" or "by default" to both locations to clarify that focus trapping
only occurs when the modal prop is enabled. This ensures the documentation
accurately reflects that focus trapping is conditional behavior rather than a
default always-on feature.
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.
TLDR
Description
Brings the two swingset Dialog doc pages in line with the documentation archetypes in
packages/swingset/CLAUDE.md. The Mosaic component page (/components/dialog) is restructured to the knob-driven archetype: a live<Preview>playground followed by an interactive<PropTable>(knobValuecolumn, no description) positioned above the rest of the page. The headless primitive page (/primitives/dialog) is corrected against the actual@clerk/headlesssource — it now documents the missingDialog.Viewportpart, moveslockScrollfromBackdroptoViewport(which actually owns scroll lock), drops a fabricated Portal "scoped mode," and clarifies thatdata-cl-slotis applied by the styled Mosaic layer (not the headless parts). To test: runpnpm run dev:swingsetand visit/components/dialog(confirm thesizeknob drives the preview) and/primitives/dialog. Docs-only change to a private, unpublished package, so the changeset is empty.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change