Uh oh!
There was an error while loading. Please reload this page.
refactor(ui): Improve UserButton/OrgSwitcher accessibility - #8325
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 2dabbf1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
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 |
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:
📝 WalkthroughWalkthroughThis pull request updates the UserButton component's accessibility architecture by replacing the menu-based pattern (with Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/components/UserButton/__tests__/UserButton.test.tsx`:
- Around line 243-266: The current test "traps focus within the popover when
open" only asserts the trigger loses focus on Tab, which can pass even if focus
moves outside the dialog; update the loop to assert that the dialog element
(retrieved as dialog = screen.getByRole('dialog', { name: 'Account panel' }))
contains the active element after each await userEvent.tab() call instead of
checking expect(trigger).not.toHaveFocus(); i.e., replace the per-iteration
assertion with a containment check like dialog.contains(document.activeElement)
(using the existing dialog variable and userEvent.tab()) so each Tab verifies
focus remains inside the popover.
🪄 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
Run ID: 6d4f3cc8-0b08-4506-8719-c6ce29033b7f
📒 Files selected for processing (10)
.changeset/user-button-a11y.mdpackages/localizations/src/en-US.tspackages/shared/src/types/localization.tspackages/ui/src/components/UserButton/SessionActions.tsxpackages/ui/src/components/UserButton/UserButtonPopover.tsxpackages/ui/src/components/UserButton/UserButtonTrigger.tsxpackages/ui/src/components/UserButton/__tests__/UserButton.test.tsxpackages/ui/src/components/UserButton/index.tsxpackages/ui/src/elements/Popover.tsxpackages/ui/src/hooks/usePopover.ts
Uh oh!
There was an error while loading. Please reload this page.
@clerk/astro@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/electron@clerk/electron-passkeys@clerk/eslint-plugin@clerk/expo@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/vuecommit: |
ae1e533 to
204f597CompareThe {{name}} - Open/Close account panel pattern and the LocalizationValue<'name'>
type narrowing will ship in a separate PR so the localization rollout can be
scoped and reviewed independently. This reverts the action key values and type
changes while keeping the three new optional label keys added in this PR.Adds label__userButtonPopover, label__accountActions, and label__activeSessions to all 48 non-English locale files so screen reader labels for the new dialog structure are available in every supported language.
OrganizationSwitcher was missing dialog semantics and escape-to-close tests that UserButton already had. Menu was missing a test verifying Tab exits the menu (validating modal=false behaviour). These guard against regressions in focus management across all Popover consumers.
The identity-first trigger label was reverted; E2E tests still expected the old 'Open account panel' text.
Summary
Replaces
role="menu"/role="menuitem"withrole="dialog"androle="group"in the UserButton and OrganizationProfile popovers. The popover contains mixed content (profile preview, action buttons, session cards, footer) — not a flat action list — somenusemantics were misleading screen readers.What changed
role="dialog"with labeledrole="group"sections ("Account actions", "Active sessions")useInteractions(useClick,useRole,useDismiss) instead of manualonClick/aria-*wiring. Focus moves to the dialog container on open, Tab is trapped inside, Escape restores focus to the triggerrole="menuitem"fromAction/ExtraSmallActioncomponents — no caller uses menu semanticsFixes USER-4842
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change