Improve resetting values when using the nullable prop on the Combobox component (#2660)

* 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.

* ensure pressing `escape` also clears the input in nullable, single value mode without an active value

* 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.

* ensure `input` event is fired when firing keydown events

* 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.

* update changelog

* fix `than` / `then` typo
This commit is contained in:
Robin Malfait
2023-08-08 19:09:01 +02:00
committed by GitHub
parent 842890d054
commit 88b068cff1
9 changed files with 78 additions and 44 deletions
+1
View File
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Don't assume `<Tab />` 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
@@ -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)
@@ -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
@@ -871,23 +879,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
@@ -981,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:
@@ -1001,6 +1004,17 @@ 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 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
// ctrl/cmd+x.
if (data.nullable && data.mode === ValueMode.Single) {
if (event.target.value === '') {
clear()
}
}
// Open the combobox to show the results based on what the user has typed
actions.openCombobox()
})
@@ -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)
+1
View File
@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Don't assume `<Tab />` 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
@@ -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)
@@ -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 &&
@@ -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
@@ -837,24 +846,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
@@ -942,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
@@ -963,6 +966,17 @@ 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 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
// ctrl/cmd+x.
if (api.nullable.value && api.mode.value === ValueMode.Single) {
if (event.target.value === '') {
clear()
}
}
// Open the combobox to show the results based on what the user has typed
api.openCombobox()
}
@@ -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
@@ -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)