refactor(ui): Menu interaction and accessibility improvements#8333
refactor(ui): Menu interaction and accessibility improvements#8333alexcarpenter wants to merge 4 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: b643f14 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR refactors Menu to provide Floating UI interaction handlers ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 2
🤖 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/elements/Menu.tsx`:
- Around line 96-103: The prop rename from arialLabel to ariaLabel in
MenuTrigger/MenuTriggerProps is a breaking change; restore backward
compatibility by accepting both names: update MenuTriggerProps to allow
ariaLabel?: string | ((open: boolean) => string) and a deprecated arialLabel?:
string | ((open: boolean) => string), then in MenuTrigger compute the effective
label by preferring ariaLabel when present and falling back to arialLabel (e.g.,
derive normalizedAriaLabel from ariaLabel if defined, otherwise arialLabel), and
document/mark arialLabel as deprecated so consumers have a compatibility window;
ensure getReferenceProps/usage uses the normalized value.
- Around line 65-71: Add a regression test that renders the Menu component using
the same composition that wires useListNavigation and SimpleButton (i.e., Menu
with three items where the middle item has isDisabled) and assert keyboard arrow
navigation skips the disabled item: simulate focus on the first item, send
ArrowDown (and ArrowUp) key events, and verify focus moves from the first item
directly to the third item (and vice versa) rather than landing on the disabled
SimpleButton; reference the Menu component, useListNavigation, SimpleButton and
the isDisabled prop when locating code to test.
🪄 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: 9e51c4b4-043b-46d4-8292-2788f995a779
📒 Files selected for processing (6)
.changeset/menu-keyboard-a11y.mdpackages/ui/src/components/UserProfile/__tests__/ConnectedAccountsSection.test.tsxpackages/ui/src/elements/Menu.tsxpackages/ui/src/elements/Popover.tsxpackages/ui/src/elements/ThreeDotsMenu.tsxpackages/ui/src/elements/__tests__/Menu.test.tsx
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@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/vue
commit: |
Description
Improve Menu keyboard navigation and accessibility. Menus now support
Enter/Spaceto open from the trigger,ArrowDown/ArrowUp/Home/Endto move focus,Escapeto close and return focus to the trigger, and skip disabled items during arrow-key navigation. Menus no longer mark the rest of the page asaria-hiddenwhile open, so assistive technologies can still reach surrounding content.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change