feat(DST-1665): add selection to ListView - #5777
Open
sarahgm wants to merge 36 commits into
Open
Conversation
…ation `SelectList/SelectionIndicator` moves to `utils/GridSelectionIndicator` so `ListView` can consume it rather than fork it. Both consumers are `GridListItem`-backed rows, which is what distinguishes this indicator from `ListBox/SelectionIndicator` (a `contents` wrapper injecting a hidden `<Check>`) and from RAC's own, already imported by `SegmentedControl`. None of the three is publicly exported, so the rename is internal only. `selectionMode` widens to `@react-types/shared`'s `SelectionMode` and `'none'` now returns nothing, which `ListView` needs because it defaults to `'none'`. Returning `null` rather than an empty wrapper leaves the row's `auto` indicator track at zero width. `SelectList` can't reach that branch. No behaviour change to `SelectList`: same branches for `'single'` and `'multiple'`. Widening the prop type also removes the `as 'single' | 'multiple'` cast at the call site, which only existed because the type was narrower than the render prop it was fed from.
The row template gains an `indicator` column and area name, listed in both template rows so it spans them without a span count, the way `actions` already does. Adding a column is the only edit named areas need; nothing else in the template moves. Spacing rides on the cell (`me-3`) rather than a column gap, mirroring `actions`' `ms-3`. With no element in the area the `auto` track sizes to zero and the margin never applies, so a list without a `selectionMode` renders exactly as before. Also adds the `selected:` fill and the row cursor. The `not-disabled:` prefix on the cursor is required rather than tidiness: ungated, `data-selection-mode:cursor-pointer` has equal specificity to `disabled:cursor-not-allowed` and wins on source order, so a disabled row would show a pointer.
…xt stack The indicator aligned to the label row while the `action` slot beside it spanned both rows, so on an option carrying a description the two disagreed by roughly 8px. Both now span the stack, which is one rule for every non-text region rather than two. Done here rather than in a follow-up because `ListView` adds the same indicator in the same change: deferring would ship a divergence, a source comment explaining it, and a ticket to undo both. Visible only on options with a description; single-line options render identically.
`selectedKeys`, `defaultSelectedKeys`, `onSelectionChange` and `disallowEmptySelection` pass through from RAC untouched. `selectionMode` is redeclared to carry a doc comment and default to `'none'`, so selection is opt-in and no shipped list changes. `selectionBehavior` stays omitted and hardcoded to `'toggle'`, which is what lets a row carry focusable controls. No default for `disallowEmptySelection`, deliberately diverging from `SelectList`'s `true`-in-single: a view's selection is transient state the user must be able to abandon, and `true` also disables Escape-to-clear. `ListView.Item` moves to the render-prop form to reach `selectionMode` / `isSelected` / `isDisabled`, the same shape `SelectList.Option` uses. `ItemChildren` stays a separate component because the stacked-actions warning depends on running inside the row on RAC's second render pass. Adds `SingleSelection` and `MultipleSelection` stories. These are load-bearing rather than decoration: a bare `<ListView>` throws without a theme provider, so a story is the only way to exercise the new modes. Also trims this file's comments, which the header change forced anyway: it claimed every region claims its cell through a slot context, and the indicator does not.
Covers the ARIA shape (`aria-multiselectable` in multiple mode only, `aria-selected` present only when selectable, one gridcell per row) and the keyboard contract (arrow keys move focus without changing selection, Space selects, Escape clears). Adds `SelectionWithRowActions` to prove the GridList advantage over ListBox: pressing a row's `ActionMenu` opens it without toggling the row. The indicator now publishes `data-grid-area="indicator"`, so it is assertable like every other region instead of only reachable through a className. This also makes `SelectList`'s indicator assertable, which it was not. Two notes for review. The row checkbox announces as RAC's generic "Select" rather than taking its name from the row; identity lives on the row's own `aria-label` plus `aria-selected`. And selection announcements come from RAC's live region, which is relied on rather than asserted here. Running axe with `a11y.test: 'error'` shows the row description failing AA on a selected row (3.60:1, needs 4.5:1). That is a pre-existing system-wide token problem, not a selection one: `text-secondary` at 12px already fails on hover (4.35:1) in the shipped component, and ten theme files pair the two. Hover was never caught because axe only tests static states. Filed separately.
Four stories, all `component-test` tagged: - `SelectionWithMixedRowContent` is the regression story the review asked for: a selection column, two trailing controls in a `<ButtonGroup>`, and a non-Button child. It asserts every named region keeps its cell and that the badge, which claims nothing, cannot take one. - `SelectAll` shows the recipe rather than an API — a `<Checkbox>` above the list driving `onSelectionChange`, reading mixed while partial. - `SelectionWithRowActivation` pins RAC's `onAction` mode switch in both directions: a row press opens while nothing is selected, and toggles once something is. - `SelectableDisabledRows` exercises `disabledBehavior="selection"`, which had no observable effect before this ticket. Two findings worth keeping. A native checkbox carries its mixed state on the DOM `indeterminate` property, not as `aria-checked`, so the matcher is `toBePartiallyChecked`. And RAC drops `aria-selected` entirely on a selection-disabled row rather than reporting `false`, which is the right call: a row that cannot be selected should not advertise a selection state. Also corrects the `disabledBehavior` argType, which still claimed ListView has no selection, and renames a test that described the component where it meant the default. The two placement assertions added in edd05f6 moved here from the unit tests: `testing-library/no-node-access` forbids `querySelector` in `*.test.tsx`, and structure is the story tests' job anyway.
The indicator slot carried `justify-center`, copied from `SelectList`. Anything that widens the indicator column then centres the checkbox inside the wider box, so checkboxes drift out of line row to row. Measured with a `<Badge>` authored as a sibling of `<TextValue>`: the column went 24px to 64px and the checkbox moved 14px right of the row below it. `justify-self-start` also content-sizes the wrapper, so the mark sits at the column's start edge whatever else the row carries. Pinned by a story test comparing both rows' indicator `x`. Introduced in 5237c7a, so this is a fix within the ticket rather than a pre-existing defect.
Four changes, all from reviewing the stories rather than the code. `SelectAll` becomes `WithActionBar`. The select-all recipe was a bare checkbox above the list, and no placement fixed it: Marigold's Bulk Actions pattern puts select-all in the collection's header checkbox and the count in the ActionBar, and `ListView` has no header region. `<ActionBar>` needs no new API though — it reads `selectedItemCount` / `onClearSelection` from props and only falls back to the context `Table` publishes. So the story shows selection revealing the bar, with nothing at rest. Select-all is DST-1751. `SelectableDisabledRows` is gone, its assertion moved into the unit tests. With `disabledBehavior="selection"` RAC sets neither `data-disabled` nor `aria-disabled`, so every disabled style is inert and the row gives no reason it cannot be selected. Worth pinning as behaviour, not publishing as an example. The argType now says so. The `<Badge>` moves inside `<TextValue>`. As a sibling it auto-placed into an implicit third row in the indicator column, widening it and pushing that row's label out of line with the whole list. Nesting unslotted content inside a slotted parent is the rule the docs already state for `<Text>`. A test now compares both rows' indicator `x` so the alignment cannot regress. `selectionMode` gains a select control, and the six stories that hardcoded it after the args spread now set it through `args` instead, so the control is live rather than silently overridden.
Adds the Selection section the feature needs: the two modes and the mark each one shows, that the selection is view state the consumer commits, the dual gesture when `onAction` is set alongside it, and the ActionBar composition. Two demos carry it, one per mode. Also corrects three things the change makes wrong or incomplete: - "A `<ListView>` also has no selection" is gone, replaced by the line that still holds: the selection never submits. - The anatomy gains the indicator, and the row text moves right to clear its column. The label points at row 2, where there is room for it, and the viewBox stays at the width `docs/CLAUDE.md` pins. - The `<Badge>` guidance moves from "sits in the trailing cell while that cell is empty" to "nest it in the row's text". A selectable row makes the old advice actively harmful: an unslotted child lands in the indicator column and pushes every row's text out of line.
ADR-0006 requires the same decision test on both component pages, because a consumer who lands on `SelectList` first never sees a test written only on `ListView`. The test is one question, "does the selection need to submit with the form?", with the field-versus-view rationale underneath as explanation rather than as the decision aid. Two more pages pointed somewhere that is now wrong: - Bulk Actions is written around a `<Table>`, and stays that way, because select-all needs a header checkbox. It now says what a `<ListView>` can and cannot carry instead of leaving the reader to infer it. - ActionBar's "Outside a table" list names `<ListView>`, since a selectable list is the case that section describes best.
The section led with `selectionMode` and its two values, which answers "how" to a reader who has not yet decided "whether". It now opens with the two situations that earn selection, names the mode each one wants, and says to leave it off otherwise. Two paragraphs that went too deep for a usage section are gone with it: - The mark each mode shows is already an Anatomy bullet, and why the radio circle stays out of the accessibility tree is already under Accessibility. - `onAction` firing or staying quiet is restated as what the user sees, the row opening or being marked.
Two bullets describe a behaviour that changes under the reader's hands, which is exactly the case a demo carries better than prose. The line under the list names which gesture is live right now, so pressing a row before and after checking one shows the switch rather than asserting it.
It opened by asserting the two are easy to confuse and went straight into the question. Now it starts from what the reader has already noticed, that the two look alike on screen, names the axis that separates them, and only then puts the question.
The demo only printed a line claiming a file had opened, which asks the reader to trust a label while nothing on screen changes. A row press now leaves the list for the file's detail view, with a way back, so opening is something you watch happen and the second gesture reads as the contrast it is. Also corrects the paragraph under it. It said users know this from their file manager, which describes React Aria's `replace` behaviour: click selects, double click acts. Marigold hardcodes `toggle`, where the action fires only while nothing is selected. The paragraph now states the consequence that needs designing around, that opening is unreachable while a selection exists, and names Escape as the way back.
The detail view was more demo than the point needed: a second screen, a headline, a back button and an icon, for a section about which gesture a press performs. It is now the list, `onAction`, and an alert, the same idiom the other demos on the page use to show that something ran. The prose above already explains the two states, so the demo does not narrate them as well.
…urce ADR-0006 requires every deliberate divergence between the two wrappers to be commented where it lives, so it cannot later read as drift. The `disallowEmptySelection` default was the one divergence and the only uncommented one.
Measured rather than inferred, because the React Aria guide only covers clicking. With a selection present: Enter does nothing at all, not even what a click would do, Space marks the focused row, and Escape empties the selection so Enter opens again. The docs said "every row press marks a row instead", which is true of a click and wrong about Enter. They now name the dead key, since a keyboard user pressing Enter mid-selection gets no response of any kind, and say to keep a clear button in reach instead of relying on Escape.
The comment cited a record that is no longer in the repo. The reason it gives stands on its own, so it keeps the reason and loses the pointer.
Only the ListView copy got smoothed, so the shared parts had already drifted a day after being written. The opening and the rationale are now the same words on both pages, which they can be because both are symmetric about the two components. The closing paragraph stays page-specific on purpose. A reader on the SelectList page needs the warning about a dialog pick that never submits, which is where the wrong choice actually gets made; a reader on the ListView page needs to be told not to use "field versus view" as the decision aid.
Two copies of the same four paragraphs had already drifted once inside a day, which is the argument against keeping them. The ListView page owns the full comparison; this page keeps the answer inline, one sentence either way, so a reader still learns which component to use without following a link, then points at it for the reasoning. The dialog-pick warning stays here rather than moving with the rest. It is the mistake made on this page, by a reader who has already decided `<SelectList>` is the component, and it is the one thing the ListView page has no reason to say.
The decision test asks whether the selection submits with the form, which turns out to be necessary but not sufficient. Checking the Pick pattern found a pick with no `<form>` anywhere that still has to stay a `<SelectList>`: it carries a visible label, helper text, and a minimum-one rule, and `ListView` has no `label`, `description`, `errorMessage`, `error` or `required` prop to render any of that. Both pages now name the exception, so the test cannot send a labelled pick to a component that would silently drop its label.
"List or table" recommended a `<SelectList>` for any pick lighter than a table, which since this branch is wrong for half of them. It now chooses by surface: staged in a dialog and committed by its button is a `<ListView>`, a labelled field on the page stays a `<SelectList>`. It also says which of the page's own two demos is which, so the pair reads as a deliberate contrast rather than an inconsistency. The people pick migrates with it. `onChange(Key[])` becomes `onSelectionChange(Selection)`, which is not a rename: `Selection` can be the string `'all'`, so the off-view merge resolves that to the visible ids first. Without it a select-all would drop the staged rows that filtering had hidden, which is the bug the merge exists to prevent. The Abonnement pick is deliberately left as a `<SelectList>`. It is the labelled-field case from the exception above.
The plan asked for the scroll-padding caveat and I had written it as prose, describing a failure the demo could not show: the demo put the bar below a three-row list, which is the one arrangement that needs nothing. Checking it turned up the fact that changes the advice. `<ActionBar>` carries `sticky bottom-(--actionbar-offset)` in the component, not the theme, so it pins itself to whatever scroll container it is rendered inside. Nothing about that is Table-specific, and `useActionBar` is exported, so the composition a real consumer needs is a few lines rather than a warning. The demo now scrolls six rows, drives the bar with `useActionBar`, and reserves the measured height in both `padding-bottom` and `scroll-padding-bottom`. The paragraph moved ahead of it and says why that measurement is needed. One line after it keeps the simpler path for a list too short to scroll.
Three rather than the two the plan called for. The plan was written before the focus-ring change existed, and its argument for splitting the alignment fix out applies to that one too, more strongly: a consumer asking why their focus rings look different should find a line about focus rings, not have to read a selection feature entry to reach it. - feature: system/components/theme-rui minor, docs patch - SelectList indicator alignment: theme-rui patch - ListView row focus ring: theme-rui patch `@marigold/system` is listed because the `ListView` slot union gained `indicator`, which is a real change for anyone writing a custom theme.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
github-actions
Bot
added
type:docs
Contributor
|
Accessibility tests executed. Download the report here. |
`Theme['components']['ListView']` gained `indicator`, and the manifest is what `stub-missing-slots` reads to generate a consumer's missing slots. Left out, a consumer with their own ListView theme gets a type error on upgrade and no stub to fix it with, since the slot union is a `Record` and every key is required. `theme-drift.test.ts` caught it, which is the job it exists for.
…stories `findByRole` waits for presence, not visibility, so it resolved while the bar was still at `animate-slide-in`'s `opacity: 0` frame and `toBeVisible` counted it hidden. Green locally, red on the slower CI runner. Wrapped in `waitFor`, which is what every other ActionBar assertion in the repo does. The unit tests' selection block also built its own row fixture, provider wrapper and `Record<string, unknown>` prop bag while this branch was adding stories that render exactly that. It renders `MultipleSelection.Component` now, per CLAUDE.md, and the render-unmount-render test is two tests. Drops `onVenueSelectionChange`, a module-scope spy every test overrode, and takes `Selection` from `../types` like `Table.stories.tsx`.
`GridSelectionIndicator` stamped `data-grid-area="indicator"` itself, so every SelectList option carried an attribute naming a cell its grid has no named areas for. It takes a `gridArea` now and `ListViewItem` passes it, matching how label/title/description/actions already work: whoever supplies the positioning className supplies the attribute. `ListView`'s context memo could never hit, because `useClassNames` returns a fresh object every render. It memoises on the slot strings instead, which matters now that a selection change re-renders the list on every click. Two one-line rationales back in the theme: why the row has no column gap, and why the focus ring is inlined rather than `ui-state-focus-item`.
The selection demo invented a fourth venue dataset while `@/lib/data/venues`
is what `table-selection` and `selectlist-selectable-cards` both draw from.
It takes the same `slice(2, 5)`, so the three components the page tells you
to compare now show the same three records.
`pick-select-list.demo.tsx` contains no SelectList any more, so it is
`pick-list.demo.tsx`. Its merge handler stops spreading both branches into
arrays only to spread them into a Set.
Demo source is what consumers copy, so `Selection` comes from
`@marigold/components` rather than a transitive react-aria package, and the
inert `space={2}` and `[!code highlight:1]` count are gone.
🦋 Changeset detectedLatest commit: 3dbe62d The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 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 |
Contributor
|
Accessibility tests executed. Download the report here. |
The v18 manifest's `componentDependencies` names the components whose implementation hardcodes tokens the release added, so a consumer on a standalone theme is told to define them rather than discovering a partially unstyled component. It listed `SelectList` alone. `GridSelectionIndicator` is now rendered by `ListView` as well, and its single-select radio still hardcodes `selected-bold`, `selected-bold-foreground` and `disabled-surface`. Without an entry, `reportTokenDependencies` stays silent for a `ListView` with `selectionMode="single"`, because it only warns for components it finds imported. The multiple branch needs no entry: it is a themeable `<Checkbox>`.
Both the docs page and the changeset said an unslotted `<Badge>` pushes the text of every row in the list out of line. It does not. Each `ListView.Item` is its own grid inside a `flex flex-col` list, so the widened `auto` track is scoped to the row that carries the badge. That row stops lining up with the others, which do not move. Same visible symptom, wrong mechanism, in text that ships to consumers.
The test named "keeps the interactive cursor off a disabled row" rendered no disabled row and asserted that a class substring was present on an enabled one, so it could not fail for the cascade it exists to pin. Unit tests run in real Firefox with the theme CSS loaded, so it now disables a row and reads the computed cursor off both. Dropping the `not-disabled:` gate fails it with `expected 'pointer' to be 'not-allowed'`, which is the regression itself.
Contributor
|
Accessibility tests executed. Download the report here. |
Member
Author
|
/run-chromatic |
All six new selection stories were becoming Chromatic baselines. Two of them carry no visual information the other four do not. `SelectionWithRowActivation` renders the same two rows in the same mode as `MultipleSelection` with nothing selected. Its only arg difference is `onAction`, which has no visual effect, so the snapshot is a duplicate. The gesture switch it exists for is pinned by its tests. `WithActionBar` starts with an empty selection, and `ActionBar` is closed while `selectedItemCount` is 0, so the bar never reaches the snapshot. What Chromatic captured was `MultipleSelection` plus a row. The four that remain each hold a state nothing else does: the radio mark over a selected row, the bare checkbox row, an indicator beside the actions cell, and the badge alignment case.
Contributor
|
Accessibility tests executed. Download the report here. |
`onSelectionChange` can hand back the string `'all'`, not just a `Set`, and Cmd/Ctrl+A reaches it in any multi-select list. The page said select-all was missing and gave a keyboard inventory without the shortcut, so a reader had every reason to assume `'all'` could not occur here. Reading `.size` off it returns `undefined`, which lands as a wrong number in a confirmation rather than an error. Also names `disallowEmptySelection` where Escape is load-bearing: it blocks Escape too, and Escape is the only documented way out of the state where Enter does nothing.
Nothing covered the `'all'` branch, though every demo guards it and the count it feeds drives a destructive confirmation. Two tests: Cmd/Ctrl+A reports the string rather than a filled Set, and a controlled `'all'` marks every row. `Mod` resolves off `navigator.platform`, the same read react-aria's `isMac()` makes, so the story test agrees with the implementation on either platform instead of hardcoding one.
Contributor
|
Accessibility tests executed. Download the report here. |
Member
Author
|
/run-chromatic |
sarahgm
marked this pull request as ready for review
September 1, 2026 13:41
OsamaAbdellateef
self-requested a review
September 4, 2026 11:09
| @@ -1,10 +1,13 @@ | |||
| import { useState } from 'react'; | |||
Contributor
There was a problem hiding this comment.
🔴 Visual Regression Tests — not run
This PR changes UI-affecting files (packages/components/src/**, themes/**, and several *.stories.tsx files) but the Visual-Regression-Tests workflow has not run for this branch (feat/DST-1665-listview-selection).
The PR description itself calls out three visual changes to review (selection indicator column, <SelectList> indicator drop, focus-ring style). Please trigger the check by commenting /run-chromatic on this PR, or by manually dispatching the workflow.
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
2 participants
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.
Description
<ListView>shipped with selection deliberately omitted: all six selection props were stripped from its public type andselectionModewas hardcoded to"none". This adds it.Current behaviour: rows are operated where they sit, nothing can be marked.
New behaviour:
selectionMode="single"or"multiple"makes rows selectable, defaulting to"none"so a list that does not ask for selection is unchanged.The selection is view state, not a field value: read it through
onSelectionChange, hold it yourself, commit it when you decide. NoFieldBase, no hidden input, noname/form/validate. That split is what keeps twoGridListwrappers from being redundant now that both render a nearly identical stack of rows, and it is the question both docs pages now lead with: does the selection need to submit with the form?Signed off by Sebastian Sebald on 2026-08-04 in the ticket, with the two conditions carried through.
Closes DST-1665
What is in here
selectionMode,selectedKeys,defaultSelectedKeys,onSelectionChange,disallowEmptySelectionun-omitted.selectionBehaviorstays hidden and fixed to"toggle", matching<Table>.onSelectionChangecarries React Aria's rawSelection('all' | Set<Key>), deliberately not<SelectList>'s mode-typedonChange.indicatorregion; theListViewslot union in@marigold/systemgainsindicatorto match.SelectList/SelectionIndicator.tsx→utils/GridSelectionIndicator.tsx, used by both wrappers. Nothing was publicly exported, so this is internal naming only.indicatorin the anatomy, the decision test on both component pages, and the Pick pattern's dialog pick migrated from<SelectList>to<ListView>.Not in here
Select-all. The Bulk Actions pattern puts it in a header checkbox and a list has no header row to hold one. A multi-select list still composes with
<ActionBar>(driven byuseActionBar, which measures the bar so you can reserve its height), but a flow where users select every visible record still wants a<Table>. Filed as DST-1751.Highlight-style selection. No equivalent of Spectrum's
selectionStyle="highlight", so no press replaces the whole selection and Shift+click takes no range.<Table>is the same, so this is system-wide.In here, but outside the ticket's scope
DST-1665 lists three of the things in this PR under Not included, one of them as a decision it had already made the other way. All three are deliberate and two carry their own changeset, but "separate ticket" was the ticket's call, so reversing it is a reviewer's rather than the diff's.
1.
<SelectList>'s indicator alignment. The ticket scoped<ListView>'s centred indicator as a deliberate divergence, and put bringing<SelectList>into line in a separate ticket, being "a visible change to a merged component". It went further and pre-committed to "a source comment namingSelectList.styles.tsand this decision".Shipped here instead, with its own changeset and its own row in the visual-changes table below. The alternative was shipping a divergence plus a ticket to undo it, plus a source comment explaining a mismatch nobody had a reason to want. This is the one that reverses a settled decision rather than filling a gap, so it is the one most worth pushing back on.
2. The Pick pattern's people pick. The ticket put migrating the Pick demos onto the selectable
<ListView>in a follow-up depending on this one. The people pick moves here, because a dialog that stages a selection and commits it with its own button is the exact case the field-versus-view split was drawn for, and leaving it on<SelectList>ships docs that argue for a rule and a demo that breaks it. It touchespick-list.demo.tsxand the "List or table" section, which DST-1651 is also working in.3.
<ActionBar>. The ticket put bulk-action bar wiring in a separate follow-up. No wiring was added.<ActionBar>already takesselectedItemCountandonClearSelectionand only falls back to the context a<Table>publishes, so what is here is a story, a demo and a docs section. This one reads as demonstrating a composition that already worked rather than building it.Screenshots / Preview
Three visual changes for review, two of them in components this ticket was not about. Please look rather than bulk-accepting.
<ListView><SelectList>, all variants<ListView>rowTwo things I could not judge from source and would like a second opinion on:
/patterns/user-input/picknow uses<ListView>, which draws no frame where<SelectList>drew a boxed surface. Does it read as intentional inside the dialog, or unmoored?max-h-64scroll container so the sticky bar has something to pin to. Does the reserved padding at the bottom look deliberate or like a gap?Test Instructions
pnpm sb→ Components/ListView. Use the newselectionModedropdown (none/single/multiple) on any story.SelectionWithRowActivation: press a row with nothing selected (it opens), tick a checkbox, press another row (it marks instead). Press Escape, then press a row again.WithActionBar: select rows, check the count, clear it.SelectionWithMixedRowContent: confirm the indicators of both rows line up despite the badge.pnpm start→/components/collection/listview,/components/form/selectlist,/patterns/user-input/pick.pnpm test:sbandpnpm test:unit.Breaking Changes
No.
selectionModedefaults to"none", so existing lists are unchanged. The two visual changes are patch-level and carry their own changesets.Three findings worth a reviewer's attention
1. React Aria: Enter is inert while a selection exists. With
onActionset alongsideselectionMode, the gestures split by whether anything is selected. Measured, not inferred:A click at least marks the row. Enter fires no action and does not toggle, so a keyboard user gets no response and no hint why. React Aria's selection guide documents only "clicking or tapping a row toggles the selection" and says nothing about Enter going dead, so keyboard and pointer diverging here looks like an oversight rather than a decision. It is arguably a 2.1.1 concern: the open action is reachable by mouse in one state and by keyboard only in the other.
Pinned by a story test (
Enter opens only while the selection is empty) and named in the ListView docs, which tell you to keep a clear button in reach rather than relying on Escape. No ticket filed yet — worth deciding whether we report it upstream, work around it, or accept it.2. The decision test now lives on two pages with nothing checking they agree. It drifted once within a day of being written, which is why the SelectList page now keeps a one-sentence answer and cross-links to the full comparison rather than repeating it. There is no automated guard.
3. The
<SelectList>indicator converged on one axis, not two.row-span-2lines the indicator up with theactionslot beside it, which already centred against the whole text stack, so vertically the two wrappers now follow one rule. Horizontally they still differ:<ListView>justify-self-start, pinned to the column start<SelectList>justify-center, centred in the columnThis is only observable once the indicator column widens, which happens when a child claims no cell and auto-places into it.
<ListView>documents that case (a stray<Badge>) and pins the indicator so the widening cannot drag it.<SelectList>has no equivalent documented case, so the divergence is latent rather than live. Left alone deliberately, to keep this PR to one visual change in a merged component rather than two.The selection changeset says centring is the rule "both wrappers now follow, so the two cannot be told apart on this point". That holds for the vertical axis and overstates the horizontal one. Flagged here rather than edited, so the changelog keeps one sentence instead of a caveat.
Blast radius of the indicator move, since the changeset gives only the headline number:
<Description>do not move. The grid isgrid-rows-[minmax(1.25rem,auto)_auto], so the second row collapses to 0 and centring over one row or two lands on the same pixel.<Stack>that auto-places into row 1 and never uses<Description>, so row 2 stays empty. The tallest option shape in the system is untouched.One latent risk: a description that wraps to several lines drops the indicator further from the label it belongs to. Nothing in the repo does that today and the docs ask for a short description. If it comes up, the fix is
self-startpast a threshold rather than reverting torow-start-1, which would reintroduce the mismatch withaction.Checklist
component-testtag where applicable)/run-chromatic, see the three diffs abovepnpm changeset)🤖 Generated with Claude Code