Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changeset/mosaic-user-button-pending-announcement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
Comment on lines +1 to +2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

package_json="$(fd --type f --max-depth 1 '^package\.json$' packages/ui | head -n1)"
test -n "$package_json"
jq '{name, private}' "$package_json"

Repository: clerk/javascript

Length of output: 199


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- packages/ui/package.json ---'
cat packages/ui/package.json

printf '%s\n' '--- changeset configuration ---'
fd --type f --max-depth 2 '^(config\.json|package\.json)$' .changeset packages | sort | head -40
if test -f .changeset/config.json; then
  cat .changeset/config.json
fi

printf '%s\n' '--- workspace package metadata ---'
rg -n '"`@clerk/ui`"|packages/ui|private|publishConfig' package.json pnpm-workspace.yaml turbo.json .changeset packages --glob 'package.json' --glob 'pnpm-workspace.yaml' --glob 'turbo.json' --glob '!**/node_modules/**' | head -160

Repository: clerk/javascript

Length of output: 9775


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- target changeset ---'
cat .changeset/mosaic-user-button-pending-announcement.md

printf '%s\n' '--- UserButtonBusyState definitions and references ---'
rg -n -C 3 'UserButtonBusyState' packages

printf '%s\n' '--- current change summary ---'
git diff --stat
git diff --unified=3 -- packages/ui packages/react packages/clerk-js .changeset/mosaic-user-button-pending-announcement.md | head -240

Repository: clerk/javascript

Length of output: 2744


Add a Changeset entry for @clerk/ui

@clerk/ui is publicly published. Add the appropriate version bump and release note for the exported UserButtonBusyState change.

🤖 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/mosaic-user-button-pending-announcement.md around lines 1 - 2,
Add a Changeset entry for the publicly published `@clerk/ui` package, specifying
the appropriate version bump and a release note describing the exported
UserButtonBusyState change.

Sources: Coding guidelines, Learnings

Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,9 @@ describe('UserButtonView, the workspace list', () => {
it('says so to a screen reader, since nothing else reports the rows landing', () => {
renderList({ organizationsLoading: true });

expect(screen.getByRole('status')).toHaveTextContent('Loading organizations…');
// The popup's own region speaks for the action a row is running, so the placeholder is asked
// for by its text: the two report different waits and are both on the page here.
expect(screen.getByText('Loading organizations…').closest('[role="status"]')).toBeInTheDocument();
});

it('leaves the account row above it alone, since it does not wait on the list', () => {
Expand Down Expand Up @@ -726,6 +728,48 @@ describe('UserButtonView, one action at a time', () => {
expect(onSwitchSession).not.toHaveBeenCalled();
expect(row).toHaveFocus();
});

// The row keeps focus while it stands down, so nothing about the surface reports the wait on its
// own: the spinner is a picture, and `aria-disabled` says a row cannot be used rather than why.
// Named from the key, so the surface says what is running rather than that something is.
it.each([
[userButtonBusyKeys.selectOrganization('org_2'), 'Switching to Other Co', {}],
[userButtonBusyKeys.selectOrganization(null), 'Switching to Personal account', {}],
[userButtonBusyKeys.switchSession('sess_2'), 'Switching to bob@example.com', {}],
[userButtonBusyKeys.signOutSession('sess_2'), 'Signing out of bob@example.com', {}],
[userButtonBusyKeys.signOutAll(), 'Signing out of all accounts', {}],
[userButtonBusyKeys.acceptInvitation('inv_1'), 'Joining Gamma', { invitations: [gamma] }],
[userButtonBusyKeys.acceptSuggestion('sug_1'), 'Requesting to join Beta', { suggestions: [beta] }],
])('names the wait in a live region: %s', (pendingKey, announcement, props) => {
render(surface(pendingKey, props));

expect(screen.getByRole('status')).toHaveTextContent(announcement);
});

// A region that mounts with its message already in it is not announced, so the popup carries an
// empty one from the moment it opens and the message lands in a region that is already there.
it('carries the region while idle', () => {
render(surface(null));

expect(screen.getByRole('status')).toBeEmptyDOMElement();
});

// Picking a workspace closes the popup behind it, so a region living in the popup would be taken
// off the page while it was still being read out. It belongs to the surface, which stays.
it('carries the region whether or not the popup is open', () => {
render(surface(userButtonBusyKeys.selectOrganization('org_2'), { defaultOpen: false }));

expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
expect(screen.getByRole('status')).toHaveTextContent('Switching to Other Co');
});

// A key can outlive what it names: an account signs out from its own row, and the row is gone
// before the action lands. The region says nothing rather than announcing a half-filled template.
it('says nothing for an action it cannot name', () => {
render(surface(userButtonBusyKeys.switchSession('sess_9')));

expect(screen.getByRole('status')).toBeEmptyDOMElement();
});
});

describe('UserButtonTrigger', () => {
Expand Down
8 changes: 8 additions & 0 deletions packages/ui/src/mosaic/user-button/user-button.messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ export const userButtonBase = {
popup: {
label: 'Account',
},
// What the popup's live region says while an action runs, by the affordance that owns it.
status: {
switching: 'Switching to {name}',
signingOut: 'Signing out of {identifier}',
signingOutAll: 'Signing out of all accounts',
joining: 'Joining {name}',
requesting: 'Requesting to join {name}',
},
workspaces: {
personal: 'Personal account',
loading: 'Loading organizations…',
Expand Down
11 changes: 11 additions & 0 deletions packages/ui/src/mosaic/user-button/user-button.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@ import * as stylex from '@stylexjs/stylex';
import { colorVars, fontWeightVars, radiusVars, space, typeScaleVars } from '../tokens.stylex';

export const styles = stylex.create({
// Takes the live region out of the layout without taking it out of the accessibility tree, which
// `display: none` and `visibility: hidden` both do.
visuallyHidden: {
overflow: 'hidden',
clipPath: 'inset(50%)',
position: 'absolute',
whiteSpace: 'nowrap',
height: '1px',
width: '1px',
},
Comment on lines +8 to +15

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably should lift to an atom in the future to reuse.


// The avatar is the trigger, so the button paints nothing of its own.
trigger: {
padding: 0,
Expand Down
69 changes: 68 additions & 1 deletion packages/ui/src/mosaic/user-button/user-button.view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,48 @@ function useBusy(key?: string): { busy: boolean; disabled: boolean } {
return { busy: pendingKey === key, disabled: pendingKey !== key };
}

/**
* What the surface says while an action runs, by the key that owns it. Keyed through
* `userButtonBusyKeys` rather than by parsing `pendingKey`, so the key grammar stays in one place,
* and built from the rows' own names so the announcement names the same thing the row does.
*/
function pendingAnnouncements(data: UserButtonContextValue): Map<string, string> {
const switching = (name: string) => fill(m.status.switching, { name });
const announcements = new Map<string, string>([
[userButtonBusyKeys.selectOrganization(null), switching(m.workspaces.personal)],
[userButtonBusyKeys.signOutAll(), m.status.signingOutAll],
]);

// The active organization is described whole rather than found in `memberships`, so it is named
// here even while the list it belongs to is still loading.
for (const membership of data.activeOrganization
? [data.activeOrganization, ...data.memberships]
: data.memberships) {
announcements.set(userButtonBusyKeys.selectOrganization(membership.organizationId), switching(membership.name));
}
// Accounts are named by identifier, the way their rows are.
for (const session of [data.activeSession, ...data.additionalSessions]) {
announcements.set(userButtonBusyKeys.switchSession(session.sessionId), switching(session.identifier));
announcements.set(
userButtonBusyKeys.signOutSession(session.sessionId),
fill(m.status.signingOut, { identifier: session.identifier }),
);
}
for (const invitation of data.invitations) {
announcements.set(
userButtonBusyKeys.acceptInvitation(invitation.id),
fill(m.status.joining, { name: invitation.organizationName }),
);
}
for (const suggestion of data.suggestions) {
announcements.set(
userButtonBusyKeys.acceptSuggestion(suggestion.id),
fill(m.status.requesting, { name: suggestion.name }),
);
}
return announcements;
}

interface ActiveWorkspace {
name: string;
imageUrl?: string;
Expand Down Expand Up @@ -937,7 +979,10 @@ export function UserButtonRoot(props: UserButtonRootProps): ReactElement {
placement={placement ?? 'bottom-start'}
sideOffset={sideOffset}
>
<UserButtonContext.Provider value={{ ...data, layout }}>{children}</UserButtonContext.Provider>
<UserButtonContext.Provider value={{ ...data, layout }}>
{children}
<ActionStatus />
</UserButtonContext.Provider>
</Popover.Root>
);
}
Expand Down Expand Up @@ -989,6 +1034,28 @@ export function UserButtonTrigger({
);
}

/**
* Speaks the one in-flight action. `pendingKey` allows a single action at a time, so the surface
* needs one region rather than one per affordance.
*
* It belongs to the surface rather than to the popup or the acting row, both of which go while the
* action is still running: picking a workspace closes the popup behind it, and a row leaves when the
* list re-sorts or an account signs out. A region taken off the page mid-announcement is not read.
*/
function ActionStatus(): ReactElement {
const data = useUserButtonContext();
// Mounted whether or not anything is running: a region that arrives with its message already in
// it is not announced, so the message has to land in a region that is already on the page.
return (
<span
role='status'
{...stylex.props(styles.visuallyHidden)}
>
{(data.pendingKey && pendingAnnouncements(data).get(data.pendingKey)) || ''}
</span>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
);
}

/** The popover surface: header, workspace list, additional accounts, and footer. */
export function UserButtonPopup(): ReactElement {
return (
Expand Down
Loading