FE-822: Add Select to ds#8842
Conversation
PR SummaryMedium Risk Overview The dropdown reuses Shared form recipes are aligned for Select and TextInput: Reviewed by Cursor Bugbot for commit 1d75f91. Bugbot is set up for automated code reviews on this repo. Configure here. |
| ); | ||
| } | ||
|
|
||
| return <ItemRow key={entry.id} item={entry} ctx={ctx} />; |
There was a problem hiding this comment.
Menu group keys omit id
Low Severity
Grouped menu rows still use child.id as the React key while sibling rendering uses getItemId(child). For items that omit id but use string text, keys become undefined, which can cause duplicate-key warnings and unstable list updates even though menu values use getItemId.
Reviewed by Cursor Bugbot for commit 753ff53. Configure here.
| emptyState ?? | ||
| (loading ? "Loading options…" : "No options available") | ||
| } | ||
| /> |
There was a problem hiding this comment.
Select dropdown loading spinner missing
Medium Severity
When loading is true and items is empty, the list panel shows the emptyState text (e.g. “Loading options…”) instead of SelectableList’s loading UI, because loading is never passed through to SelectableList even though the trigger shows a spinner.
Reviewed by Cursor Bugbot for commit fe5a92a. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 5 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1d75f91. Configure here.
| const next = nextValue[0]; | ||
| if (next === NONE_VALUE) { | ||
| (onChange as (value: null) => void)(null); | ||
| return; |
There was a problem hiding this comment.
Dropdown clear skips onClear
Medium Severity
Choosing the optional empty list entry calls onChange(null) only, while the clear button invokes clearable.onClear(). Side effects tied to onClear (analytics, resetting related state) do not run when the user clears via the dropdown.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1d75f91. Configure here.
| createListCollection<Item>({ | ||
| items: flattenItems(menuItems), | ||
| itemToValue: (item) => getItemId(item), | ||
| itemToString: (item) => getItemId(item), |
There was a problem hiding this comment.
Typeahead uses value not label
Medium Severity
createListCollection sets itemToString to getItemId(item) (the option value), not the visible label from SelectItem.text or rendered item text. Keyboard typeahead and related string matching follow the raw value when label and value differ.
Reviewed by Cursor Bugbot for commit 1d75f91. Configure here.


🌟 What is the purpose of this PR?
Adds a Select component to the design system
Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR: