Repository navigation
UN-4301 [FEAT] Add the v1 selection inputs: Select, Combobox, MultiSelect and SegmentedControl (P1-T04) - #2466
Open
praveen-formido wants to merge 19 commits into
Open
praveen-formido wants to merge 19 commits into
praveen-formido wants to merge 19 commits into
Conversation
…classes Co-Authored-By: Claude Fable 5.1 <[email protected]>
…-ahead guards in tests Co-Authored-By: Claude Fable 5.1 <[email protected]>
Co-Authored-By: Claude Fable 5.1 <[email protected]>
…n the form pass-throughs in Select Co-Authored-By: Claude Fable 5.1 <[email protected]>
…tual list Co-Authored-By: Claude Fable 5.1 <[email protected]>
…ity, and close the engine's untested guards Co-Authored-By: Claude Fable 5.1 <[email protected]>
Co-Authored-By: Claude Fable 5.1 <[email protected]>
…ed a preset Combobox value Co-Authored-By: Claude Fable 5.1 <[email protected]>
Lift the icon-button class, the blur containment check and the list content props into shared.ts so the MultiSelect can reuse them; the Combobox behaves as before. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Several choices from a long or server-loaded list: chips before the input, Enter or a click toggles an option and keeps the list open with the query cleared, Backspace in an empty input removes the last chip, and a name submits one hidden input per value. Co-Authored-By: Claude Fable 5.1 <[email protected]>
…ab order, and pin the MultiSelect chevron A toggle clears the query but leaves its option active, so the arrows carry on from it. Chip gains removeFocusable; MultiSelect passes false, so Tab reaches the input in one stop. The chevron's open/close branch and its focus-keeping mousedown are now tested, and a controlled value with duplicates is collapsed. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Co-Authored-By: Claude Fable 5.1 <[email protected]>
…docs Co-Authored-By: Claude Fable 5.1 <[email protected]>
…, and open the Combobox on click Select wraps Radix's value element (which drops className) in a flex wrapper whose child variants truncate the label and keep the chevron in the trigger. Option ids follow the option's value, so the active id changes when the first match does. A click on the Combobox or MultiSelect input opens the list, the shown label is selected so typing replaces it, and a press on the MultiSelect frame focuses the input. Enter is kept from the form while a list is open; closing drops loaded options and opening reports loading at once. The listbox takes the field's name, the chevron controls it, virtual rows carry setsize and posinset, and onOpenChange(false) closes through close(). Co-Authored-By: Claude Fable 5.1 <[email protected]>
…k between Combobox and MultiSelect; give SegmentedControl the field props shared.ts becomes shared.tsx with ChevronButton, ListPopover, the ListFieldProps type and fieldInputProps, closeOnLeave, toggleList, and useLoadedOptions, which owns the served mirror, the seen map and the loader, and so creates the listbox itself. Both fields name a value from the current options, then any option seen, then the preset option(s). SegmentedControl takes id, aria-describedby and invalid on its group, drawn as a destructive outline outside the track. Co-Authored-By: Claude Fable 5.1 <[email protected]>
typeAheadIndex had no caller (Radix Select has its own type-ahead, and the Combobox and MultiSelect filter), so it and its tests go. The empty Select's "No options" row becomes a disabled option outside Radix's collection: axe rejects a status element inside the listbox even in a group, and an empty listbox, and a page scan of the open empty Select is now clean apart from Radix's modal aria-hidden-focus. The READMEs say which components take SelectOption and which SegmentedOption, and the Chip props are a table with className. Co-Authored-By: Claude Fable 5.1 <[email protected]>
praveen-formido
requested review from
athul-rs,
jaseemjaskp and
vishnuszipstack
November 10, 2026 05:27
praveen-formido
added
the
main-only
Contributor
 [Low impact] The PR appears safe to merge, with two non-blocking input edge cases to fix.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…sts at the free height and keep a saved value active while it loads Answers the first review of the selection inputs. A query change now resets useAsyncOptions to its loading state, so the previous query's results are neither shown nor selectable while the next load waits out the debounce (Enter during that wait selected a row the server had not matched). ListPopover's Listbox scroll element takes the popover's available-height cap beside the surface's 320px one, as Select's Viewport already did, so a short viewport scrolls the list instead of pushing the active option off-screen. Opening a server-loaded field with a saved value before its options arrive keeps that value active, so the first result no longer takes its place. The three axe-only tests assert through the registered matcher, which SonarCloud reads as an assertion; the helper call it could not see failed the quality gate. Co-Authored-By: Claude Fable 5.1 <[email protected]>
A saved value remembered while its server options were loading could arrive disabled and take the active row, so Enter did nothing although enabled options were there. The active index now skips a disabled match and falls back to the first enabled option, which also holds for an index set by hand. Co-Authored-By: Claude Fable 5.1 <[email protected]>
athul-rs
reviewed
Nov 10, 2026
athul-rs
left a comment
athul-rs
left a comment
Contributor
There was a problem hiding this comment.
Field and useFieldControl are in #2416 (P1-T03, not P1-T06 as the README says). Once it merges, could Select, Combobox and MultiSelect call useFieldControl(props) like Input and Textarea do? Then <Field label> and P1-T06's FormField label them with no wiring in each form, and they still work standalone because the hook passes the props through when there is no Field. SegmentedControl keeps aria-label/aria-labelledby as you note.
This was referenced Nov 11, 2026
# Conflicts: # frontend/src/v1/README.md
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
Comment on lines
+183
to
+187
| const onKeyDown = (event: KeyboardEvent<HTMLElement>) => { | ||
| switch (event.key) { | ||
| case "ArrowDown": | ||
| case "ArrowUp": | ||
| event.preventDefault(); |
Contributor
There was a problem hiding this comment.
While an IME is composing text, useListbox still captures ArrowUp and ArrowDown and prevents their normal action. These keys also move through IME candidates, so users entering Chinese or Japanese text can move through the picker instead of their text candidates. Return early for composing events before the key switch, not just inside the Enter branch.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/v1/components/ui/select/useListbox.ts
Line: 183-187
Comment:
**Arrows interrupt text composition**
While an IME is composing text, `useListbox` still captures ArrowUp and ArrowDown and prevents their normal action. These keys also move through IME candidates, so users entering Chinese or Japanese text can move through the picker instead of their text candidates. Return early for composing events before the key switch, not just inside the Enter branch.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+67
to
+71
| const [uncontrolled, setUncontrolled] = useState( | ||
| () => | ||
| defaultValue ?? options.find((option) => !option.disabled)?.value ?? "", | ||
| ); | ||
| const current = value ?? uncontrolled; |
Contributor
There was a problem hiding this comment.
SegmentedControl accepts an unknown defaultValue without checking it against options. For example, defaultValue="old" with options "list" and "grid" leaves every item unchecked. This breaks the documented first-enabled fallback and the promise that one item stays selected. Check the supplied default before using it; fall back to the first enabled option when it does not match.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/v1/components/ui/select/SegmentedControl.tsx
Line: 67-71
Comment:
**Unknown defaults leave nothing selected**
`SegmentedControl` accepts an unknown `defaultValue` without checking it against `options`. For example, `defaultValue="old"` with options `"list"` and `"grid"` leaves every item unchecked. This breaks the documented first-enabled fallback and the promise that one item stays selected. Check the supplied default before using it; fall back to the first enabled option when it does not match.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
Unstract test resultsPer-group results
Critical paths
|
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.
What
frontend/src/v1/components/ui/select/, on Radix and the P0-T04 tokens:Select(Radix Select): 32-tall trigger, placeholder, leading icon beside the label (Radix drops the value's class, so a wrapper lays it out), popover list with a check on the current choice; arrows, type-ahead, Home/End, Enter, Escape; a value that matches no option shows the placeholder; an empty list shows a disabled "No options" row (the one shape axe accepts inside Radix's listbox). For lists under about 50 options.Combobox: the ARIA combobox-with-listbox pattern (aria-activedescendantwith ids stable per option, so the active option is re-announced while filtering) over a Radix Popover; a click or ArrowDown/Up opens, the selected label is text-selected on focus so typing replaces it, typing filters, Enter selects, Escape/blur/Tab close and restore the selected label; chevron and Clear buttons;loadOptions(query, signal)for server-loaded options with a 250 ms debounce, aborts, stale responses dropped, loading, empty and error rows;selectedOptionnames a saved value before the first load; lists above 100 options render only the rows in view (@tanstack/react-virtual).MultiSelect: the same engine with chips (20 tall), a multiselectable list that stays open on toggle, Backspace removing the last chip when the input is empty, chip remove buttons named "Remove X" and kept out of the tab order,selectedOptionsfor preset values.SegmentedControl(Radix ToggleGroup single): 32 tall, 2 to 5 options, exactly one selected, rendered as a radio group with manual activation (arrows move focus, Space/Enter commit); takesid,aria-describedbyandinvalidlike the others.SelectOptionmodel (label, description, leading logo or icon, disabled, keywords for the filter),OptionRow,Chip,trigger.ts(the P1-T03 FieldFrame recipe so a select and an input sit identically),useListbox(keyboard and active-option engine),useAsyncOptions,Listbox, andshared.tsx(ChevronButton,ListPopover,useLoadedOptions) so Combobox and MultiSelect share one popover, chevron and loaded-options path.select.gallery.tsxlists sixteen static variants and states;markup.test.tsxsnapshots each one to__snapshots__/markup.mdand runs axe on each, asui/codedoes. A folder README and a "Selection inputs" section insrc/v1/README.md.Why
How
FieldFramerecipe (32 tall, radius 8,inputborder, hovermuted-foreground, invaliddestructive, ring only while the control is focused), copied not importedui:cursor-pointer(it is a button; the Combobox and MultiSelect frames hold a text input).rounded-lg border bg-popover p-1.5 shadow-md,max-h-80, trigger-width, a flex column with hidden overflow and one scroll element insideshadow-md. One scroll container keeps Radix's scroll-into-view working and lets both the surface cap and Radix's available-height cap apply (tailwind-merge collapses twomax-hon one element).<output>elements beside therole="listbox"elementSelectrenders every option; long and server lists go toComboboxkeywordsfeed that filtershadow-sm) and manual activationselectedOption/selectedOptionspropsCan this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
frontend/src/v1/components/ui/select/, plus astubLayouttest helper insrc/v1/test/dom.tsand a section insrc/v1/README.md. No v0 file, dependency or build file changes, and no v0 route renders v1.Database Migrations
Env Config
Relevant Docs
frontend/src/v1/components/ui/select/README.md;frontend/src/v1/README.md, Selection inputs section.Related Issues or PRs
main(adcf3ed). Builds on UN-4299 [FEAT] v1 design tokens, themes and style isolation (P0-T04) #2399 (P0-T04 tokens) and UN-4329 [FEAT] Add the v1 unit-test harness and the frontend-v1-unit rig group #2429 (P0-T08 harness). Independent of UN-4300 [FEAT] Add the v1 toggles: Checkbox, Radio and RadioGroup, Switch and Slider (P1-T05) #2462 (P1-T05 toggles).id,aria-describedby,invalid,disabled, matchinguseFieldControl's input; until P1-T06 lands, do not put these insideField); P1-T01 (gallery registration); P1-T12 (filter selects); P7-T05 (mode menu).--inputborder and--mutedtrack contrast raised on UN-4300 [FEAT] Add the v1 toggles: Checkbox, Radio and RadioGroup, Switch and Slider (P1-T05) #2462 applies to these triggers too; it needs the designer.Dependencies Versions
@tanstack/react-virtual3.14 was already a dependency (JsonView).Notes on Testing
aria-activedescendantchanging while filtering, a 1,000-option list rendering fewer than 60 rows witharia-setsize/aria-posinset). Every behavioural guard was mutation-checked: removing it fails a test.bun run test:v1: 618 tests green, coverage about 98% lines / 95% branches;lint:v1,typecheck:v1andbiome ci src/v1clean. The open-empty-Select axe test disables one rule,aria-hidden-focus, because Radix Select is modal and hides the page behind it.stubLayoutfrom@v1/test.claudedocs/frontend-v1-lane-c/plans/2026-11-09-P1-T04-sdd-ledger.md(not committed).🤖 Generated with Claude Code