fix(core): restore visible keyboard focus ring on Collapsible trigger#3722
Open
bhamodi wants to merge 1 commit into
Open
fix(core): restore visible keyboard focus ring on Collapsible trigger#3722bhamodi wants to merge 1 commit into
bhamodi wants to merge 1 commit into
Conversation
|
@bhamodi is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
5f56c9c to
b545d17
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Collapsible's trigger is a real<button>, butstyles.triggerstarts withall: unset(packages/core/src/Collapsible/Collapsible.tsx:54), which wipes the browser's default focus outline. No:focus-visiblestyle was reintroduced anywhere in the file, so keyboard users could Tab to the trigger but saw no focus indicator -- a WCAG 2.4.7 (Focus Visible) failure.This restores a keyboard-only focus ring on the trigger using the design system's established convention:
outline: 2px solid var(--color-accent)withoutlineOffset: 2px, both gated on:focus-visible. The treatment mirrors theTabMenutrigger button (packages/core/src/TabList/TabMenu.tsx:93-100), the closest structural analog (another reset trigger button that restores its ring this way), and uses the exact same token, width, style, and offset asButton,Dialog,Pagination,Tab, andSlider. Purely additive; the ring only appears on keyboard focus, so mouse/touch interaction is unchanged.Changes
Collapsible/Collapsible.tsx: addoutline/outlineOffset:focus-visiblerules tostyles.trigger(restores the ring removed byall: unset).Test plan
StyleX compiles styles to atomic class names, so jsdom cannot assert the computed
outlinevalue -- the existing:focus-visibletests in the repo (Slider,Token,FileInput) verify focus behavior by mockingmatches(':focus-visible'), not the outline CSS itself. There is no established pattern for asserting a focus-ring style value, so I did not add a brittle class-name-snapshot test.node_modules/.bin/vitest run --root . packages/core/src/Collapsible-- 32 pass (existing suite stays green).node_modules/.bin/tsc --project packages/core/tsconfig.json --noEmit-- clean.node_modules/.bin/eslint packages/core/src/Collapsible/Collapsible.tsx-- clean.TabMenu). I could not run a browser in this environment, so this step is un-run; the values are copied verbatim from already-shipping components that render the ring correctly.Notes
Found during a broader a11y audit of core components. Scoped to only the focus-visible style on the Collapsible trigger --
aria-controlswiring is covered separately by #3707, and the chevron/other styling is untouched.