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

Ensure ComboboxInput does not sync while you are still typing #3259

Merged
merged 4 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Prevent focus on `<Checkbox />` when it is `disabled` ([#3251](https://github.com/tailwindlabs/headlessui/pull/3251))
- Fix visual jitter in `Combobox` component when using native scrollbar ([#3190](https://github.com/tailwindlabs/headlessui/pull/3190))
- Use `useId` instead of React internals (for React 19 compatibility) ([#3254](https://github.com/tailwindlabs/headlessui/pull/3254))
- Ensure `ComboboxInput` does not sync with current value while typing ([#3259](https://github.com/tailwindlabs/headlessui/pull/3259))

## [2.0.4] - 2024-05-25

Expand Down
54 changes: 31 additions & 23 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import { useDefaultValue } from '../../hooks/use-default-value'
import { useDisposables } from '../../hooks/use-disposables'
import { useElementSize } from '../../hooks/use-element-size'
import { useEvent } from '../../hooks/use-event'
import { useFrameDebounce } from '../../hooks/use-frame-debounce'
import { useId } from '../../hooks/use-id'
import { useInertOthers } from '../../hooks/use-inert-others'
import { useIsoMorphicEffect } from '../../hooks/use-iso-morphic-effect'
Expand Down Expand Up @@ -112,6 +111,8 @@ interface StateDefinition<T> {
activeOptionIndex: number | null
activationTrigger: ActivationTrigger

isTyping: boolean

__demoMode: boolean
}

Expand All @@ -120,6 +121,7 @@ enum ActionTypes {
CloseCombobox,

GoToOption,
SetTyping,

RegisterOption,
UnregisterOption,
Expand Down Expand Up @@ -170,6 +172,7 @@ type Actions<T> =
idx: number
trigger?: ActivationTrigger
}
| { type: ActionTypes.SetTyping; isTyping: boolean }
| {
type: ActionTypes.GoToOption
focus: Exclude<Focus, Focus.Specific>
Expand Down Expand Up @@ -202,6 +205,8 @@ let reducers: {
activeOptionIndex: null,
comboboxState: ComboboxState.Closed,

isTyping: false,

// Clear the last known activation trigger
// This is because if a user interacts with the combobox using a mouse
// resulting in it closing we might incorrectly handle the next interaction
Expand Down Expand Up @@ -230,6 +235,10 @@ let reducers: {

return { ...state, comboboxState: ComboboxState.Open, __demoMode: false }
},
[ActionTypes.SetTyping](state, action) {
if (state.isTyping === action.isTyping) return state
return { ...state, isTyping: action.isTyping }
},
[ActionTypes.GoToOption](state, action) {
if (state.dataRef.current?.disabled) return state
if (
Expand Down Expand Up @@ -268,6 +277,7 @@ let reducers: {
...state,
activeOptionIndex,
activationTrigger,
isTyping: false,
__demoMode: false,
}
}
Expand Down Expand Up @@ -308,6 +318,7 @@ let reducers: {
return {
...state,
...adjustedState,
isTyping: false,
activeOptionIndex,
activationTrigger,
__demoMode: false,
Expand Down Expand Up @@ -413,6 +424,7 @@ let ComboboxActionsContext = createContext<{
registerOption(id: string, dataRef: ComboboxOptionDataRef<unknown>): () => void
goToOption(focus: Focus.Specific, idx: number, trigger?: ActivationTrigger): void
goToOption(focus: Focus, idx?: number, trigger?: ActivationTrigger): void
setIsTyping(isTyping: boolean): void
selectActiveOption(): void
setActivationTrigger(trigger: ActivationTrigger): void
onChange(value: unknown): void
Expand Down Expand Up @@ -662,6 +674,7 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
let [state, dispatch] = useReducer(stateReducer, {
dataRef: createRef(),
comboboxState: __demoMode ? ComboboxState.Open : ComboboxState.Closed,
isTyping: false,
options: [],
virtual: virtual
? { options: virtual.options, disabled: virtual.disabled ?? (() => false) }
Expand Down Expand Up @@ -793,6 +806,8 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
let selectActiveOption = useEvent(() => {
if (data.activeOptionIndex === null) return

actions.setIsTyping(false)

if (data.virtual) {
onChange(data.virtual.options[data.activeOptionIndex])
} else {
Expand All @@ -816,6 +831,10 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
onClose?.()
})

let setIsTyping = useEvent((isTyping: boolean) => {
dispatch({ type: ActionTypes.SetTyping, isTyping })
})

let goToOption = useEvent((focus, idx, trigger) => {
defaultToFirstOption.current = false

Expand Down Expand Up @@ -875,6 +894,7 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
onChange,
registerOption,
goToOption,
setIsTyping,
closeCombobox,
openCombobox,
setActivationTrigger,
Expand Down Expand Up @@ -995,8 +1015,6 @@ function InputFn<
let inputRef = useSyncRefs(data.inputRef, ref, useFloatingReference())
let ownerDocument = useOwnerDocument(data.inputRef)

let isTyping = useRef(false)

let d = useDisposables()

let clear = useEvent(() => {
Expand Down Expand Up @@ -1044,7 +1062,7 @@ function InputFn<
([currentDisplayValue, state], [oldCurrentDisplayValue, oldState]) => {
// When the user is typing, we want to not touch the `input` at all. Especially when they are
// using an IME, we don't want to mess with the input at all.
if (isTyping.current) return
if (data.isTyping) return

let input = data.inputRef.current
if (!input) return
Expand All @@ -1060,7 +1078,7 @@ function InputFn<
// the user is currently typing, because we don't want to mess with the cursor position while
// typing.
requestAnimationFrame(() => {
if (isTyping.current) return
if (data.isTyping) return
if (!input) return

// Bail when the input is not the currently focused element. When it is not the focused
Expand All @@ -1080,7 +1098,7 @@ function InputFn<
input.setSelectionRange(input.value.length, input.value.length)
})
},
[currentDisplayValue, data.comboboxState, ownerDocument]
[currentDisplayValue, data.comboboxState, ownerDocument, data.isTyping]
)

// Trick VoiceOver in behaving a little bit better. Manually "resetting" the input makes VoiceOver
Expand All @@ -1094,7 +1112,7 @@ function InputFn<
if (newState === ComboboxState.Open && oldState === ComboboxState.Closed) {
// When the user is typing, we want to not touch the `input` at all. Especially when they are
// using an IME, we don't want to mess with the input at all.
if (isTyping.current) return
if (data.isTyping) return

let input = data.inputRef.current
if (!input) return
Expand Down Expand Up @@ -1128,18 +1146,13 @@ function InputFn<
})
})

let debounce = useFrameDebounce()
let handleKeyDown = useEvent((event: ReactKeyboardEvent<HTMLInputElement>) => {
isTyping.current = true
debounce(() => {
isTyping.current = false
})
actions.setIsTyping(true)

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

case Keys.Enter:
isTyping.current = false
if (data.comboboxState !== ComboboxState.Open) return

// When the user is still in the middle of composing by using an IME, then we don't want to
Expand All @@ -1162,16 +1175,15 @@ function InputFn<
break

case Keys.ArrowDown:
isTyping.current = false
event.preventDefault()
event.stopPropagation()

return match(data.comboboxState, {
[ComboboxState.Open]: () => actions.goToOption(Focus.Next),
[ComboboxState.Closed]: () => actions.openCombobox(),
})

case Keys.ArrowUp:
isTyping.current = false
event.preventDefault()
event.stopPropagation()
return match(data.comboboxState, {
Expand All @@ -1191,13 +1203,11 @@ function InputFn<
break
}

isTyping.current = false
event.preventDefault()
event.stopPropagation()
return actions.goToOption(Focus.First)

case Keys.PageUp:
isTyping.current = false
event.preventDefault()
event.stopPropagation()
return actions.goToOption(Focus.First)
Expand All @@ -1207,19 +1217,16 @@ function InputFn<
break
}

isTyping.current = false
event.preventDefault()
event.stopPropagation()
return actions.goToOption(Focus.Last)

case Keys.PageDown:
isTyping.current = false
event.preventDefault()
event.stopPropagation()
return actions.goToOption(Focus.Last)

case Keys.Escape:
isTyping.current = false
if (data.comboboxState !== ComboboxState.Open) return
event.preventDefault()
if (data.optionsRef.current && !data.optionsPropsRef.current.static) {
Expand All @@ -1240,7 +1247,6 @@ function InputFn<
return actions.closeCombobox()

case Keys.Tab:
isTyping.current = false
if (data.comboboxState !== ComboboxState.Open) return
if (data.mode === ValueMode.Single && data.activationTrigger !== ActivationTrigger.Focus) {
actions.selectActiveOption()
Expand Down Expand Up @@ -1275,7 +1281,6 @@ function InputFn<
let handleBlur = useEvent((event: ReactFocusEvent) => {
let relatedTarget =
(event.relatedTarget as HTMLElement) ?? history.find((x) => x !== event.currentTarget)
isTyping.current = false

// Focus is moved into the list, we don't want to close yet.
if (data.optionsRef.current?.contains(relatedTarget)) return
Expand Down Expand Up @@ -1819,7 +1824,10 @@ function OptionFn<
virtualizer ? virtualizer.measureElement : null
)

let select = useEvent(() => actions.onChange(value))
let select = useEvent(() => {
actions.setIsTyping(false)
actions.onChange(value)
})
useIsoMorphicEffect(() => actions.registerOption(id, bag), [bag, id])

let enableScrollIntoView = useRef(data.virtual || data.__demoMode ? false : true)
Expand Down
18 changes: 0 additions & 18 deletions packages/@headlessui-react/src/hooks/use-frame-debounce.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,7 @@ export let ComboboxInput = defineComponent({
function handleKeyDown(event: KeyboardEvent) {
isTyping.value = true
debounce(() => {
if (isComposing.value) return
isTyping.value = false
})

Expand Down
Loading