tailwindlabs / headlessui

Completely unstyled, fully accessible UI components, designed to integrate beautifully with Tailwind CSS.
https://headlessui.com
MIT License
25.76k stars 1.06k forks source link

Combobox selects the active value when the focus is moved away #2934

Closed tobia closed 7 months ago

tobia commented 8 months ago

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

Both 1.7.18 and 0.0.0-insiders.3b961a6

What browser are you using?

Firefox, Chrome

Reproduction URL

This can be easily reproduced on the demo page: https://headlessui.com/react/combobox

Describe your issue

  1. Open the demo page https://headlessui.com/react/combobox
  2. Click on the double-arrow button to open the list.
  3. Use the keyboard to navigate the list, for instance hitting Down a couple times: this will change the active item, but not the selected item.
  4. Do *not select the item with either Enter or Click, instead use Tab to move out of the field.

Expected behaviour: the field's selected value should remain unchanged.

Actual behaviour: the item that was active in the menu gets selected as the field value.

This is particularly problematic with the new immediate option available in the insiders release, because now you can alter the field values just by Tabbing through a form.

This is because as soon as the focus enters a Combobox in immediate mode, the menu of options is made visible and the first item is automatically made active, then hitting Tab again selects the active item, thus changing the value of the field.

Proposed solution: the active item should become selected only when a specific set of events occur, namely mouse Click or Enter key.

tobia commented 7 months ago

For what it's worth, I think the two cases in combobox.tsx where the code tests for data.activationTrigger !== ActivationTrigger.Focus should be removed. I replaced both tests with false and it solved the issue without introducing any obvious bugs.

Hassan1984 commented 7 months ago

or the solution porposed here. Relates to https://github.com/tailwindlabs/headlessui/issues/2932

Barbapapazes commented 7 months ago

Got the same issue https://github.com/unjs/website/issues/285 Clearly a regression from the previous version.

reinink commented 7 months ago

Hey! So the "tab to select" behavior is intended for this component. This is consistent with how other combobox libraries work, like React Aria Components, as well as with the native <datalist> element (see here).

If you're using the new immediate prop that's available in the insiders release, it will open the combobox immediately, but it won't select the first option if you hit tab right away — that will only happen if you press the up/down arrows to select an option, or if you perform a search. Meaning if you're simply tabbing through multiple inputs in a form, the combobox should not auto select the first option.

I have tested this on the latest insiders version of both the React and Vue libraries. Here's a video demonstrating this behavior (with the immediate prop enabled):

https://github.com/tailwindlabs/headlessui/assets/882133/bed2f70b-082e-4ff6-960e-27dbc48a1913

Going to close this one for now, since this all feels like expected behavior to me. However, if for some strange reason this isn't what you're experiencing, please provide a minimal reproduction and maybe even a video showing the issue and we can take another look 👍

sandros94 commented 6 months ago

Just throwing my two cents here, mainly on a click navigation:

Isn't the concept of "clicking away, without explicitely selecting something" be considered as "cancel the operation"? With the current implementation it seems to me that there is no way to cancel the operation using only mouse navigation

tobia commented 6 months ago

@Sandros94 I agree 100%. Which is the reason I'm using the patch I outlined in the 2nd post of this issue.

@reinink In addition to the point above, I was seeing a worse behaviour, because I'm loading the list of suggestions on the fly when the user focuses the field.

What happened was:

Would it be possible to have my patch above as a toggle prop? Would you accept a pull requests that adds the prop?

Fabioni commented 6 months ago

this behavior is reasonable in some cases but unreasonable in others, so please give us an option to deactivate it. Clicking away should be aborting the selection.

reinink commented 6 months ago

@Sandros94 @tobia @Fabioni Hey! So just looking at this issue again I am realizing that I never mentioned the nullable prop, which does allow you to click away without having the combobox automatically select the first option.

You can read more about this in my comment here: https://github.com/tailwindlabs/headlessui/issues/2932#issuecomment-1932953992

I think this will solve the issue for you. Right now, by default, the Combobox requires that a value is set, which is why it auto selects the first item if no other value is set already, however the nullable prop allows you to get around this.

We're actually considering making this the default behavior in Headless UI v2, although that's not a guarantee yet.

Hope that helps! 🙏

reinink commented 5 months ago

Hey folks! Just wanted to let you know that the upcoming v2 release is going to make the combobox "nullable by default". You can read more about this change in #3064.

Thanks for sharing all this feedback! 👍