From 47bae273b3ee9365ff72997caf7ab3ffee33ed53 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 31 May 2024 15:18:18 +0200 Subject: [PATCH 1/4] track `isTyping` in state While you are typing, we should not sync the value with the `` because otherwise it would override your changes. The moment you close the Combobox (by selecting an option, clicking outside, pressing escape or tabbing away) we can mark the component as not typing anymore. Once you are not typing anymore, then we can re-sync the input with the given value. --- .../src/components/combobox/combobox.tsx | 54 +++++++++++-------- .../src/components/combobox/combobox.ts | 1 + 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 638f2e877..9bacd461b 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -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' @@ -112,6 +111,8 @@ interface StateDefinition { activeOptionIndex: number | null activationTrigger: ActivationTrigger + isTyping: boolean + __demoMode: boolean } @@ -120,6 +121,7 @@ enum ActionTypes { CloseCombobox, GoToOption, + SetTyping, RegisterOption, UnregisterOption, @@ -170,6 +172,7 @@ type Actions = idx: number trigger?: ActivationTrigger } + | { type: ActionTypes.SetTyping; isTyping: boolean } | { type: ActionTypes.GoToOption focus: Exclude @@ -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 @@ -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 ( @@ -268,6 +277,7 @@ let reducers: { ...state, activeOptionIndex, activationTrigger, + isTyping: false, __demoMode: false, } } @@ -308,6 +318,7 @@ let reducers: { return { ...state, ...adjustedState, + isTyping: false, activeOptionIndex, activationTrigger, __demoMode: false, @@ -413,6 +424,7 @@ let ComboboxActionsContext = createContext<{ registerOption(id: string, dataRef: ComboboxOptionDataRef): () => 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 @@ -662,6 +674,7 @@ function ComboboxFn false) } @@ -793,6 +806,8 @@ function ComboboxFn { if (data.activeOptionIndex === null) return + actions.setIsTyping(false) + if (data.virtual) { onChange(data.virtual.options[data.activeOptionIndex]) } else { @@ -816,6 +831,10 @@ function ComboboxFn { + dispatch({ type: ActionTypes.SetTyping, isTyping }) + }) + let goToOption = useEvent((focus, idx, trigger) => { defaultToFirstOption.current = false @@ -875,6 +894,7 @@ function ComboboxFn { @@ -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 @@ -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 @@ -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 @@ -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 @@ -1128,18 +1146,13 @@ function InputFn< }) }) - let debounce = useFrameDebounce() let handleKeyDown = useEvent((event: ReactKeyboardEvent) => { - 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 @@ -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, { @@ -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) @@ -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) { @@ -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() @@ -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 @@ -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) diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index f2d3ef627..5bafc677e 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -1068,6 +1068,7 @@ export let ComboboxInput = defineComponent({ function handleKeyDown(event: KeyboardEvent) { isTyping.value = true debounce(() => { + if (isComposing.value) return isTyping.value = false }) From 174872df5b43d8d855ed14639937c1c59707d778 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 31 May 2024 17:03:02 +0200 Subject: [PATCH 2/4] remove unused `useFrameDebounce` hook --- .../src/hooks/use-frame-debounce.ts | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 packages/@headlessui-react/src/hooks/use-frame-debounce.ts diff --git a/packages/@headlessui-react/src/hooks/use-frame-debounce.ts b/packages/@headlessui-react/src/hooks/use-frame-debounce.ts deleted file mode 100644 index 94c085340..000000000 --- a/packages/@headlessui-react/src/hooks/use-frame-debounce.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { useDisposables } from './use-disposables' -import { useEvent } from './use-event' - -/** - * Schedule some task in the next frame. - * - * - If you call the returned function multiple times, only the last task will - * be executed. - * - If the component is unmounted, the task will be cancelled. - */ -export function useFrameDebounce() { - let d = useDisposables() - - return useEvent((cb: () => void) => { - d.dispose() - d.nextFrame(cb) - }) -} From 4c8827146476825c612339dda765e70986f8d403 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 31 May 2024 22:40:48 +0200 Subject: [PATCH 3/4] require `isTyping` boolean --- .../@headlessui-react/src/components/combobox/combobox.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 9bacd461b..8482c8a35 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -424,7 +424,7 @@ let ComboboxActionsContext = createContext<{ registerOption(id: string, dataRef: ComboboxOptionDataRef): () => void goToOption(focus: Focus.Specific, idx: number, trigger?: ActivationTrigger): void goToOption(focus: Focus, idx?: number, trigger?: ActivationTrigger): void - setIsTyping(isTyping?: boolean): void + setIsTyping(isTyping: boolean): void selectActiveOption(): void setActivationTrigger(trigger: ActivationTrigger): void onChange(value: unknown): void @@ -831,7 +831,7 @@ function ComboboxFn { + let setIsTyping = useEvent((isTyping: boolean) => { dispatch({ type: ActionTypes.SetTyping, isTyping }) }) From 74eea6a8fbba2d7c9672e8fd75b85621d8b74110 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 31 May 2024 22:37:04 +0200 Subject: [PATCH 4/4] update changelog --- packages/@headlessui-react/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 25ae52a5a..c115bd045 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Prevent focus on `` 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