feat(ui): announce the user button's in-flight action - #9404
feat(ui): announce the user button's in-flight action#9404alexcarpenter wants to merge 3 commits into
Conversation
The row that starts an action keeps focus while it stands down, and its spinner is decorative, so nothing reported the wait. The popup now carries a polite live region from the moment it opens, and `pendingLabel` names the running action for it to speak.
🦋 Changeset detectedLatest commit: f300e5c 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.
|
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
@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: |
|
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 (1)
📝 WalkthroughWalkthroughUserButton now generates localized announcements for pending account, organization, sign-out, invitation, and suggestion actions. Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🧹 Nitpick comments (1)
packages/ui/src/mosaic/user-button/__tests__/user-button.view.test.tsx (1)
731-738: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest a live-region update after mount.
The test mounts
pendingLabelin the first render. It only checks DOM text. It does not verify that an existingrole='status'element receives the later label. Render the idle state first, keep the status element, then rerender with the pending label.As per coding guidelines, unit tests are required for all new functionality. The PR objective requires a persistent live region to announce the in-flight action.
Proposed test update
it('reports the wait in a live region', () => { - render(surface(userButtonBusyKeys.switchSession('sess_2'), { pendingLabel: 'Switching to Bob Jones' })); + const pendingLabel = 'Switching to Bob Jones'; + const { rerender } = render(surface(null, { pendingLabel })); + const status = screen.getByRole('status'); + expect(status).toBeEmptyDOMElement(); + + rerender(surface(userButtonBusyKeys.switchSession('sess_2'), { pendingLabel })); - expect(screen.getByRole('status')).toHaveTextContent('Switching to Bob Jones'); + expect(status).toHaveTextContent(pendingLabel); });🤖 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/__tests__/user-button.view.test.tsx` around lines 731 - 738, Update the reports-the-wait-in-a-live-region test to render the idle state first, retain the existing role="status" element, then rerender with userButtonBusyKeys.switchSession('sess_2') and the pendingLabel. Assert the persistent status element receives the label after the update rather than only verifying text from the initial mount.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.
Inline comments:
In @.changeset/mosaic-user-button-pending-announcement.md:
- Around line 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.
In `@packages/ui/src/mosaic/user-button/user-button.view.tsx`:
- Around line 998-1008: Update ActionStatus to render pendingLabel only when
pendingKey is present, treating null or absent pendingKey as idle and rendering
an empty status region. Keep the mounted region and existing pendingLabel
behavior unchanged while an action is active.
---
Nitpick comments:
In `@packages/ui/src/mosaic/user-button/__tests__/user-button.view.test.tsx`:
- Around line 731-738: Update the reports-the-wait-in-a-live-region test to
render the idle state first, retain the existing role="status" element, then
rerender with userButtonBusyKeys.switchSession('sess_2') and the pendingLabel.
Assert the persistent status element receives the label after the update rather
than only verifying text from the initial mount.
🪄 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: 7011c16e-9cb7-487c-b0ef-327e72f2c851
📒 Files selected for processing (5)
.changeset/mosaic-user-button-pending-announcement.mdpackages/ui/src/mosaic/user-button/__tests__/user-button.view.test.tsxpackages/ui/src/mosaic/user-button/user-button.styles.tspackages/ui/src/mosaic/user-button/user-button.types.tspackages/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/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
| --- | ||
| --- |
There was a problem hiding this comment.
📐 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 -160Repository: 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 -240Repository: 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
The announcement was a raw `pendingLabel` prop, so the one string assistive tech actually hears was the one string the surface did not own and the localization layer could never reach. The view now derives it from `pendingKey` and the rows' own names, out of `userButtonBase.status`.
| visuallyHidden: { | ||
| overflow: 'hidden', | ||
| clipPath: 'inset(50%)', | ||
| position: 'absolute', | ||
| whiteSpace: 'nowrap', | ||
| height: '1px', | ||
| width: '1px', | ||
| }, |
There was a problem hiding this comment.
probably should lift to an atom in the future to reuse.
Picking a workspace closes the popup behind it, so a region living in the popup was taken off the page while it was still being read out. It moves to the root, which stays as long as the surface.
|
closing, gonna explore an alternate route. |
2 participants
Description
A row in the Mosaic user button that starts an action keeps focus while it stands down, and its spinner is decorative, so nothing reported the wait to assistive tech:
aria-disabledsays a row cannot be used, not why.The popup now carries a polite live region from the moment it opens, and names the running action in it ("Switching to Foundry") for as long as it runs.
userButtonBase.status, like every other string the surface renders, so it localizes with the rest.pendingKeyand the rows' own names, keyed throughuserButtonBusyKeysrather than parsed, so the key grammar stays in one place and a container announces without passing anything extra.pendingKeyalready allows a single action at a time.A key the surface cannot name (a row that has already left the list) leaves the region silent rather than announcing a half-filled template.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change