Repository navigation
fix(combobox): restore selected-pill focus & multi-selection semantics (#3211) - #3213
Merged
interactivellama merged 3 commits intoSep 28, 2026
Merged
Conversation
…ics (#3211) The rewrite (#3195) dropped the tabIndex/aria-selected passthrough on the Pill root element, so selected pills in a multi-select Combobox were no longer focusable and roving-tabindex / arrow-key navigation broke. - pill: forward `tabIndex` and `aria-selected` to the root span so option pills participate in the listbox roving tabindex. - selected-listbox: add `multiselectable` prop and set `aria-multiselectable` on the owning listbox so a list with multiple aria-selected pills exposes consistent selection semantics to assistive tech. - combobox: pass `multiselectable={multiple}` to all SelectedListBox usages. - stories: make Base / BaseWithMenuSubheader multi-select so the pill navigation behavior is demonstrable in Storybook/Chromatic. - tests: regression coverage for pill attribute passthrough and combobox multi-selection ARIA + roving tabindex.
Contributor
|
@showerbee The CI is failing. It looks like unit tests were updated, but the HTML/DOM snapshots were not updated. Please update the HTML/DOM snapshots. I believe it's |
…snapshots (#3211) Restoring aria-selected on option pills (previous commit) means the pill container now renders multiple aria-selected pills inside a role="listbox". A pill container is always a collection of selected options, so mark its owning listbox aria-multiselectable for consistent selection semantics — mirroring the Combobox fix. Also regenerates the story HTML snapshots, which now reflect the restored tabIndex/aria-selected passthrough and the aria-multiselectable listboxes (CI verify was failing on stale snapshots).
…inks (#3211) The selection-change effect only re-seeded the active selected option when it was unset, so a controlled parent removing a pill through any path other than the built-in remove handler could leave activeSelectedOptionIndex pointing past the end of `selection`. With the restored roving tabindex, that left no pill with tabindex=0, making the selected-pill listbox unreachable by keyboard (WCAG 2.1.1 / 2.4.3). Clamp the active index into range, mirroring the same guard PillContainer already has. Adds a regression test that shrinks a controlled selection while the last pill is active and asserts exactly one pill remains tabbable.
Contributor
|
@showerbee This appears to fix the issues with the base example and also makes the single selection inline-listbox (what we typically think of a standard combobox for SF entities), so we can move forward with it. I'm still seeing some missing Storybook examples that were present in the previous iteration. This is the original list minus the stories present only for snapshot testing.
|
interactivellama
approved these changes
Sep 28, 2026
interactivellama
left a comment
interactivellama
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the quick turnaround.
interactivellama
deleted the
fix/3211-combobox-pill-focus-navigation
branch
September 28, 2026 05:04
This was referenced Sep 28, 2026
interactivellama
pushed a commit
that referenced
this pull request
Oct 1, 2026
* docs(combobox): restore Storybook coverage lost in TS rewrite (#3213) The React 19/TS rewrite (#3195) replaced storybook-stories.jsx with Combobox.stories.tsx but dropped coverage for several already-supported props/variants: field-level help tooltip, menu separators, inherited menu width, scrollable menu, custom input, search/add-entity items, controlled open state, async loading, popover (dialog) variant, per-option disabled tooltips, and readonly-variant disabled options/deselect/custom item rendering. No component code changes are needed — this only restores Storybook documentation parity per the gap flagged in review. * fix(combobox): wire up onRequestRemoveSelectedOption in stories missing it Several stories rendered a removable selection pill but never handled its remove event, so clicking the pill's X did nothing. Pre-existing on master, not introduced by the new stories added in this PR. * test(combobox): add HTML snapshots for new Storybook stories The story-snapshot suite auto-discovers every story via composeStories, so the 14 stories added in this PR need their own committed snapshots. No existing snapshots changed. --------- Co-authored-by: showerbee <13712377+showerbee@users.noreply.github.com>
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
Fixes #3211.
The React 19 rewrite (#3195) rebuilt the
Pillcomponent and dropped thetabIndex/aria-selectedpassthrough that the previous implementationspread onto the root element. As a result, selected pills in a multi-select
Combobox were no longer focusable, and roving-tabindex / arrow-key navigation
between pills stopped working. The Base story also silently became
single-select, hiding the regression.
Changes
components/pill/index.tsx— forwardtabIndexandaria-selectedtothe root
<span>sooption-variant pills participate in the listboxroving tabindex and expose their selected state.
components/pill-container/private/selected-listbox.tsx— add amultiselectableprop and setaria-multiselectableon the owning listboxelement, so a listbox containing multiple
aria-selectedpills reportsconsistent multi-selection semantics to assistive technology.
components/combobox/combobox.tsx— passmultiselectable={multiple}to all three
SelectedListBoxusages (base, inline, read-only).components/combobox/__docs__/Combobox.stories.tsx— make theBaseandBaseWithMenuSubheaderstories multi-select with seeded pills so the pillnavigation is demonstrable in Storybook/Chromatic.
Testing
tabIndex/aria-selectedreach the root, andtabIndexis omitted when not provided.aria-multiselectablewitharia-selectedoption pills and a rovingtabindex; inline-listbox exposes the container as the multiselectable
listbox with the inner list as a group; single-selection is not marked
multiselectable.
npx vitest runfor the pill + combobox suites: 36 passed.Accessibility behavior (roving tabindex)
The selected-pill group is a single Tab stop by design (WAI-ARIA listbox
pattern): Tab enters the group onto the active pill, arrow keys (← / →) move
between pills, and the next Tab exits the group. This matches the pre-rewrite
(0.10.x) behavior.