Combobox improvements #1101
Combobox improvements #1101
Conversation
These are implemented on the `Combobox.Option` instead of the `Combobox.Options`. This allows you to have additional visual padding between `Combobox.Options` and `Combobox.Option` and if you hover over that area then the option becomes inactive. If we implement it on the `Combobox.Options` instead then this isn't _that_ easy to do. We can do it by checking the target and whether or not it is inside a headlessui-combobox-option. This would only have a single listener instead of `N` listeners though. Potential improvements!
|
This pull request is being automatically deployed with Vercel (learn more). headlessui-react – ./packages/playground-react🔍 Inspect: https://vercel.com/tailwindlabs/headlessui-react/Fv8h1q3htFVr2yNSHyz3A8tTZ2do headlessui-vue – ./packages/playground-vue🔍 Inspect: https://vercel.com/tailwindlabs/headlessui-vue/DUdRdU62EdzVDN2JWh8GwokAM6uH |
| @@ -111,7 +114,8 @@ let reducers: { | |||
| }, | |||
| [ActionTypes.GoToOption](state, action) { | |||
| if (state.disabled) return state | |||
| if (state.comboboxState === ComboboxStates.Closed) return state | |||
| if (!state.optionsPropsRef.current.static && state.comboboxState === ComboboxStates.Closed) | |||
There was a problem hiding this comment.
Would it make sense to treat a combobox with static options as always open or would that break more things?
There was a problem hiding this comment.
I think that it might be okay for the combobox but not required. We can't do it for a Listbox / Menu because of focus trap / tab trap behaviour that is enabled based on that open / closed state. So maybe for that reason alone we shouldn't do it here either for consistency?
You’ve taken control of the open/close state yourself in which case this should be allowed to be handled by other event handlers
This PR contains a bunch more improvements after internal testing and dog fooding the component.