[switch] Fix focus-visible keyboard activation#5129
Conversation
commit: |
Bundle size
PerformanceTotal duration: 1,071.14 ms -4.45 ms(-0.4%) | Renders: 78 (+0)
13 tests within noise — details Check out the code infra dashboard for more information about this PR. |
✅ Deploy Preview for base-ui ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Browser: Firefox CleanShot.2026-06-26.at.14.51.00.mp4 |
lyzno1
requested review from
atomiks,
colmtuite,
flaviendelangle,
jjenzz and
michaldudak
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b896c2e1c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
lyzno1
force-pushed
the
codex/switch-focus-visible-firefox
branch
from
d6e3651 to
717274e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 717274ef19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| element.blur(); | ||
| element.focus({ | ||
| preventScroll: true, | ||
| focusVisible: true, |
There was a problem hiding this comment.
When consumers provide handlers on the rendered element, e.g. <Switch.Root render={<span onBlur={...} onFocus={...} />} />, this real blur/refocus still reaches them: useRenderElement merges render.props after the Base UI props (packages/react/src/internals/useRenderElement.tsx:146), and mergeProps runs those rightmost handlers first, so the new guard cannot prevent them. Fresh evidence beyond the existing direct-prop case is that this supported render-element path still leaks the artificial blur/focus after pointer focus + Space/Enter, which can mark fields touched or run validation even though focus stays on the switch.
Useful? React with 👍 / 👎.
|
I realized I don’t have enough context to properly fix this issue at the moment, so I’ll convert this PR to a draft for now. If maintainers think this isn’t the right direction, or if it’s not worth keeping open, please feel free to close the PR or the issue. No worries at all. |
1 participant
Fixes the Firefox case where a Switch remains focused after pointer interaction followed by Space/Enter activation, but does not match
:focus-visible.Changes:
:focus-visible.Validation:
pnpm exec prettier --check packages/react/src/switch/root/SwitchRoot.tsx packages/react/src/switch/root/SwitchRoot.test.tsxpnpm eslint packages/react/src/switch/root/SwitchRoot.tsx packages/react/src/switch/root/SwitchRoot.test.tsxpnpm test:jsdom SwitchRoot --no-watchpnpm test:chromium SwitchRoot --no-watchpnpm test:chromium SwitchRoot --no-watch -t "focus-visible"pnpm test:firefox SwitchRoot --no-watch -t "focus-visible"pnpm test:webkit SwitchRoot --no-watch -t "focus-visible"Fixes #5130