feat(ui): Mosaic Settings component - #9377
Conversation
🦋 Changeset detectedLatest commit: c3954e5 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.
|
e014d4e to
cfe215d
Compare
austincalvelage
changed the title
cfe215d to
1ef2b96
Compare
952b17c to
4bf4850
Compare
austincalvelage
changed the title
4bf4850 to
c3ce60e
Compare
f592827 to
34b1d21
Compare
c3ce60e to
a3b0bd3
Compare
34b1d21 to
c234942
Compare
a3b0bd3 to
ef07a60
Compare
maxyinger
left a comment
maxyinger
left a comment
There was a problem hiding this comment.
lgtm.
Heads up designs might be changing here tomorrow
https://clerkinc.slack.com/archives/C0BHNCTDZ89/p1786470840387689
austincalvelage
changed the title
Settings block
ef07a60 to
5a6ccdb
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: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
📝 WalkthroughWalkthroughAdded a composable Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 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: 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 @.changeset/settings-block.md:
- Around line 1-2: Update the changeset frontmatter in settings-block.md to
include an appropriate release bump for `@clerk/ui`, and add a concise release
note describing the new public Settings API.
In `@packages/swingset/src/stories/settings.mdx`:
- Around line 7-74: Reorganize the Settings compound-component page to use the
required top-level headings in order: Example, Usage, Parts, and Styling. Move
the Default story under Example, document Settings subcomponents under Parts,
and move the --cl-settings-items-gap guidance under Styling while preserving the
existing usage and example stories and the archetype required by meta.group.
In `@packages/ui/src/mosaic/block/settings.tsx`:
- Around line 149-180: Update SettingsTitle and the Settings root labeling flow
so a title ID is available during SSR, allowing aria-labelledby to reference it
before hydration when no explicit aria-label is provided; keep IDs stable
between server and client and preserve explicit labels. Add an SSR regression
test covering matching aria-labelledby and title id output.
🪄 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: 73fe7bf6-67e0-4c05-bc0b-15f4ede55be6
📒 Files selected for processing (12)
.changeset/settings-block.mdpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/settings.mdxpackages/swingset/src/stories/settings.stories.tsxpackages/ui/src/mosaic/block/settings.markers.stylex.tspackages/ui/src/mosaic/block/settings.test.tsxpackages/ui/src/mosaic/block/settings.tsxpackages/ui/src/mosaic/block/settings.vars.stylex.tspackages/ui/src/mosaic/components/reset.test.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/clerk-ios(auto-detected)clerk/cli(auto-detected)clerk/clerk-android(auto-detected)
| --- | ||
| --- |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add a release entry for @clerk/ui.
This PR adds the public Settings API. An empty changeset produces no @clerk/ui release or changelog entry. Add the appropriate version bump and a concise release note.
Proposed changeset
---
+"`@clerk/ui`": minor
---
+
+Add the Settings Mosaic block.As per coding guidelines, “Use Changesets for version management and changelogs.” Based on learnings, empty changesets apply only when changes do not include functionality changes.
📝 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.
| --- | |
| --- | |
| --- | |
| "`@clerk/ui`": minor | |
| --- | |
| Add the Settings Mosaic 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 @.changeset/settings-block.md around lines 1 - 2, Update the changeset
frontmatter in settings-block.md to include an appropriate release bump for
`@clerk/ui`, and add a concise release note describing the new public Settings
API.
Sources: Coding guidelines, Learnings
| ## Usage | ||
|
|
||
| ```tsx | ||
| import { Settings } from '@clerk/ui/mosaic/block/settings'; | ||
|
|
||
| <Settings.Root> | ||
| <Settings.Title>Account</Settings.Title> | ||
| <Settings.Group> | ||
| <Settings.Row> | ||
| <Settings.Item> | ||
| <Settings.Media>{icon}</Settings.Media> | ||
| <Settings.Content> | ||
| <Settings.Label>Name</Settings.Label> | ||
| <Settings.Description>Shown throughout the application.</Settings.Description> | ||
| </Settings.Content> | ||
| <Settings.Actions>{control}</Settings.Actions> | ||
| </Settings.Item> | ||
| </Settings.Row> | ||
| </Settings.Group> | ||
| </Settings.Root> | ||
| ``` | ||
|
|
||
| `Settings.Group` owns the outlined surface. Its direct `Settings.Row` children own separation and may stack one or more `Settings.Item` entries. Each item lays out optional media, flexible content, and trailing actions. `Settings.Media` accepts `md` and `lg` sizes. | ||
|
|
||
| Use `Settings.Items` for a nested value list beneath a row's header item. The row keeps an 8px header-to-list gap, nested items stack without gaps at a 44px minimum height, and their content receives 12px vertical padding. | ||
|
|
||
| Override `--cl-settings-items-gap` on `Settings.Root` or a specific `Settings.Row` to customize the space between the row header and its nested value list. It defaults to 8px. | ||
|
|
||
| ## Examples | ||
|
|
||
| <Story | ||
| name='Default' | ||
| storyModule={SettingsStories} | ||
| composition={[ | ||
| { name: 'Avatar', href: '/components/avatar', layer: 'Components' }, | ||
| { name: 'Button', href: '/components/button', layer: 'Components' }, | ||
| ]} | ||
| /> | ||
|
|
||
| ### Multiple email addresses and phone numbers | ||
|
|
||
| <Story | ||
| name='MultipleEmailAndPhoneNumbers' | ||
| storyModule={SettingsStories} | ||
| composition={[ | ||
| { name: 'Badge', href: '/components/badge', layer: 'Components' }, | ||
| { name: 'Button', href: '/components/button', layer: 'Components' }, | ||
| { name: 'Icon', href: '/components/icon', layer: 'Components' }, | ||
| ]} | ||
| /> | ||
|
|
||
| ### Connected accounts | ||
|
|
||
| <Story | ||
| name='ConnectedAccounts' | ||
| storyModule={SettingsStories} | ||
| composition={[ | ||
| { name: 'Button', href: '/components/button', layer: 'Components' }, | ||
| { name: 'Icon', href: '/components/icon', layer: 'Components' }, | ||
| ]} | ||
| /> | ||
|
|
||
| ### Destructive setting | ||
|
|
||
| <Story | ||
| name='Destructive' | ||
| storyModule={SettingsStories} | ||
| /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the compound component page structure.
Settings is a compound component. This page starts with Usage and Examples and omits Example, Parts, and Styling.
Use Example, Usage, Parts, and Styling as the top-level headings in that order. Put Default in Example. Document subcomponents in Parts. Document --cl-settings-items-gap in Styling.
As per coding guidelines, “Compound Components pages must use the exact section order: Example, Usage, Parts, then Styling.” Based on learnings, preserve the archetype required by meta.group.
🤖 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/settings.mdx` around lines 7 - 74, Reorganize
the Settings compound-component page to use the required top-level headings in
order: Example, Usage, Parts, and Styling. Move the Default story under Example,
document Settings subcomponents under Parts, and move the
--cl-settings-items-gap guidance under Styling while preserving the existing
usage and example stories and the archetype required by meta.group.
Sources: Coding guidelines, Learnings
| const [titleIds, setTitleIds] = React.useState<string[]>([]); | ||
|
|
||
| const element = useRender({ | ||
| defaultTagName: 'section', | ||
| render, | ||
| ref, | ||
| props: { | ||
| ...mergeStyleProps(themeProps('settings'), stylex.props(reset.base, styles.root), className, style), | ||
| ...rest, | ||
| 'aria-label': ariaLabel, | ||
| 'aria-labelledby': ariaLabelledBy ?? (ariaLabel ? undefined : titleIds.join(' ') || undefined), | ||
| }, | ||
| }); | ||
|
|
||
| return <SettingsTitleContext.Provider value={setTitleIds}>{element}</SettingsTitleContext.Provider>; | ||
| }); | ||
|
|
||
| const Title = React.forwardRef<HTMLHeadingElement, SettingsTitleProps>(function SettingsTitle( | ||
| { id: idProp, render, className, style, ...rest }, | ||
| ref, | ||
| ) { | ||
| const setTitleIds = React.useContext(SettingsTitleContext); | ||
| const generatedId = React.useId(); | ||
| const id = idProp ?? (setTitleIds ? `cl-settings-${generatedId}-title` : undefined); | ||
|
|
||
| useSafeLayoutEffect(() => { | ||
| if (!id || !setTitleIds) { | ||
| return undefined; | ||
| } | ||
|
|
||
| setTitleIds(ids => (ids.includes(id) ? ids : [...ids, id])); | ||
| return () => setTitleIds(ids => ids.filter(value => value !== id)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target outline ---'
ast-grep outline packages/ui/src/mosaic/block/settings.tsx || true
printf '%s\n' '--- target section ---'
sed -n '1,230p' packages/ui/src/mosaic/block/settings.tsx
printf '%s\n' '--- related references ---'
rg -n --glob '*.{ts,tsx}' 'SettingsTitleContext|Settings\.Title|SettingsTitle|titleIds|renderToStaticMarkup' packages/ui/src/mosaic packages/ui/src | head -250Repository: clerk/javascript
Length of output: 12745
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- settings tests ---'
cat -n packages/ui/src/mosaic/block/settings.test.tsx
printf '%s\n' '--- safe layout effect implementation ---'
rg -n --glob '*.{ts,tsx,js}' 'function useSafeLayoutEffect|const useSafeLayoutEffect|useSafeLayoutEffect' packages/shared packages | head -120
printf '%s\n' '--- React and test setup ---'
rg -n '"react"|"react-dom"|renderToString|renderToStaticMarkup|hydrate' package.json packages/ui/package.json packages/ui/src --glob '*.{json,ts,tsx,js}' | head -200Repository: clerk/javascript
Length of output: 12994
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- useRender implementation and call shape ---'
rg -n --glob '*.{ts,tsx}' 'export .*useRender|function useRender|const useRender' packages/headless packages/ui packages | head -80
printf '%s\n' '--- SSR test conventions ---'
cat -n packages/ui/src/mosaic/components/field/field.ssr.test.tsx
cat -n packages/ui/src/mosaic/__tests__/MosaicProvider.ssr.test.tsx
printf '%s\n' '--- source-level SSR verifier ---'
python3 - <<'PY'
from pathlib import Path
p = Path('packages/ui/src/mosaic/block/settings.tsx')
s = p.read_text()
checks = {
'Root initializes titleIds to empty': 'React.useState<string[]>([])' in s,
'Root derives aria-labelledby from titleIds': "titleIds.join(' ') || undefined" in s,
'Title registration uses useSafeLayoutEffect': 'useSafeLayoutEffect(() =>' in s,
'Title registration is not performed during render': 'setTitleIds(ids =>' not in s.split('useSafeLayoutEffect(() =>', 1)[0],
}
for name, ok in checks.items():
print(f'{name}: {ok}')
print('Conclusion: SSR markup cannot obtain a title ID from this state/effect path.')
PYRepository: clerk/javascript
Length of output: 5888
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- useRender server-relevant implementation ---'
sed -n '150,270p' packages/headless/src/utils/use-render.tsx
printf '%s\n' '--- generated ID and relationship patterns ---'
rg -n --glob '*.{ts,tsx}' 'React\.useId\(\)|aria-labelledby.*useId|createContext.*string|Context.*string' packages/ui/src/mosaic packages/ui/src | head -160
printf '%s\n' '--- current settings SSR coverage ---'
rg -n 'Mosaic Settings|Settings SSR|Settings\.Root|aria-labelledby' packages/ui/src/mosaic --glob '*.{test,spec}.{ts,tsx}' || trueRepository: clerk/javascript
Length of output: 11961
🌐 Web query:
HTML section element accessible name implicit region role aria-labelledby server rendered heading accessible name
💡 Result:
The HTML section element does not have an implicit landmark role [1][2]. It only functions as a region landmark (with an implicit role of region) when it is given an accessible name [3][4]. Accessible Name and Region Role The section element requires an accessible name—provided via attributes like aria-labelledby, aria-label, or title—to be exposed as a region to assistive technology (AT) [3][1][5]. Without an accessible name, the section element is treated as a generic container (like a div) and is not included in landmark navigation [5][2]. Because "region" is a generic landmark, it should be used sparingly to avoid creating "noise" in screen readers [6][5][7]. Overusing region landmarks can make it difficult for users to navigate the page effectively; therefore, developers should only define a section as a landmark when it is important enough for a user to want to navigate directly to it [6][5]. Using aria-labelledby When a section requires a name, aria-labelledby is the preferred method, especially when a visible heading already exists within the section [6][8]. By referencing the ID of that heading, you provide both a clear programmatic name and maintain consistency between the visual structure and the accessibility tree [6][9]. Server-Rendered Headings For server-rendered content, ensuring your heading elements have unique IDs and that corresponding section elements use aria-labelledby to point to those IDs is a best practice for creating robust, predictable accessibility [6][8]. While adding an accessible name to every section is technically possible, it is often discouraged if the section is not significant enough to warrant being a landmark, as unnecessary landmarks can hinder rather than help user navigation [5][7][10]. Top Results: [3] https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/ [6] https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/region_role [1] https://a11ysupport.io/tech/html/section_element [5] https://www.scottohara.me/blog/2021/07/16/section.html [4] https://w3c.github.io/wcag/techniques/html/H101 [2] https://www.stefanjudis.com/today-i-learned/section-accessible-name/ [7] https://web.dev/learn/html/headings-and-sections?hl=ar [8] https://www.w3.org/TR/WCAG20-TECHS/ARIA13
Citations:
- 1: https://a11ysupport.io/tech/html/section_element
- 2: https://www.stefanjudis.com/today-i-learned/section-accessible-name/
- 3: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
- 4: https://w3c.github.io/wcag/techniques/html/H101
- 5: https://www.scottohara.me/blog/2021/07/16/section.html
- 6: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/region_role
- 7: https://web.dev/learn/html/headings-and-sections?hl=ar
- 8: https://www.w3.org/TR/WCAG20-TECHS/ARIA13
- 9: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Attributes/aria-labelledby
- 10: Add aria-labelledby on every section which already has a visible heading? w3c/aria-practices#995
Preserve the Settings region name during SSR.
When Settings.Root has no explicit label, Settings.Title registers its ID only in useSafeLayoutEffect. Server markup therefore omits aria-labelledby, so the <section> is not exposed as the named region available after hydration. Generate a stable title ID during render, or require an explicit root label. Add an SSR regression test for the matching aria-labelledby and title ID.
🤖 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/block/settings.tsx` around lines 149 - 180, Update
SettingsTitle and the Settings root labeling flow so a title ID is available
during SSR, allowing aria-labelledby to reference it before hydration when no
explicit aria-label is provided; keep IDs stable between server and client and
preserve explicit labels. Add an SSR regression test covering matching
aria-labelledby and title id output.
Source: Coding guidelines
Relocates the Settings compound component out of `mosaic/block` into `mosaic/components/settings`, and moves its swingset docs entry from the Blocks layer to Components.
alexcarpenter
changed the title
Settings blockSettings component
alexcarpenter
left a comment
alexcarpenter
left a comment
There was a problem hiding this comment.
moved this into components vs blocks. similar to like the card component for example.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/mosaic/components/settings/settings.test.tsx (1)
9-16: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the token alias target, not only variable names.
These assertions verify that
settingsVarsexposes self-named CSS variables. They do not prove thatsettingsVars.backgroundresolves fromcolorVars.backgroundor thatsettingsVars.itemsGapresolves fromspace['2']. An incorrect source mapping would still pass. Assert the compiled StyleX declaration or the resolved CSS variable relationship.🤖 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/settings/settings.test.tsx` around lines 9 - 16, Update the test around the colorVars and settingsVars assertions to verify the compiled StyleX declaration or resolved CSS variable relationship: confirm the settings background aliases colorVars.background and settings itemsGap aliases space['2'], rather than asserting self-referential variable names. Preserve coverage of both settings tokens while ensuring incorrect source mappings fail.
🧹 Nitpick comments (1)
packages/ui/src/mosaic/components/settings/index.ts (1)
1-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid adding a component-level re-export barrel.
As per coding guidelines,
**/index.tsfiles should avoid barrel re-exports because they can cause circular dependencies. Move these exports to the established Mosaic entry point, or document an explicit exception if this path is required for the public API.🤖 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/settings/index.ts` around lines 1 - 15, Remove the component-level re-exports from the settings index.ts barrel, and expose Settings and its associated types through the established Mosaic entry point instead. If this index.ts path is required as part of the public API, document an explicit exception rather than retaining the undocumented barrel.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.
Outside diff comments:
In `@packages/ui/src/mosaic/components/settings/settings.test.tsx`:
- Around line 9-16: Update the test around the colorVars and settingsVars
assertions to verify the compiled StyleX declaration or resolved CSS variable
relationship: confirm the settings background aliases colorVars.background and
settings itemsGap aliases space['2'], rather than asserting self-referential
variable names. Preserve coverage of both settings tokens while ensuring
incorrect source mappings fail.
---
Nitpick comments:
In `@packages/ui/src/mosaic/components/settings/index.ts`:
- Around line 1-15: Remove the component-level re-exports from the settings
index.ts barrel, and expose Settings and its associated types through the
established Mosaic entry point instead. If this index.ts path is required as
part of the public API, document an explicit exception rather than retaining the
undocumented barrel.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e20f7233-89eb-435b-ad8b-e05237cac928
📒 Files selected for processing (11)
packages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/settings.mdxpackages/swingset/src/stories/settings.stories.tsxpackages/ui/src/mosaic/components/reset.test.tsxpackages/ui/src/mosaic/components/settings/index.tspackages/ui/src/mosaic/components/settings/settings.markers.stylex.tspackages/ui/src/mosaic/components/settings/settings.test.tsxpackages/ui/src/mosaic/components/settings/settings.tsxpackages/ui/src/mosaic/components/settings/settings.vars.stylex.tspackages/ui/src/mosaic/styles/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)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/swingset/src/stories/settings.stories.tsx
- packages/ui/src/mosaic/styles/index.ts
- packages/swingset/src/lib/registry.ts
- packages/swingset/src/stories/settings.mdx
- packages/ui/src/mosaic/components/reset.test.tsx
3 participants
Description
https://swingset-git-austin-user-profile-02-settings-group.clerkstage.dev/blocks/settings
Adds the composable Mosaic
Settingsblock:Settings.Root,Settings.Title,Settings.Group,Settings.Row,Settings.Item,Settings.Media,Settings.Content,Settings.Label,Settings.Description, andSettings.Actionsinterface.Settings.Titleto the section with a generated ID andaria-labelledby.fieldprop.Settings.Itemchildren. The first item is the row header and subsequent items receive their nested spacing and sizing automatically.Single item:
Multiple items:
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change