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

Add immediate prop to <Combobox /> for immediately opening the Combobox when the input receives focus #2686

Merged
merged 10 commits into from
Aug 31, 2023

Conversation

tzurbaev
Copy link
Contributor

@tzurbaev tzurbaev commented Aug 21, 2023

Maintainers note:

This PR introduces a new immediate prop on the <Combobox /> component to open the <Combobox /> immediately once the <Combobox.Input /> receives focus.

It does handle edge cases where focus is moved into the <Combobox.Input /> without opening the <Combobox /> again. These edge cases are:

  • Clicking the <Combobox.Button /> to close the <Combobox /> will move focus into the <Combobox.Input /> this will not open the <Combobox /> again.
  • Clicking on a <Combobox.Option /> will temporarely move focus to the <Combobox.Option />, then focus is moved back into the <Combobox.Input />. This will not open the <Combobox /> again.

Another edge case that is important to mention:

  • When a <Combobox /> is open, the first <Combobox.Option /> is selected. Tabbing away will select the currently active <Combobox.Option />. If immediate mode is enabled, and you are tabbing through a form with multiple inputs, once you hit the <Combobox.Input /> the <Combobox /> will be opened and the first <Combobox.Option /> will be selected. Tabbing again will not select this option to prevent accidental and unwanted state mutations to the current form.

This is my attempt to make combobox options open on input focus, as there're many requests in discussions (#1236).

This adds boolean openOnFocus prop both to Vue's ComboboxInput & React's Combobox.Input components. Initially I tried to add it to root Combobox components, but due to my lack of React & TS types knowledge, I couldn't figure out how to introduce new boolean prop on React's component.

If openOnFocus is present, focusing on input (by clicking on label/input or tabbing into input) will make options list appear. I made sure that this changes properly work when options list is already visible and user tries to close it by clicking the combobox button. Also it won't show options list if combobox button is disabled.

handleFocus function inside ComboboxInput components checks if options list should be opened. The HTMLBodyElement instance check added mostly for tests because for some reason body element is being passed as relatedTarget in test environment and it breaks the checks.

The only UI change this PR introduces is when user clicks on some option in single-mode combobox or clicks the button, input won't be refocused. I added additional check for openOnFocus prop (to the !isMobile() condition and to button's handleClick) before refocusing input because without it options list will stay open until user clicks on something non-related to combobox.

I'm not sure what scenario is better (not refocusing input or not closing combobox), so I decided to make it at least visually similar to how combobox works now.

Also, in React component I added new action SetOpenOnFocus because I needed to somehow mutate openOnFocus value of ComboboxDataContext so ComboboxOption component can perform check before refocusing input. I have limited experience with React, so I don't know if this is the best way to do it.

I added several new tests around openOnFocus prop and created new playground pages (http://localhost:3000/combobox/combobox-open-on-focus) for both frameworks (they are copy-pasted from pure Tailwind examples).

@vercel
Copy link

vercel bot commented Aug 21, 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 31, 2023 1:23pm
headlessui-vue ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 31, 2023 1:23pm

tzurbaev and others added 7 commits August 31, 2023 14:11
… selected item

When you have a fwe inputs such as:

```html
<form>
   <input />
   <input />
   <input />
   <Combobox>
      <Combobox.Input />
   </Combobox>
   <input />
   <input />
   <input />
</form>
```

Tabbing through this list will open the combobox once you are on the
input field. When you continue tabbing, the first item would be
selected. However, if the combobox is not marked as nullable, it means
that just going through the form means that we set a value we can't
unset anymore.

We still want to open the combobox, we just don't want to select
anything in this case.
outside

If the focus is coming from the `<Combobox.Button />` or as a side
effect of selecting an `<Combobox.Option />` then we don't want to
re-open the `<Combobox />`
Safari doesn't fire a `focus` event when clicking a button, therefore it
does not become the `document.activeElement`, and events like `blur` or
`focus` doesn't set the button as the `event.relatedTarget`.

Keeping track of a history like this solves that problem. We already had
the code for the `FocusTrap` component.
@RobinMalfait RobinMalfait merged commit fa95262 into tailwindlabs:main Aug 31, 2023
7 checks passed
@RobinMalfait
Copy link
Collaborator

Hey, thanks so much for this contribution! 🙏

I made a few adjustments, and we decided to name the prop immediate which lives on the <Combobox /> itself for immediately opening the <Combobox /> once the <Combobox.Input /> receives focus.

You can already try it using:

  • npm install @headlessui/react@insiders.
  • npm install @headlessui/vue@insiders.

@tzurbaev
Copy link
Contributor Author

@RobinMalfait thank you for your adjustments and for merging this PR! Much appreciated!

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.

None yet

2 participants