-
Notifications
You must be signed in to change notification settings - Fork 1
CRAFT-1822 | combobox refactor #630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 4ec2402 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
7b98059 to
3149372
Compare
3149372 to
e1b53ff
Compare
e1b53ff to
c2b0f32
Compare
c2b0f32 to
fa42fd3
Compare
fa42fd3 to
b84163a
Compare
6dd59e4 to
69c6c57
Compare
69c6c57 to
33b066e
Compare
| </Stack> | ||
| </ComboBox.Option> | ||
| )} | ||
| <ComboBox.Root size="md" items={options} placeholder="Search..."> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that 0b1a626 added slight changes so that the code examples would render appropriately.
Prior to the update, all Visual options displayed as Error: cannot be rendered outside a collection.
And the Async example displayed as ReferenceError: useAsyncList is not defined
valoriecarli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a teeny tiny commit, as well as a note for myself to handle i18n (first time we've updated existing keys/values).
Crazy amazing work, and the new Storybook tests made me smile.
A++
f4ada83 to
0b1a626
Compare
…nd collection are now being set in a usable way
…ct is working with tag group, filtering is working - basic functionality using single context achieved
… are looking good, still needs testing, etc tho
…ectly and add a few basic tests, add a set of custom filters for use with combobox and test them, update comments and documentation, update testing plan, update readme, etc
… child components in trigger do not receive external values
* chore(nimbus): combobox - layout structure tests * chore(nimbus): combobox - multi-select tag display tests * chore(nimbus): combobox - input field behavior tests * chore(nimbus): combobox - button visibility and behavior tests * chore(nimbus): combobox - focus behavior tests * chore(nimbus): notes * chore(nimbus): combobox - keyboard navigation tests * chore(nimbus): combobox - menu opening and closing tests * chore(nimbus): combobox - option selection tests * chore(nimbus): combobox - clear button functionality tests * chore(nimbus): combobox - multi and single select behavior tests * chore(nimbus): combobox - selection persistence tests * chore(nimbus): combobox - basic text filtering tests * chore(nimbus): combobox - custom filter tests * chore(nimbus): combobox - empty state handling tests * chore(nimbus): combobox - basic custom option creation tests * chore(nimbus): combobox - single select creation tests * chore(nimbus): combobox - a11y keyboard tests * chore(nimbus): combobox - a11y relationship tests * chore(nimbus): combobox - visual variant tests * chore(nimbus): combobox - state visual tests * chore(docs): remove temporary testing plan file * chore(combobox): revisions * feat(combobox option): options in listbox don't focus on hover * chore(combobox): add test for modal integration * chore(dialog): squelch button context for children * Revert "chore(dialog): squelch button context for children" This reverts commit af7f799. * chore(combobox): edit tests after rebase * chore(combobox): await in test, fix --------- Co-authored-by: Byron Wall <[email protected]>
0b1a626 to
4ec2402
Compare
misama-ct
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return ( | ||
| <Stack direction="column" gap="400"> | ||
| <ComposedComboBox aria-label="First combobox" items={simpleOptions} /> | ||
| <ComposedComboBox |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variant:solid story fails for me in the Storybook preview but not on the branch... approving anywayz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a light mode/dark mode thing. Fails for me on dark mode too, good catch

Refactored ComboBox to use a unified context-based architecture for better state management and added several missing features that were causing usability issues.
Why this change was necessary:
The previous ComboBox implementation had fragmented state management that made it difficult to coordinate behavior between the input, dropdown, and selection components. This caused issues with:
The new architecture consolidates state management into a single context pattern, enabling proper coordination between all component parts and making it straightforward to add features like async loading, custom filters, and creatable options. This brings ComboBox up to par with other production-grade autocomplete components and unblocks several user-facing feature requests.