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.

Sign up for GitHub

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

Jump to bottom

combobox.input value gets out of sync with search term state #1875

Closed
stefanprobst opened this issue Sep 27, 2022 · 4 comments · Fixed by #1916
Closed

combobox.input value gets out of sync with search term state #1875

stefanprobst opened this issue Sep 27, 2022 · 4 comments · Fixed by #1916
Assignees
@thecrypticace

Comments

@stefanprobst
Copy link

What package within Headless UI are you using?

@headlessui/react - Combobox

What version of that package are you using?

1.7.2

What browser are you using?

firefox

Reproduction URL

https://github.com/stefanprobst/headlessui-combobox

Describe your issue

in a multiple combobox, the combobox.input value is set to an empty string when clicking outside without selecting anything. however, since this is set directly on the ref, no input/change event is fired, so the searchterm useState will not get notified. in this example, you can see that when focusing the combobox input again, the searchTerm is still set to the previously entered phrase, even though the input is empty:

combobox.mp4
@thecrypticace thecrypticace self-assigned this Sep 28, 2022
@robertkern
Copy link

Just fyi, I updated to 1.7.3 and this issue exists there too

@thecrypticace thecrypticace mentioned this issue Oct 10, 2022
Merged
@thecrypticace
Copy link
Contributor

Hey, thanks for reporting this!

This should be fixed by #1916, and will be available in the next release.

You can already try it using (may take a few minutes to publish to npm):

  • npm install @headlessui/react@insiders.
  • npm install @headlessui/vue@insiders.

@frankleng
Copy link

@thecrypticace still running into this via @headlessui/react@insiders
we set empty input when user types in special char, ie >
Combox.Input is calling onChange with the char even with input state was set to empty

@RobinMalfait RobinMalfait mentioned this issue Nov 23, 2022
Merged
RobinMalfait added a commit that referenced this issue Nov 23, 2022
@RobinMalfait
syncing of the input should happen when the value changes internally …
a09dd84 
…or externally

I also got rid of the manually dispatching of the change event if the
value changes from internally.

I think the correct mental model is:
- That the `Combobox.Input` value should change if the selected value
  changes from the outside or from the inside.
  - Note: It should _not_ do that if you are currently typing (once you
    choose a new value it will re-sync, once you reset (escape / outside
    click) it will also sync again).
- The `onChange`/`onInput` of the `Combobox.Input` itself should only be
  called if you as the user type something. Not when the value is
  "synced" based on the selected value. We were currently manually
  dispatching events which works (to a certain extend) but smelled a bit
  fishy to me.

The manual dispatching of events tried to solve an issue
(#1875), but I think
this can be solved in 2 other ways that make a bit more sense:

1. (Today) Use the `onBlur` on the input to reset the `query` value to filter
   options.
2. (In the future)  Use an exposed `onClose` (or similar) event to reset
   your `query` value.
RobinMalfait added a commit that referenced this issue Nov 23, 2022
@RobinMalfait
Improve syncing of the Combobox.Input value (#2042)
4da0b3a 
* make combobox playgrounds in React and Vue similar

* syncing of the input should happen when the value changes internally or externally

I also got rid of the manually dispatching of the change event if the
value changes from internally.

I think the correct mental model is:
- That the `Combobox.Input` value should change if the selected value
  changes from the outside or from the inside.
  - Note: It should _not_ do that if you are currently typing (once you
    choose a new value it will re-sync, once you reset (escape / outside
    click) it will also sync again).
- The `onChange`/`onInput` of the `Combobox.Input` itself should only be
  called if you as the user type something. Not when the value is
  "synced" based on the selected value. We were currently manually
  dispatching events which works (to a certain extend) but smelled a bit
  fishy to me.

The manual dispatching of events tried to solve an issue
(#1875), but I think
this can be solved in 2 other ways that make a bit more sense:

1. (Today) Use the `onBlur` on the input to reset the `query` value to filter
   options.
2. (In the future)  Use an exposed `onClose` (or similar) event to reset
   your `query` value.

* update changelog

* ignore flakey test
lightshine555 pushed a commit to lightshine555/headless-ui-tailwind that referenced this issue Sep 6, 2023
@RobinMalfait
Improve syncing of the Combobox.Input value (#2042)
1d204fd 
* make combobox playgrounds in React and Vue similar

* syncing of the input should happen when the value changes internally or externally

I also got rid of the manually dispatching of the change event if the
value changes from internally.

I think the correct mental model is:
- That the `Combobox.Input` value should change if the selected value
  changes from the outside or from the inside.
  - Note: It should _not_ do that if you are currently typing (once you
    choose a new value it will re-sync, once you reset (escape / outside
    click) it will also sync again).
- The `onChange`/`onInput` of the `Combobox.Input` itself should only be
  called if you as the user type something. Not when the value is
  "synced" based on the selected value. We were currently manually
  dispatching events which works (to a certain extend) but smelled a bit
  fishy to me.

The manual dispatching of events tried to solve an issue
(tailwindlabs/headlessui#1875), but I think
this can be solved in 2 other ways that make a bit more sense:

1. (Today) Use the `onBlur` on the input to reset the `query` value to filter
   options.
2. (In the future)  Use an exposed `onClose` (or similar) event to reset
   your `query` value.

* update changelog

* ignore flakey test
@eexmellie
Copy link

We wanted to include Combobox component into our app, but ran into the same issue on version 1.7.16 of @headlessui/vue.

I saw the comment for the last commit linked to this issue with the suggestion of "Use the onBlur on the input to reset the query value to filter options".

Unfortunately, this does not look feasible. We're using multiple prop and the issues are:

  • blur event is fired on each click while selecting/deselecting an option, so we can't just reset filter query on any blur event.
  • in case of clicking outside, blur is fired before the value is actually cleared and the open slot prop is set to false. Correct me if I'm wrong, but there is no straightforward and non-hacky way to know that this blur event is the one that we need to use to reset the filter query.

So I was wondering if there are any plans for fixing and/or improving this particular behaviour in the forseeable future?

googoogaga0xbb added a commit to googoogaga0xbb/ui-headless that referenced this issue Nov 29, 2023
@googoogaga0xbb
Improve syncing of the Combobox.Input value (#2042)
afc1e5d 
* make combobox playgrounds in React and Vue similar

* syncing of the input should happen when the value changes internally or externally

I also got rid of the manually dispatching of the change event if the
value changes from internally.

I think the correct mental model is:
- That the `Combobox.Input` value should change if the selected value
  changes from the outside or from the inside.
  - Note: It should _not_ do that if you are currently typing (once you
    choose a new value it will re-sync, once you reset (escape / outside
    click) it will also sync again).
- The `onChange`/`onInput` of the `Combobox.Input` itself should only be
  called if you as the user type something. Not when the value is
  "synced" based on the selected value. We were currently manually
  dispatching events which works (to a certain extend) but smelled a bit
  fishy to me.

The manual dispatching of events tried to solve an issue
(tailwindlabs/headlessui#1875), but I think
this can be solved in 2 other ways that make a bit more sense:

1. (Today) Use the `onBlur` on the input to reset the `query` value to filter
   options.
2. (In the future)  Use an exposed `onClose` (or similar) event to reset
   your `query` value.

* update changelog

* ignore flakey test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@thecrypticace thecrypticace

Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@thecrypticace @robertkern @frankleng @stefanprobst @eexmellie