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

Conversation

RobinMalfait
Copy link
Member

@RobinMalfait RobinMalfait commented Aug 8, 2023

This PR fixes a bug where clearing out the value of the Combobox.Input didn't always reset the value properly.

We will now make sure to:

  1. Ensure that pressing escape when in nullable mode goes back to the previous option
  2. Ensure that pressing escape when in nullable mode, after the input has been cleared doesn't go to any option
  3. Ensure that the onChange (React) or @update:modelValue (Vue) is being called with null when a reset happens (this is important for you to reset a potential query value for filtering purposes)
  4. Ensure that all kinds of "clearing the input" works, not only when working with escape/delete. This now also works with ctrl/cmd+x

One gotcha is that typically you reset the query for filtering in the onChange/@change of the Combobox.Input. However, when pressing escape the value is cleared but the onChange/@change won't be fired because it is cleared programmatically and not by the end user.

To solve this, you can use the following snippet to ensure that the query is still being cleared when the value changes to null:

React:

  import { useState } from 'react'
  import { Combobox } from '@headlessui/react'
  
  const people = [
    'Durward Reynolds',
    'Kenton Towne',
    'Therese Wunsch',
    'Benedict Kessler',
    'Katelyn Rohan',
  ]
  
  const App = () => {
    const [selectedPerson, setSelectedPerson] = useState(people[0])
    const [query, setQuery] = useState('')
  
    const filteredPeople =
      query === ''
        ? people
        : people.filter((person) => {
            return person.toLowerCase().includes(query.toLowerCase())
          })
  
    return (
      <div className="p-12">
        <Combobox
          value={selectedPerson}
          onChange={(value) => {
+           if (value === null) {
+               setQuery('') // You can also call this unconditionally
+           }
            setSelectedPerson(value)
          }}
          nullable
        >
          <Combobox.Input onChange={(event) => setQuery(event.target.value)} />
          <Combobox.Options>
            {filteredPeople.map((person) => (
              <Combobox.Option key={person} value={person}>
                {person}
              </Combobox.Option>
            ))}
          </Combobox.Options>
        </Combobox>
      </div>
    )
  }
  
  export default App

Vue:

  <template>
    <div class="p-12">
      <Combobox
        v-model="selectedPerson"
+       @update:modelValue="
+         (value) => {
+           if (value === null) {
+             query = '' // You can also call this unconditionally
+           }
+         }
+       "
        nullable
      >
        <ComboboxInput @change="query = $event.target.value" />
        <ComboboxOptions>
          <ComboboxOption v-for="person in filteredPeople" :key="person" :value="person">
            {{ person }}
          </ComboboxOption>
        </ComboboxOptions>
      </Combobox>
    </div>
  </template>
  
  <script setup>
  import { ref, computed } from 'vue'
  import { Combobox, ComboboxInput, ComboboxOptions, ComboboxOption } from '@headlessui/vue'
  
  const people = [
    'Durward Reynolds',
    'Kenton Towne',
    'Therese Wunsch',
    'Benedict Kessler',
    'Katelyn Rohan',
  ]
  
  let selectedPerson = ref(people[0])
  let query = ref('')
  
  const filteredPeople = computed(() =>
    query.value === ''
      ? people
      : people.filter((person) => {
          return person.toLowerCase().includes(query.value.toLowerCase())
        })
  )
  </script>

Fixes: #2433

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.
…ctive 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.
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.
@vercel
Copy link

vercel bot commented Aug 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
headlessui-react ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 8, 2023 5:03pm
headlessui-vue ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 8, 2023 5:03pm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing options when using nullable and the escape key
1 participant