fix(headless): keep pointer interactions from painting a focus ring - #9349
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: fd1bdbc 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 |
5fa30bb to
4c721c0
Compare
Safari never focuses a button it was clicked on, so a popup opened or dismissed with the mouse leaves the trigger or first control matching :focus-visible. Focus modality now decides both ends: the popup itself takes focus on a pointer open, and a pointer dismiss leaves focus where it is instead of restoring it.
4c721c0 to
fd1bdbc
Compare
@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: |
📝 WalkthroughWalkthroughAdds keyboard-aware interaction utilities and the Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@packages/headless/src/primitives/menu/menu.test.tsx`:
- Around line 403-440: Add a menu-level integration test alongside the pointer
and keyboard opening tests that opens the menu, clicks a focusable element
outside it with the pointer, waits for dismissal, and asserts
document.activeElement remains that outside target rather than returning to the
trigger. Reuse the existing Menu.Root structure and verify the returnFocusRef
behavior through the public interaction.
In `@packages/headless/src/primitives/popover/popover-context.ts`:
- Around line 25-26: Update useReturnFocus and the Popover/Select context
contracts to return a reactive false | ref value, using false when pointer-close
clears the return target so FloatingFocusManager does not resolve or focus a
fallback. Propagate this value through PopoverRoot, PopoverPositioner, and
SelectPositioner into both focus managers, preserving correct behavior for
trigger-click and outside-pointer close paths. Affected sites:
packages/headless/src/primitives/popover/popover-context.ts:25-26 (update
contract); packages/headless/src/primitives/popover/popover-root.tsx:87-88, 114,
135 (propagate value);
packages/headless/src/primitives/popover/popover-positioner.tsx:61-62 (pass
value to focus manager);
packages/headless/src/primitives/select/select-positioner.tsx:87 (update
contract/value usage).
In `@packages/headless/src/primitives/popover/popover.test.tsx`:
- Around line 261-290: Add tests in the popover dismissal suite for return focus
after an outside click and after toggling the trigger closed, asserting focus
returns to the popover trigger for pointer interactions. Keep the existing
Escape and Popover.Close coverage, and do not add Popover.Item-specific tests.
🪄 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: 6b61ac6b-3841-46df-a631-246fbd36e0ee
📒 Files selected for processing (23)
.changeset/headless-return-focus.mdpackages/headless/src/hooks/use-return-focus.test.tspackages/headless/src/hooks/use-return-focus.tspackages/headless/src/primitives/autocomplete/autocomplete.test.tsxpackages/headless/src/primitives/dialog/dialog-context.tspackages/headless/src/primitives/dialog/dialog-popup.tsxpackages/headless/src/primitives/dialog/dialog-root.tsxpackages/headless/src/primitives/drawer/drawer-popup.tsxpackages/headless/src/primitives/drawer/drawer-root.tsxpackages/headless/src/primitives/menu/menu-context.tspackages/headless/src/primitives/menu/menu-positioner.tsxpackages/headless/src/primitives/menu/menu-root.tsxpackages/headless/src/primitives/menu/menu.test.tsxpackages/headless/src/primitives/popover/popover-context.tspackages/headless/src/primitives/popover/popover-positioner.tsxpackages/headless/src/primitives/popover/popover-root.tsxpackages/headless/src/primitives/popover/popover.test.tsxpackages/headless/src/primitives/select/select-context.tspackages/headless/src/primitives/select/select-positioner.tsxpackages/headless/src/primitives/select/select-root.tsxpackages/headless/src/utils/index.tspackages/headless/src/utils/interaction-modality.test.tspackages/headless/src/utils/interaction-modality.ts
🔗 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)
Description
Preview: https://swingset-git-carp-headless-return-focus.clerkstage.dev/components/popover
Opening a Popover or Menu with the mouse in Safari painted a
:focus-visiblering on the first control inside it, and dismissing it painted one on the trigger. Safari never focuses a button it was clicked on, so the popup becomes the page's first focused element, and every programmatic focus after that matches:focus-visible.Focus modality now decides both ends:
Popover.RoottakesinitialFocus="first"to opt back into the old behavior (Autocomplete needs it).useReturnFocushook resolvesFloatingFocusManager'sreturnFocusfrom the interaction that closed the popup. Escape, Enter/Space, a Close button, and item clicks return focus to the trigger; a pointer press outside or a mouse click on the trigger leaves focus where it is. It backs Popover, Menu, Select, Dialog, and Drawer. Autocomplete is excluded: its reference is a text input, where restoring focus is correct combobox behavior.BEFORE
before.mov
AFTER
aftrer.mov
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change