Allow custom values in Combobox and DynamicCombobox#948
Allow custom values in Combobox and DynamicCombobox#948lkostrowski wants to merge 6 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: be8bff9 The changes in this PR will be included in the next version bump. 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 |
There was a problem hiding this comment.
Pull request overview
Adds support for submitting user-entered (non-option) values in both Static Combobox and DynamicCombobox, with Storybook examples and a changeset for release.
Changes:
- Introduces
allowCustomValue,onCustomValueSubmit, andlocale.addNewLabelprops to combobox components. - Adds a new “Add new: {input}” row in dropdown menus and Enter-key submission support via
useCombobox. - Adds Storybook stories demonstrating custom value submission + locale override, and a changeset for versioning.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/Combobox/Common/useCombobox.tsx | Adds custom-value detection, Enter handling, and submission helper plumbing. |
| src/components/Combobox/Static/Combobox.tsx | Renders custom submit row in the static combobox menu and wires new props. |
| src/components/Combobox/Dynamic/DynamicCombobox.tsx | Renders custom submit row in the dynamic combobox menu and wires new props. |
| src/components/Combobox/Static/StaticCombobox.stories.tsx | Adds stories for allow-custom-value and localized label. |
| src/components/Combobox/Dynamic/DynamicCombobox.stories.tsx | Adds an allow-custom-value story for dynamic combobox. |
| .changeset/pretty-foxes-pull.md | Declares a minor release and describes the new props. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return; | ||
| } | ||
|
|
||
| setLoading(true); | ||
| const response = await fetch( | ||
| `https://swapi.dev/api/people/?search=${criteria}` | ||
| ); | ||
| const body = await response.json(); | ||
|
|
||
| setOptions( | ||
| body.results.map((result: { name: string }) => ({ | ||
| value: result.name, | ||
| label: result.name, | ||
| })) | ||
| ); | ||
| setLoading(false); |
There was a problem hiding this comment.
If criteria becomes empty, this returns early without resetting loading to false. After a previous search set loading true, the story can get stuck in a loading state. Consider calling setLoading(false) (and possibly clearing any in-flight request) before returning.
| return; | |
| } | |
| setLoading(true); | |
| const response = await fetch( | |
| `https://swapi.dev/api/people/?search=${criteria}` | |
| ); | |
| const body = await response.json(); | |
| setOptions( | |
| body.results.map((result: { name: string }) => ({ | |
| value: result.name, | |
| label: result.name, | |
| })) | |
| ); | |
| setLoading(false); | |
| setLoading(false); | |
| return; | |
| } | |
| setLoading(true); | |
| try { | |
| const response = await fetch( | |
| `https://swapi.dev/api/people/?search=${criteria}` | |
| ); | |
| const body = await response.json(); | |
| setOptions( | |
| body.results.map((result: { name: string }) => ({ | |
| value: result.name, | |
| label: result.name, | |
| })) | |
| ); | |
| } finally { | |
| setLoading(false); | |
| } |
| "@saleor/macaw-ui": minor | ||
| --- | ||
|
|
||
| Combobox and DynamicCombobox now accepts new props that allow entering custom values |
There was a problem hiding this comment.
Grammar: “Combobox and DynamicCombobox now accepts …” should use plural verb (“now accept …”).
| Combobox and DynamicCombobox now accepts new props that allow entering custom values | |
| Combobox and DynamicCombobox now accept new props that allow entering custom values |
| </List.Item> | ||
| )} | ||
|
|
||
| {isOpen && !hasItemsToSelect && children} |
There was a problem hiding this comment.
When hasCustomValueToSubmit is true, the empty-state children (e.g. <Combobox.NoOptions />) will still render because the condition is only !hasItemsToSelect. This can show “No items to select” alongside the “Add new …” row; consider also checking !hasCustomValueToSubmit (or rendering one or the other).
| {isOpen && !hasItemsToSelect && children} | |
| {isOpen && !hasItemsToSelect && !hasCustomValueToSubmit && children} |
| const handleCustomValueSubmit = (value: string) => { | ||
| onCustomValueSubmit?.(value); | ||
| closeMenu(); | ||
| setInputValue(""); | ||
| }; |
There was a problem hiding this comment.
handleCustomValueSubmit calls React’s setInputValue(""), but the actual input element value is managed by Downshift (since useDownshiftCombobox controls getInputProps). This means the visible input likely won’t clear after submitting a custom value (even though local state is cleared). Consider clearing Downshift’s input value via its action helpers (e.g. destructure and call setInputValue/reset) or fully control Downshift’s inputValue with your state so UI and filtering stay in sync.
| </List.Item> | ||
| )} | ||
|
|
||
| {isOpen && !loading && !hasItemsToSelect && children} |
There was a problem hiding this comment.
When hasCustomValueToSubmit is true, the empty-state children will still render because the condition is only !hasItemsToSelect. This can display a “No options” message alongside the “Add new …” row; consider also checking !hasCustomValueToSubmit (or render one or the other).
| {isOpen && !loading && !hasItemsToSelect && children} | |
| {isOpen && | |
| !loading && | |
| !hasItemsToSelect && | |
| !hasCustomValueToSubmit && | |
| children} |
| <List.Item | ||
| data-test-id="combobox-custom-value" | ||
| className={listItemStyle} | ||
| onClick={() => handleCustomValueSubmit(inputValue)} |
There was a problem hiding this comment.
The custom “Add new …” row isn’t created via Downshift getItemProps, so it won’t be an ARIA option and isn’t reachable via arrow-key navigation within the menu. Consider integrating it into Downshift’s items array (and using getItemProps) or adding equivalent ARIA/keyboard handling.
| onClick={() => handleCustomValueSubmit(inputValue)} | |
| {...getItemProps({ | |
| item: inputValue as unknown as T, | |
| index: itemsToSelect?.length ?? 0, | |
| onClick: () => handleCustomValueSubmit(inputValue), | |
| })} | |
| active={highlightedIndex === (itemsToSelect?.length ?? 0)} |
| const hasItemsToSelect = itemsToSelect.length > 0; | ||
| const trimmedInputValue = inputValue.trim(); | ||
| const hasCustomValueToSubmit = | ||
| !!allowCustomValue && !hasItemsToSelect && trimmedInputValue.length > 0; |
There was a problem hiding this comment.
hasCustomValueToSubmit can become true even when onCustomValueSubmit is not provided. In that case the UI will offer an “Add new …” action but won’t notify the consumer. Consider requiring onCustomValueSubmit when allowCustomValue is true (type-level) and/or include !!onCustomValueSubmit in the hasCustomValueToSubmit condition.
| !!allowCustomValue && !hasItemsToSelect && trimmedInputValue.length > 0; | |
| !!allowCustomValue && | |
| !!onCustomValueSubmit && | |
| !hasItemsToSelect && | |
| trimmedInputValue.length > 0; |
| const trimmedInputValue = inputValue.trim(); | ||
| const hasCustomValueToSubmit = | ||
| !!allowCustomValue && !hasItemsToSelect && trimmedInputValue.length > 0; |
There was a problem hiding this comment.
You trim inputValue for submission/display, but filtering still uses the untrimmed inputValue state. Trailing/leading whitespace can prevent matches and incorrectly trigger the “Add new …” path. Consider trimming for filtering as well (or normalizing inputValue when updating state) so filtering and custom submission use the same canonical value.
Co-authored-by: Jonatan Witoszek <jonatanwitoszek@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const AllowCustomValue = () => { | ||
| const [options, setOptions] = useState<Option[]>([]); | ||
| const [value, setValue] = useState<Option | null>(null); | ||
| const [loading, setLoading] = useState(false); | ||
|
|
||
| const handleInputValueChange = async (criteria: string) => { | ||
| if (!criteria) { | ||
| setOptions([]); | ||
| return; | ||
| } | ||
|
|
||
| setLoading(true); | ||
| const response = await fetch( | ||
| `https://swapi.dev/api/people/?search=${criteria}` | ||
| ); | ||
| const body = await response.json(); | ||
|
|
||
| setOptions( | ||
| body.results.map((result: { name: string }) => ({ | ||
| value: result.name, | ||
| label: result.name, | ||
| })) | ||
| ); | ||
| setLoading(false); | ||
| }; | ||
|
|
||
| return ( | ||
| <DynamicCombobox | ||
| __width="300px" | ||
| value={value} | ||
| label="Pick or create a character" | ||
| onChange={(value) => setValue(value)} | ||
| options={options} | ||
| loading={loading} | ||
| onInputValueChange={(inputValue) => { | ||
| handleInputValueChange(inputValue); | ||
| }} | ||
| allowCustomValue | ||
| onCustomValueSubmit={fn()} | ||
| /> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
PR description mentions an AllowCustomValueWithLocale story for the dynamic combobox as well, but this file only adds AllowCustomValue. Either add the locale variant story here (to exercise locale.addNewLabel) or update the PR description/test plan to match what’s included.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Adds support for user-entered custom values in both
Combobox(static) andDynamicCombobox(dynamic) components. When no matching options exist andallowCustomValueis enabled, users can submit their own text via Enter key or by clicking the "Add new" item in the dropdown.Changes
useComboboxhook: AddedallowCustomValueandonCustomValueSubmitprops. Tracks when a custom value can be submitted (no matching items + non-empty input). Handles Enter key submission and exposeshasCustomValueToSubmit/handleCustomValueSubmitfor consumers.Combobox(static): NewallowCustomValue,onCustomValueSubmit, andlocale.addNewLabelprops. Renders an "Add new: {value}" list item when applicable.DynamicCombobox: Same new props pluslocale.addNewLabel. Renders the custom value item below search results when no matches are found.AllowCustomValueandAllowCustomValueWithLocalestories for both combobox variants.Other changes included in this branch
sprinkles.css.ts: Addedinlineto display property valuesBaseInput.css.ts: AddeduserSelect: noneto floating label spansTest plan
AllowCustomValuestories for both Static and Dynamic comboboxonCustomValueSubmitfires, menu closes, input clearslocale.addNewLabeloverrides the default "Add new" labelallowCustomValueis not set🤖 Generated with Claude Code