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.

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

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

Merged
merged 7 commits into from
Aug 8, 2023
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
48 changes: 31 additions & 17 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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()
})
Expand Down
4 changes: 3 additions & 1 deletion packages/@headlessui-react/src/test-utils/interactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions packages/@headlessui-vue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
54 changes: 34 additions & 20 deletions packages/@headlessui-vue/src/components/combobox/combobox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion packages/@headlessui-vue/src/test-utils/interactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down