Skip to content

Commit

Permalink
Improve resetting values when using the nullable prop on the `Combo…
Browse files Browse the repository at this point in the history
…box` 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
  • Loading branch information
RobinMalfait authored Aug 8, 2023
1 parent 842890d commit 88b068c
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 44 deletions.
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

2 comments on commit 88b068c

@vercel
Copy link

@vercel vercel bot commented on 88b068c Aug 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

headlessui-vue – ./packages/playground-vue

headlessui-vue-tailwindlabs.vercel.app
headlessui-vue.vercel.app
headlessui-vue-git-main-tailwindlabs.vercel.app

@vercel
Copy link

@vercel vercel bot commented on 88b068c Aug 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

headlessui-react – ./packages/playground-react

headlessui-react-git-main-tailwindlabs.vercel.app
headlessui-react.vercel.app
headlessui-react-tailwindlabs.vercel.app

Please sign in to comment.