feat(ui): add UserButton controller - #9185
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 80a1968 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to This PR adds the UserButton controller and connected container without any identified merge-blocking risk; it is merge-ready after normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
8786841 to
ba22f92
Compare
ba22f92 to
c9bdf23
Compare
alexcarpenter
changed the title
c9bdf23 to
93aa6f2
Compare
93aa6f2 to
4643795
Compare
4643795 to
4467f3d
Compare
4467f3d to
fbd5c2f
Compare
Every other action — switching account, signing out of one, joining a suggested or invited workspace — now resolves back into an open popover so the result is visible where it happened. The swingset prototypes fake the round trip they make against Clerk, so the spinner and stood-down rows are demonstrable without a running app.
The trigger carried the avatar alone. It now names what is active beside it — the organization and its plan wherever one heads the trigger, the account otherwise — behind `showLabel`, which defaults on. Badge's `neutral` color was unreadable in both schemes: its fill is a 900 and its text token is a text color, not an on-fill one. It now rides the same black/white scrim the button's neutral fill does.
`showLabel` becomes `renderTriggerLabel`, and the plan badge gets its own `renderPlanBadge`. The badge is part of the label, so it needs both.
…ccount The trigger and the popup's header now always name the same workspace. `combined` carries both switchers, so `modePriority` picks which one it leads with: the active organization by default, the account with `modePriority="user"`. Both are still listed either way.
…g state `useOrganization()` resolves before the organization list does, so the controller describes the active organization from the resource itself rather than leaving the view to find it in a list that has not arrived. `organizationsLoading` covers that window, and revoked or expired invitations are dropped since accepting is all an invitation row offers. Accepting an invitation joins the organization, so it now revalidates the membership list alongside the invitation one.
…ists load The membership count is 0 until the first page lands, so the surface had no way to tell an account with no organizations from one whose list had yet to arrive, and opened a workspace section under both. The user resource carries its own memberships, so the question is settled before any request goes out; the fetched count still counts, in case the resource is behind the server.
Managing an account and managing an organization both navigated to Clerk's built-in profile URLs. Both now open the corresponding modal instead, which is what `<UserButton>` and `<OrganizationSwitcher>` each already do, so returning from one puts you back where you were rather than on another page. Apps that would rather route take `userProfileUrl` and `organizationProfileUrl`, in the same url-plus-mode shape as the existing components: a URL is the whole opt-in to navigation, and `modal` forbids one, so the pair cannot contradict itself. The two profiles resolve apart, so routing one leaves the other a modal. Inviting members follows wherever managing the organization goes. It is the other way into administering the same organization, so splitting them would send one to the app's own page and the other to Clerk's.
The profile modals opened into `document.body`, so an app that mounts the button inside its own dialog or popover got the modal rendered behind it. Both now open into the portal root from `usePortalRoot`, matching what the pre-Mosaic `<UserButton>` and `<OrganizationSwitcher>` pass as `getContainer`.
Switching into an organization was a one-way door: nothing on the surface cleared the active one. The controller now offers the account's own workspace as a selectable row, and withholds it where there are no organizations to leave.
Selecting the personal workspace had nowhere to land, since a redirect resolved against an organization has nothing to resolve. `afterSelectPersonalUrl` gives it its own, taking the same path template or builder function as `afterSelectOrganizationUrl` and resolving against the user.
…s required
An instance with force organization selection enabled has no personal workspace to
return to: clerk-js refuses `setActive({ organization: null })` outright there, so the
row stood in the list and did nothing when clicked. It is now withheld, the way the
existing OrganizationSwitcher hides personal under the same setting.
`hidePersonal` leaves the account's own workspace out of the list, for an app whose organizations are the whole product. It joins the instance setting rather than overriding it: where organization selection is forced there is no personal workspace either way, and passing `false` cannot opt back into one.
The row was offered unconditionally, so a user whose instance restricts organization creation, or who is at their creation limit, landed on a page that turns them away. Legacy gates the same action on `user.createOrganizationEnabled`.
Create organization always navigated to Clerk's hosted page. It now resolves the same way the two profile surfaces do: a modal by default, or navigation when `createOrganizationUrl` routes it. The shared helper is renamed `openOrNavigate`, since it no longer serves only profiles.
Invite has no page of its own, so it opens Clerk.openInviteMembers() rather than following organizationProfileUrl into the app's own page.
A custom action is the app's to run, and whatever it opens takes over from here, so pressing one closes the popover. A link navigates away on its own and is left alone.
Replaces the internals write-up with what a consumer needs: what the button is, how its menu behaves, and worked examples of routing and custom menu items. Takes props as one argument so the signature reads as the component rather than its destructuring.
Loading and signed out are the same thing from here — `useUser()` reports neither until clerk-js is up, and SSR does not settle it either, since `InitialState` carries an absent user as `undefined` rather than `null`. So the button waits, the way the existing UserButton and OrganizationSwitcher already do.
The controller was reading singleSessionMode, forceOrganizationSelection and afterSwitchSessionUrl off an environment that may not have hydrated, silently defaulting all three. It now stays in loading until the environment is there, and reports organizationsEnabled instead of leaving the mode gate to guess.
The controller hands the view `displayConfig.branded`, so an instance that has paid the branding off carries no "Secured by Clerk" at the foot of the popup.
Ephem
left a comment
Ephem
left a comment
There was a problem hiding this comment.
Feels great overall and this one was easy to follow. Left a few comments and will review the machine PR next to get a feel for the entire thing.
| // Nothing stands in for the button until Clerk answers: while it is loading, a signed-out visitor | ||
| // is indistinguishable from a session still resolving, so anything rendered here is a button | ||
| // promised to people who are never going to get one. `<ClerkLoading>` is where an app that knows | ||
| // its own nav puts a placeholder. | ||
| if (controller.status !== 'ready') { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Would we expose <Avatar.Fallback> so it's easy to get a matching loading state immediately if you want that? Or put as a more general question, what is the story for people who do want a loading state immediately?
Note that <ClerkLoading> is already bundled, and since this is too, no matter where you put that loading state, I think the loading state works for SSR too out of the box! 🤯
There was a problem hiding this comment.
Likely want to expose the fallback prop like we currently do on components?
There was a problem hiding this comment.
Added here 809fb4242e
Matches the fallback prop the existing components carry, so it stands in while Clerk is still answering and is dropped once nobody is signed in (signed out is an answer, not a wait). Rendering a matching placeholder is left to the app.
Exposing Avatar.Fallback / Skeleton so the placeholder lines up without hand-sizing is a separate export-surface question, so I left it for #9338.
| export type UserButtonProps = UserButtonControllerOptions & | ||
| UserButtonTriggerProps & | ||
| UserButtonMenuProps & | ||
| Pick<UserButtonModeProps, 'modePriority'>; |
There was a problem hiding this comment.
Something that popped to mind when pondering the public API is the previous discussions about providing instrumentation callbacks to let users log their own events etc when actions are taken within the button.
Not saying that's something we need to include in a first version, just thought I'd mention it.
There was a problem hiding this comment.
yeah, thats a neat idea!
| // Mirrors the `<OrganizationSwitcher>` `afterSelectOrganizationUrl` prop: a full URL/path, a `:token` | ||
| // path template resolved against the organization, or a builder function. | ||
| type AfterSelectUrl<T> = ((entity: T) => string) | string; |
There was a problem hiding this comment.
It's been a source of confusion to me in the past that some url props support builder functions, and some do not. I now get that the AfterSelect ones needs it since the data you want to build from is not present before the click, but that form could be useful for userProfileUrl and organizationProfileUrl too for pure DX gains (not having to grab the current user/org first), so maybe this is an opportunity to streamline since you've built such a nice abstraction around it?
An total aside of course, not sure the consistency gains are worth it, feel free to ignore, just thought I'd mention it. 😄
| // Organizations, single-session and forced selection all come off the environment, and it | ||
| // hydrates on its own schedule. Waiting for it beats guessing at three answers and rearranging. |
There was a problem hiding this comment.
I'm guessing this is an LLM. I find so many of the comments in these PRs 1. Unnecessary and 2. Very hard to read. I'm honestly having a harder time parsing some of the comments than I have reading the code. 😬
If we even need a comment:
| // Organizations, single-session and forced selection all come off the environment, and it | |
| // hydrates on its own schedule. Waiting for it beats guessing at three answers and rearranging. | |
| // These effect layout, so we wait for all to avoid UI shifts |
| const { isLoaded: isUserLoaded, user } = useUser(); | ||
| const { isLoaded: isSessionLoaded, session } = useSession(); | ||
| const { isLoaded: isOrgLoaded, organization } = useOrganization(); | ||
| const { userMemberships, userInvitations, userSuggestions, ref } = useOrganizationListInView(); |
There was a problem hiding this comment.
@alexcarpenter I think this coderabbit feedback is correct, also for . Currently, we'll make these calls for all instances, including the ones that don't have orgs enabled, which seems wasteful. I think calling these will also trigger the prompt to enable organizations in dev?useOrganization
The downside is, we'll now have a request waterfall where we first have to wait to check if orgs are enabled which is not great. I also don't think we have a good way to enable/disable these conditionally right now.
This looks like a key piece to figure out, let's chat!
Update: I realized useOrganization without extra args does not result in an extra API call, it reads from clerk, so that one should be fine if we can fix the enable orgs prompt.
| onSwitchSession: sessionId => | ||
| clerk.setActive({ session: sessionId, redirectUrl: displayConfig.afterSwitchSessionUrl }), |
There was a problem hiding this comment.
I think this needs to handle session tasks as well? Current implementation lives in handleSessionClicked in useMultisessionActions.
There was a problem hiding this comment.
Resolved here 80a1968c09
Two differences from handleSessionClicked: it calls decorateUrl (the old redirectUrl path decorated for us, and clerk-js warns in dev when the callback does not), and it skips the virtual-router branch, since Mosaic never mounts inside one.
| // An app can mount the button inside its own dialog or popover; the modal has to portal into that | ||
| // same root or it renders behind the surface that opened it. | ||
| const getContainer = usePortalRoot(); | ||
| const environment = useMosaicEnvironment(); |
There was a problem hiding this comment.
Not related to this PR, but this not being reactive feels a bit iffy long term. The hook has a bunch of good comments with constraints that are not obvious in this location where you are actually consuming it.
This is more a general problem with the clerk-js->"host app" bridge though and not worth tackling right now.
| if (pendingKey) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This makes the UI feel frozen when an action is in progress, especially on throttled mobile where there's no cursor to indicate things. I saw this changed in the next PR though so will re-review there, just leaving this comment to not forget. 😄
setActive received redirectUrl alone, which routes past a session task and strands the account. It now takes a navigate callback that builds the task URL against the sign-in URL, and decorates the after-switch URL so the Safari ITP refresh survives.
2 participants
Description
Stacked on #9184. Connects the
UserButtonview to live Clerk data.useUserButtonController()returns a'loading' | 'hidden' | 'ready'union. When it isreadyit carries the view's data contract and the callback behind every row.user-button.tsxis the connected container that owns the popover.Where the data comes from:
activeSessionfromuseUser()anduseSession(). The name follows Clerk's own order: first and last name, then username, then the identifier.activeOrganizationfromuseOrganization(), wherenullis the personal workspace.memberships,suggestions, andinvitationsfromuseOrganizationList(), paged as the list scrolls.hasOrganizationsfrom the user resource, so it can answer before those lists load.additionalSessionsfrom the client, without the active one.org:sys_memberships:manage.What the rows do:
setActive. Signing out callssignOut. Invitations and suggestions accept in place and revalidate the list.userProfileUrl,organizationProfileUrl,createOrganizationUrl.afterSelectOrganizationUrlandafterSelectPersonalUrlsay where picking a workspace lands.hidePersonalwithholds it by request.customMenuItemsreach the menu through the container, and a custom action closes the popover behind whatever it opens.The button renders nothing until Clerk answers. While it loads, a signed-out visitor and a session still resolving are indistinguishable, so anything rendered then is a button promised to people who are never going to get one.
<ClerkLoading>is where an app that knows its own nav puts a placeholder.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change