Skip to content
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

feat: add onStateChange callback to Command component #972

Merged
merged 4 commits into from
Dec 10, 2024

Conversation

max-got
Copy link

@max-got max-got commented Dec 4, 2024

As i was trying to reimplement Multiple Selector for Origin UI Svelte i didn't found a way to access the current filtered items for select-{46|47}. See Original "Multiselect, Multiselect with placeholder and clear."

Initially i wanted to give the user something like useCommandState, but that felt wrong. But maybe there is a better solution. I just wanted to kickstart something like this! Feel free to close this, if you think that is the wrong approach!

This PR introduces a new onStateChange callback to the Command component, allowing consumers to track and react to internal state changes.

Changes

  • Added onStateChange prop to CommandRootProps that receives a readonly snapshot of the component's internal state
  • Implemented state batching mechanism to prevent redundant callback triggers during related updates (e.g., when typing triggers both filtering and selection changes)
  • Updated API documentation with the new onStateChange functionality

Implementation Details

The state management system now:

  • Batches related state updates to prevent multiple emissions
  • Uses afterTick to ensure all related updates are processed before notifying
  • Provides a readonly snapshot of the state to prevent unintended mutations
  • Skips notifications during the initial mount phase to avoid unnecessary updates

Example Usage

<Command.Root
onStateChange={(state) => {
// Access current search query, selected value, and filtered items
console.log('Search:', state.search);
console.log('Selected:', state.value);
console.log('Filtered count:', state.filtered.count);
}}
>
<!-- Command components -->
</Command.Root>

Testing

No test files found, tho i tested locally:

  • Verified that the callback fires only once per logical state change
  • Tested with various interaction patterns (typing, selecting, hovering)

…e notifications

- Introduced `onStateChange` prop in `CommandRootProps` to allow external handling of internal state changes.
- Updated `CommandRootState` to manage state updates more efficiently, including debouncing state change notifications.
- Enhanced `Command` component to pass `onStateChange` prop to the root state.
- Updated API documentation to reflect the new `onStateChange` functionality.
Copy link

changeset-bot bot commented Dec 4, 2024

🦋 Changeset detected

Latest commit: cadca87

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
bits-ui Patch

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

Copy link
Contributor

github-actions bot commented Dec 4, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
bits-ui ✅ Ready (View Log) Visit Preview e4ceea5

@huntabyte
Copy link
Owner

Awesome work @max-got!

I intentionally left this part out of the move over to Bits UI as I wanted to spend time thinking about how best to enable the user to manage that state, given if we have an on<Thing>Change, we also have a controlledThing and bind:thing, etc.

Where this starts to get messy is when we do provide the bindable interface.

The user can't just do cmdstate.search = "whatever I want" due to the batching of the updates and internal logic, so we'd need to come up with a way to allow the user to pass in a search state that we provide the prototype of so we can intercept those manual updates.

In the meantime though, we can merge this in as it wouldn't be a breaking change to add those in the future.

@huntabyte
Copy link
Owner

It seems there was a regression introduced though, demonstrated by the following video where the highlight doesn't restore to the top of the list:

CleanShot.2024-12-04.at.11.04.42.mp4

- Removed unnecessary flags for first mount and updating state.
- Introduced a new scheduling mechanism for state updates to improve efficiency.
- Simplified the emit logic by consolidating it into a single scheduling function.
- Enhanced the handling of item selection during filtering and sorting operations.
- Updated related methods to ensure consistent state updates without redundant emissions.
@github-actions github-actions bot requested a deployment to Preview December 5, 2024 17:54 Abandoned
@max-got
Copy link
Author

max-got commented Dec 5, 2024

I've changed the logic a quite a bit. tests for the combobox would be awesome tho. for now i am flying blind, but the issue should be fixed.

@huntabyte
Copy link
Owner

tests for the combobox would be awesome tho

Agreed 😃 , it's on my list of things to do

but the issue should be fixed.

Thank you, I'll give it a check soon!

Copy link
Owner

@huntabyte huntabyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@huntabyte huntabyte merged commit 38d38f4 into huntabyte:next Dec 10, 2024
5 checks passed
@github-actions github-actions bot mentioned this pull request Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants