refactor(ui): rename Mosaic Item.Title to Item.Label - #9412
Conversation
🦋 Changeset detectedLatest commit: 19c957d 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.
|
📝 WalkthroughWalkthroughThe Mosaic Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
`Item.Title` becomes `Item.Label`. The part that already held that name, the quiet text on a bare action row, folds into the new `Item.Label` as `variant='secondary'` rather than becoming a slot of its own. The two only ever differed by how loud they read, so an axis on one part models that better than two parts. `secondary` still declares no color, which is what keeps it on the row's own color and carries it through the row's hover promotion.
3d3cd11 to
19c957d
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
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/user-button/user-button.view.tsx (1)
295-298: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpose the
ActionRowpending state.When
busyis true, the non-link row becomesaria-disabled, but its spinner remainsaria-hiddenand the row has noaria-busystate. A screen-reader user can hear only an unavailable action, not that the action is in progress. Use the same accessible progress indicator pattern asWorkspaceRow.Proposed fix
<Item.Root size='xs' + aria-busy={busy || undefined} render={href ? asAnchor(href) : rowButton(busy || disabled)} onClick={onClick} > - <Item.Media>{busy ? <Spinner size='sm' /> : icon}</Item.Media> + <Item.Media> + {busy ? ( + <Spinner + role='progressbar' + aria-hidden={undefined} + aria-label={m.workspaces.pending} + size='sm' + /> + ) : ( + icon + )} + </Item.Media>🤖 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/user-button/user-button.view.tsx` around lines 295 - 298, Update the ActionRow rendering around the busy-state logic in user-button.view.tsx to expose progress accessibly: ensure the non-link row indicates aria-busy when busy and make the busy Spinner discoverable to assistive technologies, following the accessible progress-indicator pattern used by WorkspaceRow while preserving existing disabled behavior.
🤖 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/ui/src/mosaic/user-button/user-button.view.tsx`:
- Around line 187-188: Update the JSDoc for the UserButton view’s labelId prop
to describe the referenced control target as a “label element” instead of a
“title element,” matching the Item.Title-to-Item.Label migration and current
accessibility contract.
---
Outside diff comments:
In `@packages/ui/src/mosaic/user-button/user-button.view.tsx`:
- Around line 295-298: Update the ActionRow rendering around the busy-state
logic in user-button.view.tsx to expose progress accessibly: ensure the non-link
row indicates aria-busy when busy and make the busy Spinner discoverable to
assistive technologies, following the accessible progress-indicator pattern used
by WorkspaceRow while preserving existing disabled 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: cac96b62-abc2-4ac2-b976-bb4af6f9dd6c
📒 Files selected for processing (2)
packages/ui/src/mosaic/user-button/__tests__/user-button.view.test.tsxpackages/ui/src/mosaic/user-button/user-button.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)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui/src/mosaic/user-button/tests/user-button.view.test.tsx
| /** Names the title element, for a control in the row that has to point at the workspace it acts on. */ | ||
| titleId?: string; | ||
| labelId?: string; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the labelId documentation.
The prop now identifies Item.Label, but its JSDoc still calls the target a “title element”. Use “label element” so the accessibility contract matches the Item.Title to Item.Label migration.
Proposed wording
- /** Names the title element, for a control in the row that has to point at the workspace it acts on. */
+ /** Identifies the workspace label for controls that act on this row. */📝 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.
| /** Names the title element, for a control in the row that has to point at the workspace it acts on. */ | |
| titleId?: string; | |
| labelId?: string; | |
| /** Identifies the workspace label for controls that act on this row. */ | |
| labelId?: string; |
🤖 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/user-button/user-button.view.tsx` around lines 187 -
188, Update the JSDoc for the UserButton view’s labelId prop to describe the
referenced control target as a “label element” instead of a “title element,”
matching the Item.Title-to-Item.Label migration and current accessibility
contract.
Description
Item.TitlebecomesItem.Label.That name was already taken by a second part: the quiet text on a bare action row (
Add account,Sign out of all accounts) and on a group heading. Rather than give it a name of its own, it folds into the newItem.Labelasvariant='secondary'.The two parts only ever differed by how loud they read, so an axis on one part models that better than two parts do.
variantmatchesButton, and the value reflects asdata-variantlike every other Mosaic axis, so consumers can scope overrides on it.secondarydeclares no color at all. The reset setscolor: inheritand StyleX keeps only the last atom that declares a property, soprimarydisplaces it and holds its own color whilesecondaryleaves it standing. That is what puts the secondary label on the row's color and carries it through the row's hover promotion, which a fixed color would freeze. It is also whylabel.basecannot hold a color.Slot classes change:
cl-item-titleis nowcl-item-label, and the oldcl-item-labelis nowcl-item-label[data-variant='secondary']. Mosaic is not exported from a public@clerk/uientry, so no consumer API moves.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change