feat(ui): add UserButton view component - #9184
Conversation
🦋 Changeset detectedLatest commit: f8aec65 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.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a client-side Mosaic Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
@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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/lib/registry.ts`:
- Around line 3-8: Update the manually constructed AccountButton story module to
preserve the story’s raw __source export. Import __source from
account-button.stories and include it in the module object alongside
AccountButtonDefault, AccountButtonMultipleAccounts, AccountButtonPersonal, and
accountButtonMeta; apply the same change to the corresponding second
construction.
In `@packages/swingset/src/stories/account-button.mdx`:
- Around line 3-21: Restructure the AccountButton MDX documentation into
Archetype C format: replace the current descriptive sections with one short
paragraph covering owned state and the wired lower-layer pieces, then provide
exactly one Story. Add a composition array to that Story listing each direct
dependency, including the headless popover primitive and slot recipes, and
remove the current multi-story layout.
In `@packages/swingset/src/stories/account-button.stories.tsx`:
- Around line 10-14: Update the AccountButton story metadata to assign
accountButtonRecipe to meta.styles. In each story function, cast the knobs
Record<string, unknown> through knobsAsProps and spread the resulting props into
AccountButton so playground values are applied.
In `@packages/ui/src/mosaic/components/account-button.view.tsx`:
- Around line 446-808: Split the presentational leaves and sections from
account-button.view.tsx into focused React modules, extracting symbols such as
Avatar, Row, Header, WorkspaceList, AccountsSection, and Footer while preserving
their current behavior and context usage. Keep the public compound component
composition and exports in account-button.view.tsx, and update imports as needed
so the file remains focused and within the project’s preferred component size.
- Around line 21-85: Add comprehensive customer-facing JSDoc to the exported
AccountButtonAccount, AccountButtonMembership, AccountButtonSuggestion,
AccountButtonInvitation, AccountButtonData, AccountButtonCallbacks, and props
contracts, documenting field semantics including onUpgrade. Add explicit
React.JSX.Element return types to all four exported AccountButton components,
then request Docs-team review for the generated public API documentation.
- Around line 57-68: Remove the inert status field from AccountButtonData for
v1, since the current loading value still requires and renders ready-state data.
Update AccountButton consumers, including the account-button view, to rely on
the existing ready-state shape without status checks; only retain status if you
introduce a genuine discriminated union with a separate loading rendering path.
- Around line 488-535: Update the selection control rendered by Row to expose
its active state through the appropriate aria-current attribute, using the
active prop’s value. Apply the attribute to the interactive button and the
non-interactive div consistently, while leaving the visual check icon behavior
unchanged.
🪄 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 Plus
Run ID: 05507a88-c836-4e0f-b43f-e0d914595582
📒 Files selected for processing (8)
.changeset/account-button-switcher.mdpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/account-button.mdxpackages/swingset/src/stories/account-button.stories.tsxpackages/ui/src/mosaic/components/account-button.view.tsxpackages/ui/src/mosaic/icons/registry.tsxpackages/ui/src/mosaic/primitives/popover.tsx
alexcarpenter
changed the title
dfae67d to
ef35f2d
Compare
alexcarpenter
changed the title
ef35f2d to
8f7ffa2
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/ui/src/mosaic/user-button/user-button.view.tsx (2)
477-538: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpose the active row to assistive technology.
The check icon at Lines 527-533 is purely visual. Add
aria-currentto the interactivebutton(Line 512) and the non-interactivediv(Line 520) inRow, driven by theactiveprop, so screen-reader users can identify the active account or organization.This mirrors the same unresolved concern raised previously on the sibling
account-button.view.tsxfile.As per coding guidelines, "Implement proper ARIA attributes for accessibility in React components."
♿ Proposed fix
{onSelect ? ( <button type='button' + aria-current={active ? 'true' : undefined} onClick={onSelect} {...select} > {inner} </button> ) : ( <div {...select} + aria-current={active ? 'true' : undefined} style={{ cursor: 'default' }} > {inner} </div> )}🤖 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 477 - 538, Update the interactive and non-interactive row elements in Row to expose active state through aria-current, deriving the attribute from the active prop. Apply the same value to both the button and fallback div while preserving their existing behavior and rendering.
447-813: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit this file into focused modules.
This file still holds the data contracts, recipe, context, presentational leaves (
Avatar,Row,SuggestedBadge,InlineButton,HoverAction,AddRow), sections (Header,WorkspaceList,SessionsSection,Footer), and the public compound parts in one 912-line module. Extract the leaves and sections into separate files and keep the compound composition (UserButtonRoot,UserButtonTrigger,UserButtonPopup,UserButton) here.This mirrors the same unresolved concern raised previously on the sibling
account-button.view.tsxfile.As per coding guidelines, "Limit component size to 150-200 lines; extract logic into custom hooks."
🤖 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 447 - 813, Split the user-button view into focused modules: move the presentational leaves Avatar, Row, SuggestedBadge, InlineButton, HoverAction, and AddRow, plus Header, WorkspaceList, SessionsSection, and Footer, into separate files with their required shared contracts and imports. Keep UserButtonRoot, UserButtonTrigger, UserButtonPopup, and UserButton in the current module, preserving their existing composition and behavior while reducing the file to focused component logic.
🤖 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/lib/registry.ts`:
- Around line 108-113: Update the UserButton story imports to include the
exported __source value, then assign it to the __source property when
constructing userButtonModule alongside the existing story metadata. Preserve
the current module construction behavior for all other fields.
In `@packages/swingset/src/stories/user-button.mdx`:
- Around line 5-14: Merge the interaction and implementation descriptions in the
user-button intro into one short, present-tense paragraph, preserving the key
behavior and composition details while removing redundant wording.
- Around line 68-90: Replace the nonstandard “States & scenarios” heading in the
user-button documentation with the established “Examples” section name, keeping
the Personal and MultipleSessions stories and their descriptions under that
section.
---
Duplicate comments:
In `@packages/ui/src/mosaic/user-button/user-button.view.tsx`:
- Around line 477-538: Update the interactive and non-interactive row elements
in Row to expose active state through aria-current, deriving the attribute from
the active prop. Apply the same value to both the button and fallback div while
preserving their existing behavior and rendering.
- Around line 447-813: Split the user-button view into focused modules: move the
presentational leaves Avatar, Row, SuggestedBadge, InlineButton, HoverAction,
and AddRow, plus Header, WorkspaceList, SessionsSection, and Footer, into
separate files with their required shared contracts and imports. Keep
UserButtonRoot, UserButtonTrigger, UserButtonPopup, and UserButton in the
current module, preserving their existing composition and behavior while
reducing the file to focused component logic.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 6df10ab6-9322-4890-89da-e4419eb949e3
📒 Files selected for processing (8)
.changeset/user-button-switcher.mdpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/user-button.mdxpackages/swingset/src/stories/user-button.stories.tsxpackages/ui/src/mosaic/icons/registry.tsxpackages/ui/src/mosaic/primitives/popover.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)
🚧 Files skipped from review as they are similar to previous changes (2)
- .changeset/user-button-switcher.md
- packages/ui/src/mosaic/primitives/popover.tsx
An accept or join reported itself by dropping the button and putting a spinner on the row's trailing edge, so the affordance you pressed vanished under you. It is a labelled button, so it takes SubmitButton and keeps the spinner inside itself.
Custom actions and links join the foot of the popup ahead of Clerk's own rows. menuItemOrder arranges the foot by id, over createOrganization, addAccount, signOutAll, and each custom item's own id. Which of the built-ins the foot carries depends on the surface, so an id it has not got is ignored rather than an error.
The data contract, mode flags, and menu item shapes are what the view and a controller agree on, so they sit in a module neither one owns. Annotates the exported parts' return types: without one TS infers the global JSX.Element, which Emotion augments, so the signature published to apps read EmotionJSX.Element.
Standing in for the trigger means claiming a button is coming, and until Clerk loads there is no telling whether one is: a signed-out visitor is indistinguishable from a session still resolving, so the placeholder resolved to nothing for them. `<ClerkLoading>` is where an app that knows its own nav puts one.
`mode` took `'orgs'` while `modePriority` took `'organizations'`. Both are now `'organization'`, matching `ForPayerType` and the `for` prop on the billing components. Also corrects a stale comment in the view suite, which claimed the connected container does not expose `mode`.
The badge sits beside the trigger label and is governed by it, so it takes the same name. It stays a boolean: the plan comes off the active organization.
Which slot each affordance lands in was re-derived by every section that renders one. It is settled once now, in a per-mode table, so what a mode renders reads in one place. AccountRow was WorkspaceRow copied; it delegates to it instead.
Replaces arrangeMenuRows with a generic applyOrder helper in user-button.utils, and switches the mosaic Avatar to useSafeLayoutEffect so it stops warning during SSR.
The existing cached-image test renders through Testing Library, which wraps in act and drains passive effects before returning, so it passes just as well with a useEffect. Drive the root directly instead: flushSync stops short of passive effects, so the image has to be committed by then.
Item used pointer-events: none, where Button, Menu and Tabs use cursor: not-allowed. Keep the pointer on the row so it reports not-allowed, and exclude :disabled from each hover branch, since :hover still matches a disabled button. Opacity stays 1: a row standing down while a sibling action runs holds its look rather than dimming.
… by flag The layout table was keyed by mode, with a boolean per affordance whose name carried the place: signOutInHeader, addAccountInFooter. Transposed it: one row per affordance, one slot per mode, so what the three modes do with any one of them reads on a line. A mode may only name a section it opens, which is now checked at compile time rather than by keeping two tables in step by hand. The account row's Create organization and Sign out were placed inline with no flag at all; both read the table now. Closes four cells the tests never asserted: Invite in combined mode, Create organization withheld in user mode, Sign out of all accounts at the foot in user mode, and Sign out withheld entirely in organization mode.
The header gated Invite on whether the surface could list organizations, which is a different question from whether one is active. In the window where the active organization is known but its membership list has not landed, the header named the organization and offered the gear beside it, but withheld Invite. Both act on the same organization, so both now answer to it.
Honors a `branded` prop so an instance that has paid the branding off carries none, matching how every other Clerk surface reads `displayConfig.branded`. Withholds "Sign out of all accounts" when there is no second account, since the account's own row already signs out of it. Names the account row's menu after the account, and reports the workspace list's wait to a screen reader.
Rows stood down through the native disabled attribute, so the row that owned the in-flight action dropped out of the tab order the moment it was pressed, taking focus with it just as its spinner was announced. They now stand down through aria-disabled and drop the press themselves, the way SubmitButton already does. The Item row styles gated hover, the pressed fill and the cursor on :disabled, which no longer matches, so each selector accepts both spellings.
Ephem
left a comment
Ephem
left a comment
There was a problem hiding this comment.
Really nice work here, this is really great!
I did stumble when trying to understand the layout and how different sections render etc. Maybe I'm just unfamiliar and some parts takes some getting used to, and I don't have fully formed opinions yet, but I left a few comments as discussion points below.
There was a problem hiding this comment.
This file was hard to read, it's hard to get a sense for what the layout looks like or how it works. That reflects elsewhere as well, mainly parts of the .view. file.
I think part of it is some of the naming, part of it is some of the structure, I'll leave a few separate comments below so we can keep discussions separate.
This is subjective of course so I wanted to leave these comments first to discuss, but I'll also work on a more concrete suggestion we can look at together.
| import type { UserButtonData, UserButtonMode, UserButtonModePriority } from './user-button.types'; | ||
|
|
||
| /** Where an affordance lands. `none` is a mode not offering it at all. */ | ||
| type Slot = 'account-row' | 'accounts-heading' | 'footer' | 'header' | 'none'; |
There was a problem hiding this comment.
Reading this at first confused me since I thought this was a complete list of slots/sections, but they don't list all the slots, only the ones that are possible targets for affordances (shoutout to "The Design of Everyday Things").
Affordance feels weird here though, "action" is used in the .view. file and feels clearer, so maybe ActionSlots or something would make it clear what this actually is?
| interface Sections { | ||
| /** The workspace list carries the active account's row, and the account-wide actions with it. */ | ||
| accountRow: boolean; | ||
| /** There is a workspace list at all, and something to put in it. */ | ||
| listsOrganizations: boolean; | ||
| /** There is an Accounts group, and another account to switch to. */ | ||
| listsAccounts: boolean; | ||
| /** The Accounts group is headed, so it tells its rows apart and carries "Add account". */ | ||
| accountsHeading: boolean; | ||
| } |
There was a problem hiding this comment.
Ignoring the account naming and accountRow confusion mentioned in other comments, the naming here also feels disjoint given they are all booleans. Some read as nouns, some as verbs/adjective-like.
If it's sections, listsOrganizations and listsAccounts feels off. What I'd argue is NOT treating them as sections though, given that the headers don't feel like "sections" and that these leak out into the .view. code without preserving the section context. My suggestion is to instead name them as booleans, like showOrganizationsSection.
| /** Which sections a mode opens. Whether the data fills them is settled in `resolveUserButtonLayout`. */ | ||
| interface Sections { | ||
| /** The workspace list carries the active account's row, and the account-wide actions with it. */ | ||
| accountRow: boolean; |
There was a problem hiding this comment.
I find accounts-row and accounts-heading to be confusing, given that accounts-row is actually organization-heading structurally and visually?
Semantically, it's the activeAccount, or as I'd argue, activeSession, see next point.
I find the accounts naming confusing. It's a great word to use in user facing text, but we already have users and sessions in code, introducing a third accounts feels hard to reason about (I know we've used it in a limited fashion in other places before, but not as much as it's embraced here).
| /** There is a workspace list at all, and something to put in it. */ | ||
| listsOrganizations: boolean; |
There was a problem hiding this comment.
Workspace is something we introduced in the dashboard to disambiguate from organizations. It might be a good word to use in translations/user facing text, but I find it confusing to introduce in the code when we already use organizations everywhere else?
The .view. is what uses it in code but I thought I'd comment here to keep naming discussions in one place.
| /** | ||
| * The slots a mode can name: the two every mode has, plus whichever sections it opens above. A mode | ||
| * cannot put an affordance in a section it does not render, so the table below cannot drift out of | ||
| * step with `sections` without failing to compile. | ||
| */ | ||
| type ModeSlot<M extends UserButtonMode> = |
There was a problem hiding this comment.
Type safety is nice, but it makes this file pretty hard to read compared to something like:
type Action = 'inviteMembers' | 'signOut' | 'createOrganization' | 'addAccount' | 'signOutAll';
interface ModeLayout {
sections: Sections;
placements: Record<Action, Slot>;
}
const modeLayouts = {
combined: {
sections: { accountRow: true, listsOrganizations: true, listsAccounts: true, accountsHeading: true },
placements: {
inviteMembers: 'header',
signOut: 'account-row',
createOrganization: 'account-row',
addAccount: 'accounts-heading',
signOutAll: 'footer',
},
},
organization: {
sections: { accountRow: false, listsOrganizations: true, listsAccounts: false, accountsHeading: false },
placements: {
inviteMembers: 'header',
signOut: 'none',
createOrganization: 'footer',
addAccount: 'none',
signOutAll: 'none',
},
},
user: {
sections: { accountRow: false, listsOrganizations: false, listsAccounts: true, accountsHeading: false },
placements: {
inviteMembers: 'none',
signOut: 'header',
createOrganization: 'none',
addAccount: 'footer',
signOutAll: 'footer',
},
},
} as const satisfies Record<UserButtonMode, ModeLayout>;Not sure where I stand on that tradeoff but wanted to note that it comes at a cost.
| return organization | ||
| ? { name: organization.name, imageUrl: organization.imageUrl, shape: 'square', organization } | ||
| : { name: activeSession.name, imageUrl: activeSession.imageUrl, shape: 'circle' }; |
There was a problem hiding this comment.
@alexcarpenter Say mode is organization, organizations are optional and the user has none, and hidePersonal is true.
The existing organizationSwitcher would say "No organization selected" in that case, but this falls back to the user, which might feel off in organization mode?
3 participants
Description
The Mosaic
UserButtonsurface, presentation only. The controller that connects it to Clerk is a follow-up.user-button.view.tsxexports the parts (UserButtonRoot,UserButtonTrigger,UserButtonPopup) and the all-in-oneUserButtonView, which frees the bareUserButtonname for the connected container.user-button.types.tsholds the data contract that container has to satisfy, anduserButtonBusyKeysnames every affordance that can spin.What the surface offers:
modenarrows the surface to organizations or to accounts.modePrioritypicks which one a combined surface leads with, in its header and in the trigger.renderTriggerLabelnames the active workspace beside the avatar,renderPlanBadgeshows its plan.customMenuItemsandmenuItemOrderadd the app's own rows to the foot of the menu, each row either an action or a link.pendingKeyspins the affordance that owns the action in flight and stands the others down.@clerk/headlessgains what the surface needs from its popover: contents stay mounted while it closes,Popover.Roottakes analignOffset, and Escape inside a menu no longer bubbles to the floating element around it.Swingset documents the surface and renders it as a live prototype.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change