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

Keep focus inside of the <ComboboxInput /> component #3073

Merged
merged 16 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Prevent unnecessary execution of the `displayValue` callback in the `ComboboxInput` component ([#3048](https://github.com/tailwindlabs/headlessui/pull/3048))
- Expose missing `data-disabled` and `data-focus` attributes on the `TabsPanel`, `MenuButton`, `PopoverButton` and `DisclosureButton` components ([#3061](https://github.com/tailwindlabs/headlessui/pull/3061))
- Fix cursor position when re-focusing the `ComboboxInput` component ([#3065](https://github.com/tailwindlabs/headlessui/pull/3065))
- Keep focus inside of the `<ComboboxInput />` component ([#3073](https://github.com/tailwindlabs/headlessui/pull/3073))

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5194,7 +5194,7 @@ describe.each([{ virtual: true }, { virtual: false }])('Mouse interactions %s',
options={[
{ value: 'alice', children: 'Alice', disabled: false },
{ value: 'bob', children: 'Bob', disabled: true },
{ value: 'charile', children: 'Charlie', disabled: false },
{ value: 'charlie', children: 'Charlie', disabled: false },
]}
/>
)
Expand Down
84 changes: 72 additions & 12 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import {
import { useDescribedBy } from '../description/description'
import { Keys } from '../keyboard'
import { Label, useLabelledBy, useLabels, type _internal_ComponentLabel } from '../label/label'
import { MouseButton } from '../mouse'

enum ComboboxState {
Open,
Expand Down Expand Up @@ -1077,8 +1078,21 @@ function InputFn<
})
})

let dd = useDisposables()
Copy link
Member

Choose a reason for hiding this comment

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

The dd variable here is confusing to me — what does it mean? I know we typically use d for disposables, so is it just because you have two disposables in this case? Wondering if we can come up with a better name here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep we already had d but that had a different use case. To solve this, I created a useFrameDebounce() hook (we can improve this name if we want). But now we can do this instead:

image

let handleKeyDown = useEvent((event: ReactKeyboardEvent<HTMLInputElement>) => {
isTyping.current = true

// Re-set the typing flag, this behaves as a debounce-like implementation,
// so as long as we are typing we will not reset the `isTyping` flag, but
// once we stop typing we will reset it.
{
d.add(dd.dispose) // Ensure we cleanup when `d` is disposed
dd.dispose() // Cleanup previous disposables
dd.nextFrame(() => {
isTyping.current = false
})
}

switch (event.key) {
// Ref: https://www.w3.org/WAI/ARIA/apg/patterns/menu/#keyboard-interaction-12

Expand Down Expand Up @@ -1388,11 +1402,26 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
switch (event.key) {
// Ref: https://www.w3.org/WAI/ARIA/apg/patterns/menu/#keyboard-interaction-12

case Keys.Space:
case Keys.Enter:
event.preventDefault()
event.stopPropagation()
if (data.comboboxState === ComboboxState.Closed) {
actions.openCombobox()
}

return d.nextFrame(() => refocusInput())

case Keys.ArrowDown:
event.preventDefault()
event.stopPropagation()
if (data.comboboxState === ComboboxState.Closed) {
actions.openCombobox()
d.nextFrame(() => {
if (!data.value) {
actions.goToOption(Focus.First)
}
})
}

return d.nextFrame(() => refocusInput())
Expand Down Expand Up @@ -1424,16 +1453,31 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
}
})

let handleClick = useEvent((event: ReactMouseEvent<HTMLButtonElement>) => {
if (isDisabledReactIssue7711(event.currentTarget)) return event.preventDefault()
if (data.comboboxState === ComboboxState.Open) {
actions.closeCombobox()
} else {
event.preventDefault()
actions.openCombobox()
let handleMouseDown = useEvent((event: ReactMouseEvent<HTMLButtonElement>) => {
// We use the `mousedown` event because this will fire before the `focus`
// event. When we use `event.preventDefault()` here, the
// `document.activeElement` will stay on the current element.
//
// Typically that means that the `ComboboxInput` will stay focused, and thus
// the selection / cursor position will be preserved.
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved
event.preventDefault()

if (isDisabledReactIssue7711(event.currentTarget)) return

// When we used the `click` event we didn't have to worry about this because
// that only fires if you use the `left` mouse button. However, now that
// we use the `mousedown` event, we do have to worry about which button is
// pressed.
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved
if (event.button === MouseButton.Left) {
if (data.comboboxState === ComboboxState.Open) {
actions.closeCombobox()
} else {
actions.openCombobox()
}
}

d.nextFrame(() => refocusInput())
// Ensure we focus the input
refocusInput()
reinink marked this conversation as resolved.
Show resolved Hide resolved
})

let labelledBy = useLabelledBy([id])
Expand Down Expand Up @@ -1464,7 +1508,7 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
'aria-labelledby': labelledBy,
disabled: disabled || undefined,
autoFocus,
onClick: handleClick,
onMouseDown: handleMouseDown,
onKeyDown: handleKeyDown,
},
focusProps,
Expand Down Expand Up @@ -1689,8 +1733,24 @@ function OptionFn<
/* We also want to trigger this when the position of the active item changes so that we can re-trigger the scrollIntoView */ data.activeOptionIndex,
])

let handleClick = useEvent((event: { preventDefault: Function }) => {
if (disabled || data.virtual?.disabled(value)) return event.preventDefault()
let handleMouseDown = useEvent((event: ReactMouseEvent<HTMLButtonElement>) => {
// We use the `mousedown` event because this will fire before the `focus`
// event. When we use `event.preventDefault()` here, the
// `document.activeElement` will stay on the current element.
//
// Typically that means that the `ComboboxInput` will stay focused, and thus
// the selection / cursor position will be preserved.
event.preventDefault()

// When we used the `click` event we didn't have to worry about this because
// that only fires if you use the `left` mouse button. However, now that we
// use the `mousedown` event, we do have to worry about which button is
// pressed.
if (event.button !== MouseButton.Left) {
return
}

if (disabled || data.virtual?.disabled(value)) return
reinink marked this conversation as resolved.
Show resolved Hide resolved
select()

// We want to make sure that we don't accidentally trigger the virtual keyboard.
Expand Down Expand Up @@ -1758,7 +1818,7 @@ function OptionFn<
// both single and multi-select.
'aria-selected': selected,
disabled: undefined, // Never forward the `disabled` prop
onClick: handleClick,
onMouseDown: handleMouseDown,
onFocus: handleFocus,
onPointerEnter: handleEnter,
onMouseEnter: handleEnter,
Expand Down
4 changes: 4 additions & 0 deletions packages/@headlessui-react/src/components/mouse.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export enum MouseButton {
Left = 0,
Right = 2,
}
3 changes: 3 additions & 0 deletions packages/@headlessui-react/src/hooks/use-refocusable-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ export function useRefocusableInput(ref: MutableRefObject<HTMLInputElement | nul

return useEvent(() => {
let input = ref.current

// If the input is already focused, we don't need to do anything
if (document.activeElement === input) return
if (!(input instanceof HTMLInputElement)) return
if (!input.isConnected) return
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
4 changes: 4 additions & 0 deletions packages/@headlessui-react/src/utils/disposables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ export function disposables() {
},

add(cb: () => void) {
if (_disposables.includes(cb)) {
return
}

_disposables.push(cb)
return () => {
let idx = _disposables.indexOf(cb)
Expand Down
Loading