From 5cf1b983c75c099176b938563a142765d13a842f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 8 Aug 2023 17:24:55 +0200 Subject: [PATCH 1/7] move `nullable` handling to `onChange` of `Combobox.Input` itself We were specifically handling backspace/delete keys to verify if the `Combobox.Input` becomes empty then we can clear the value if we are in single value and in nullable mode. However, this doesn't capture other ways of clearing the `Combobox.Input`, for example when use `cmd+x` or `ctrl+y` in the input. Moving the logic, gives us some of these cases for free. --- .../src/components/combobox/combobox.tsx | 32 ++++++++--------- .../src/components/combobox/combobox.ts | 34 +++++++++---------- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index d551fc9207..c06e6c4c46 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -871,23 +871,6 @@ function InputFn< switch (event.key) { // Ref: https://www.w3.org/WAI/ARIA/apg/patterns/menu/#keyboard-interaction-12 - case Keys.Backspace: - case Keys.Delete: - if (data.mode !== ValueMode.Single) return - if (!data.nullable) return - - let input = event.currentTarget - d.requestAnimationFrame(() => { - if (input.value === '') { - actions.onChange(null) - if (data.optionsRef.current) { - data.optionsRef.current.scrollTop = 0 - } - actions.goToOption(Focus.Nothing) - } - }) - break - case Keys.Enter: isTyping.current = false if (data.comboboxState !== ComboboxState.Open) return @@ -1001,6 +984,21 @@ function InputFn< // options while typing won't work at all because we are still in "composing" mode. onChange?.(event) + // When the value becomes empty in a single value mode while being nullable than we want to clear + // the option entirely. + // + // This is can happen when you press backspace, but also when you select all the text and press + // ctrl/cmd+x. + if (data.nullable && data.mode === ValueMode.Single) { + if (event.target.value === '') { + actions.onChange(null) + if (data.optionsRef.current) { + data.optionsRef.current.scrollTop = 0 + } + actions.goToOption(Focus.Nothing) + } + } + // Open the combobox to show the results based on what the user has typed actions.openCombobox() }) diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index 18baa4140d..2d3b606dc8 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -837,24 +837,6 @@ export let ComboboxInput = defineComponent({ switch (event.key) { // Ref: https://www.w3.org/WAI/ARIA/apg/patterns/menu/#keyboard-interaction-12 - case Keys.Backspace: - case Keys.Delete: - if (api.mode.value !== ValueMode.Single) return - if (!api.nullable.value) return - - let input = event.currentTarget as HTMLInputElement - requestAnimationFrame(() => { - if (input.value === '') { - api.change(null) - let options = dom(api.optionsRef) - if (options) { - options.scrollTop = 0 - } - api.goToOption(Focus.Nothing) - } - }) - break - case Keys.Enter: isTyping.value = false if (api.comboboxState.value !== ComboboxStates.Open) return @@ -963,6 +945,22 @@ export let ComboboxInput = defineComponent({ // options while typing won't work at all because we are still in "composing" mode. emit('change', event) + // When the value becomes empty in a single value mode while being nullable than we want to clear + // the option entirely. + // + // This is can happen when you press backspace, but also when you select all the text and press + // ctrl/cmd+x. + if (api.nullable.value && api.mode.value === ValueMode.Single) { + if (event.target.value === '') { + api.change(null) + let options = dom(api.optionsRef) + if (options) { + options.scrollTop = 0 + } + api.goToOption(Focus.Nothing) + } + } + // Open the combobox to show the results based on what the user has typed api.openCombobox() } From 003794ce22f36c6ba05cfd88c277b52686f1d173 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 8 Aug 2023 17:50:26 +0200 Subject: [PATCH 2/7] ensure pressing `escape` also clears the input in nullable, single value mode without an active value --- .../src/components/combobox/combobox.tsx | 26 +++++++++++++---- .../src/components/combobox/combobox.ts | 28 +++++++++++++++---- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index c06e6c4c46..017a49763c 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -745,6 +745,14 @@ function InputFn< let d = useDisposables() + let clear = useEvent(() => { + actions.onChange(null) + if (data.optionsRef.current) { + data.optionsRef.current.scrollTop = 0 + } + actions.goToOption(Focus.Nothing) + }) + // When a `displayValue` prop is given, we should use it to transform the current selected // option(s) so that the format can be chosen by developers implementing this. This is useful if // your data is an object and you just want to pick a certain property or want to create a dynamic @@ -964,6 +972,18 @@ function InputFn< if (data.optionsRef.current && !data.optionsPropsRef.current.static) { event.stopPropagation() } + + if (data.nullable && data.mode === ValueMode.Single) { + // We want to clear the value when the user presses escape if and only if the current + // value is not set (aka, they didn't select anything yet, or they cleared the input which + // caused the value to be set to `null`). If the current value is set, then we want to + // fallback to that value when we press escape (this part is handled in the watcher that + // syncs the value with the input field again). + if (data.value === null) { + clear() + } + } + return actions.closeCombobox() case Keys.Tab: @@ -991,11 +1011,7 @@ function InputFn< // ctrl/cmd+x. if (data.nullable && data.mode === ValueMode.Single) { if (event.target.value === '') { - actions.onChange(null) - if (data.optionsRef.current) { - data.optionsRef.current.scrollTop = 0 - } - actions.goToOption(Focus.Nothing) + clear() } } diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index 2d3b606dc8..87061a25a8 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -707,6 +707,15 @@ export let ComboboxInput = defineComponent({ expose({ el: api.inputRef, $el: api.inputRef }) + function clear() { + api.change(null) + let options = dom(api.optionsRef) + if (options) { + options.scrollTop = 0 + } + api.goToOption(Focus.Nothing) + } + // When a `displayValue` prop is given, we should use it to transform the current selected // option(s) so that the format can be chosen by developers implementing this. This is useful if // your data is an object and you just want to pick a certain property or want to create a dynamic @@ -924,6 +933,18 @@ export let ComboboxInput = defineComponent({ if (api.optionsRef.value && !api.optionsPropsRef.value.static) { event.stopPropagation() } + + if (api.nullable.value && api.mode.value === ValueMode.Single) { + // We want to clear the value when the user presses escape if and only if the current + // value is not set (aka, they didn't select anything yet, or they cleared the input which + // caused the value to be set to `null`). If the current value is set, then we want to + // fallback to that value when we press escape (this part is handled in the watcher that + // syncs the value with the input field again). + if (api.value.value === null) { + clear() + } + } + api.closeCombobox() break @@ -952,12 +973,7 @@ export let ComboboxInput = defineComponent({ // ctrl/cmd+x. if (api.nullable.value && api.mode.value === ValueMode.Single) { if (event.target.value === '') { - api.change(null) - let options = dom(api.optionsRef) - if (options) { - options.scrollTop = 0 - } - api.goToOption(Focus.Nothing) + clear() } } From 8221f7d0e6c881dc8230d972e553140b7c673e5a Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 8 Aug 2023 18:16:03 +0200 Subject: [PATCH 3/7] adjust test to ensure we don't have a selected option instead of an active option We still will have an active option (because we default to the first option if nothing is active while the combobox is open). But since we cleared the value when using the `nullable` prop, then it means the `selected` option should be cleared. --- .../src/components/combobox/combobox.test.tsx | 4 ++-- .../@headlessui-vue/src/components/combobox/combobox.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index eb57a61e5c..e1f4b55800 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -2735,9 +2735,9 @@ describe('Keyboard interactions', () => { await press(Keys.Backspace) expect(getComboboxInput()?.value).toBe('') - // Verify that we don't have an active option anymore since we are in `nullable` mode + // Verify that we don't have an selected option anymore since we are in `nullable` mode assertNotActiveComboboxOption(options[1]) - assertNoActiveComboboxOption() + assertNoSelectedComboboxOption() // Verify that we saw the `null` change coming in expect(handleChange).toHaveBeenCalledTimes(1) diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts index a66d0930a9..dedeeaca85 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts @@ -4554,9 +4554,9 @@ describe('Keyboard interactions', () => { await press(Keys.Backspace) expect(getComboboxInput()?.value).toBe('') - // Verify that we don't have an active option anymore since we are in `nullable` mode + // Verify that we don't have an selected option anymore since we are in `nullable` mode assertNotActiveComboboxOption(options[1]) - assertNoActiveComboboxOption() + assertNoSelectedComboboxOption() // Verify that we saw the `null` change coming in expect(handleChange).toHaveBeenCalledTimes(1) From f2e0c8a003cca26742f03058a4451e87f1e49b88 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 8 Aug 2023 18:17:13 +0200 Subject: [PATCH 4/7] ensure `input` event is fired when firing keydown events --- packages/@headlessui-react/src/test-utils/interactions.ts | 4 +++- packages/@headlessui-vue/src/test-utils/interactions.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/@headlessui-react/src/test-utils/interactions.ts b/packages/@headlessui-react/src/test-utils/interactions.ts index 747ef523fa..dd0298fd7b 100644 --- a/packages/@headlessui-react/src/test-utils/interactions.ts +++ b/packages/@headlessui-react/src/test-utils/interactions.ts @@ -157,7 +157,9 @@ let order: Record< value: element.value.slice(0, -1), }), }) - return fireEvent.keyDown(element, ev) + + fireEvent.keyDown(element, ev) + return fireEvent.input(element, ev) } return fireEvent.keyDown(element, event) diff --git a/packages/@headlessui-vue/src/test-utils/interactions.ts b/packages/@headlessui-vue/src/test-utils/interactions.ts index 57c2ac7f32..612849023b 100644 --- a/packages/@headlessui-vue/src/test-utils/interactions.ts +++ b/packages/@headlessui-vue/src/test-utils/interactions.ts @@ -155,7 +155,9 @@ let order: Record< value: element.value.slice(0, -1), }), }) - return fireEvent.keyDown(element, ev) + + fireEvent.keyDown(element, ev) + return fireEvent.input(element, ev) } return fireEvent.keyDown(element, event) From 28fd8fd6a4529ebde4466024c3c5a9e1c408bf9b Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 8 Aug 2023 18:51:28 +0200 Subject: [PATCH 5/7] ensure `defaultToFirstOption` is always set when going to an option We recently made a Vue improvement that delayed the going to an option, but this also included a bug where the `defaultToFirstOption` was not set at the right time anymore. --- packages/@headlessui-vue/src/components/combobox/combobox.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index 87061a25a8..dc39a62131 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -281,13 +281,13 @@ export let Combobox = defineComponent({ comboboxState.value = ComboboxStates.Open }, goToOption(focus: Focus, id?: string, trigger?: ActivationTrigger) { + defaultToFirstOption.value = false + if (goToOptionRaf !== null) { cancelAnimationFrame(goToOptionRaf) } goToOptionRaf = requestAnimationFrame(() => { - defaultToFirstOption.value = false - if (props.disabled) return if ( optionsRef.value && From b64485eeb507534d63e5c2d238594fd215821e98 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 8 Aug 2023 19:00:46 +0200 Subject: [PATCH 6/7] update changelog --- packages/@headlessui-react/CHANGELOG.md | 1 + packages/@headlessui-vue/CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 54259325c7..624f22081a 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Don't assume `` components are available when setting the next index ([#2642](https://github.com/tailwindlabs/headlessui/pull/2642)) - Fix incorrectly focused `Combobox.Input` component on page load ([#2654](https://github.com/tailwindlabs/headlessui/pull/2654)) - Ensure `appear` works using the `Transition` component (even when used with SSR) ([#2646](https://github.com/tailwindlabs/headlessui/pull/2646)) +- Improve resetting values when using the `nullable` prop on the `Combobox` component ([#2660](https://github.com/tailwindlabs/headlessui/pull/2660)) ## [1.7.16] - 2023-07-27 diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 0d08b0dad1..2613dc8ae3 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Don't assume `` components are available when setting the next index ([#2642](https://github.com/tailwindlabs/headlessui/pull/2642)) - Improve SSR of the `Disclosure` component ([#2645](https://github.com/tailwindlabs/headlessui/pull/2645)) - Fix incorrectly focused `ComboboxInput` component on page load ([#2654](https://github.com/tailwindlabs/headlessui/pull/2654)) +- Improve resetting values when using the `nullable` prop on the `Combobox` component ([#2660](https://github.com/tailwindlabs/headlessui/pull/2660)) ## [1.7.15] - 2023-07-27 From 8a7078c59fb9e208aa9050d0e4224380e250289a Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 8 Aug 2023 19:02:34 +0200 Subject: [PATCH 7/7] fix `than` / `then` typo --- packages/@headlessui-react/src/components/combobox/combobox.tsx | 2 +- packages/@headlessui-vue/src/components/combobox/combobox.ts | 2 +- .../src/components/transitions/utils/transition.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 017a49763c..5e7f38a1ec 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -1004,7 +1004,7 @@ function InputFn< // options while typing won't work at all because we are still in "composing" mode. onChange?.(event) - // When the value becomes empty in a single value mode while being nullable than we want to clear + // When the value becomes empty in a single value mode while being nullable then we want to clear // the option entirely. // // This is can happen when you press backspace, but also when you select all the text and press diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index dc39a62131..a5bec86433 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -966,7 +966,7 @@ export let ComboboxInput = defineComponent({ // options while typing won't work at all because we are still in "composing" mode. emit('change', event) - // When the value becomes empty in a single value mode while being nullable than we want to clear + // When the value becomes empty in a single value mode while being nullable then we want to clear // the option entirely. // // This is can happen when you press backspace, but also when you select all the text and press diff --git a/packages/@headlessui-vue/src/components/transitions/utils/transition.ts b/packages/@headlessui-vue/src/components/transitions/utils/transition.ts index 008a01375a..a269ba5cfb 100644 --- a/packages/@headlessui-vue/src/components/transitions/utils/transition.ts +++ b/packages/@headlessui-vue/src/components/transitions/utils/transition.ts @@ -86,7 +86,7 @@ export function transition( // then we have some leftovers that should be cleaned. d.add(() => removeClasses(node, ...base, ...from, ...to, ...entered)) - // When we get disposed early, than we should also call the done method but switch the reason. + // When we get disposed early, then we should also call the done method but switch the reason. d.add(() => _done(Reason.Cancelled)) return d.dispose