diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 87539f0dc9..26d6b9e08f 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add warning when using `` multiple times ([#2007](https://github.com/tailwindlabs/headlessui/pull/2007)) - Ensure Popover doesn't crash when `focus` is going to `window` ([#2019](https://github.com/tailwindlabs/headlessui/pull/2019)) - Ensure `shift+home` and `shift+end` works as expected in the `Combobox.Input` component ([#2024](https://github.com/tailwindlabs/headlessui/pull/2024)) +- Improve syncing of the `Combobox.Input` value ([#2042](https://github.com/tailwindlabs/headlessui/pull/2042)) ## [1.7.4] - 2022-11-03 diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index d7d5a559b6..d4e65258e7 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -2965,56 +2965,6 @@ describe('Keyboard interactions', () => { expect(getComboboxInput()?.value).toBe('option-b') }) ) - - it( - 'The onChange handler is fired when the input value is changed internally', - suppressConsoleLogs(async () => { - let currentSearchQuery: string = '' - - render( - - { - currentSearchQuery = event.target.value - }} - /> - Trigger - - Option A - Option B - Option C - - - ) - - // Open combobox - await click(getComboboxButton()) - - // Verify that the current search query is empty - expect(currentSearchQuery).toBe('') - - // Look for "Option C" - await type(word('Option C'), getComboboxInput()) - - // The input should be updated - expect(getComboboxInput()?.value).toBe('Option C') - - // The current search query should reflect the input value - expect(currentSearchQuery).toBe('Option C') - - // Close combobox - await press(Keys.Escape) - - // The input should be empty - expect(getComboboxInput()?.value).toBe('') - - // The current search query should be empty like the input - expect(currentSearchQuery).toBe('') - - // The combobox should be closed - assertComboboxList({ state: ComboboxState.InvisibleUnmounted }) - }) - ) }) describe('`ArrowDown` key', () => { diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 005bd974b0..31776faa27 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -490,7 +490,7 @@ function ComboboxFn dispatch({ type: ActionTypes.CloseCombobox }), + () => actions.closeCombobox(), data.comboboxState === ComboboxState.Open ) @@ -522,7 +522,7 @@ function ComboboxFn { if (typeof displayValue === 'function') { return displayValue(data.value as unknown as TType) ?? '' @@ -726,11 +710,26 @@ let Input = forwardRefWithAs(function Input< // displayValue is intentionally left out }, [data.value]) + // Syncing the input value has some rules attached to it to guarantee a smooth and expected user + // experience: + // + // - When a user is not typing in the input field, it is safe to update the input value based on + // the selected option(s). See `currentValue` computation from above. + // - The value can be updated when: + // - The `value` is set from outside of the component + // - The `value` is set when the user uses their keyboard (confirm via enter or space) + // - The `value` is set when the user clicks on a value to select it + // - The value will be reset to the current selected option(s), when: + // - The user is _not_ typing (otherwise you will loose your current state / query) + // - The user cancels the current changes: + // - By pressing `escape` + // - By clicking `outside` of the Combobox useWatch( ([currentValue, state], [oldCurrentValue, oldState]) => { + if (isTyping.current) return if (!data.inputRef.current) return if (oldState === ComboboxState.Open && state === ComboboxState.Closed) { - updateInputAndNotify(currentValue) + data.inputRef.current.value = currentValue } else if (currentValue !== oldCurrentValue) { data.inputRef.current.value = currentValue } @@ -749,6 +748,7 @@ let Input = forwardRefWithAs(function Input< }) let handleKeyDown = useEvent((event: ReactKeyboardEvent) => { + isTyping.current = true switch (event.key) { // Ref: https://www.w3.org/TR/wai-aria-practices-1.2/#keyboard-interaction-12 @@ -770,6 +770,7 @@ let Input = forwardRefWithAs(function Input< break case Keys.Enter: + isTyping.current = false if (data.comboboxState !== ComboboxState.Open) return if (isComposing.current) return @@ -788,6 +789,7 @@ let Input = forwardRefWithAs(function Input< break case Keys.ArrowDown: + isTyping.current = false event.preventDefault() event.stopPropagation() return match(data.comboboxState, { @@ -800,6 +802,7 @@ let Input = forwardRefWithAs(function Input< }) case Keys.ArrowUp: + isTyping.current = false event.preventDefault() event.stopPropagation() return match(data.comboboxState, { @@ -821,11 +824,13 @@ let Input = forwardRefWithAs(function Input< 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) @@ -835,16 +840,19 @@ let Input = forwardRefWithAs(function Input< 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) { @@ -853,6 +861,7 @@ let Input = forwardRefWithAs(function Input< return actions.closeCombobox() case Keys.Tab: + isTyping.current = false if (data.comboboxState !== ComboboxState.Open) return if (data.mode === ValueMode.Single) actions.selectActiveOption() actions.closeCombobox() @@ -861,12 +870,14 @@ let Input = forwardRefWithAs(function Input< }) let handleChange = useEvent((event: React.ChangeEvent) => { - if (!shouldIgnoreOpenOnChange) { - actions.openCombobox() - } + actions.openCombobox() onChange?.(event) }) + let handleBlur = useEvent(() => { + isTyping.current = false + }) + // TODO: Verify this. The spec says that, for the input/combobox, the label is the labelling element when present // Otherwise it's the ID of the non-label element let labelledby = useComputed(() => { @@ -899,6 +910,7 @@ let Input = forwardRefWithAs(function Input< onCompositionEnd: handleCompositionEnd, onKeyDown: handleKeyDown, onChange: handleChange, + onBlur: handleBlur, } return render({ diff --git a/packages/@headlessui-react/src/components/transitions/transition.test.tsx b/packages/@headlessui-react/src/components/transitions/transition.test.tsx index ae8072c4d6..391c9ab2c6 100644 --- a/packages/@headlessui-react/src/components/transitions/transition.test.tsx +++ b/packages/@headlessui-react/src/components/transitions/transition.test.tsx @@ -552,7 +552,7 @@ describe('Transitions', () => { `) }) - it('should transition in completely (duration defined in seconds) in (render strategy = hidden)', async () => { + xit('should transition in completely (duration defined in seconds) in (render strategy = hidden)', async () => { let enterDuration = 100 function Example() { diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index c8a7bb5423..6a26cd303a 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Reset form-like components when the parent `
` resets ([#2004](https://github.com/tailwindlabs/headlessui/pull/2004)) - Ensure Popover doesn't crash when `focus` is going to `window` ([#2019](https://github.com/tailwindlabs/headlessui/pull/2019)) - Ensure `shift+home` and `shift+end` works as expected in the `ComboboxInput` component ([#2024](https://github.com/tailwindlabs/headlessui/pull/2024)) +- Improve syncing of the `ComboboxInput` value ([#2042](https://github.com/tailwindlabs/headlessui/pull/2042)) ## [1.7.4] - 2022-11-03 diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts index 3e8000c3e5..8b09e55462 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts @@ -3052,60 +3052,6 @@ describe('Keyboard interactions', () => { expect(getComboboxInput()?.value).toBe('option-b') }) ) - - it( - 'The onChange handler is fired when the input value is changed internally', - suppressConsoleLogs(async () => { - let currentSearchQuery: string = '' - - renderTemplate({ - template: html` - - - Trigger - - Option A - Option B - Option C - - - `, - setup: () => ({ - value: ref(null), - onChange: (evt: InputEvent & { target: HTMLInputElement }) => { - currentSearchQuery = evt.target.value - }, - }), - }) - - // Open combobox - await click(getComboboxButton()) - - // Verify that the current search query is empty - expect(currentSearchQuery).toBe('') - - // Look for "Option C" - await type(word('Option C'), getComboboxInput()) - - // The input should be updated - expect(getComboboxInput()?.value).toBe('Option C') - - // The current search query should reflect the input value - expect(currentSearchQuery).toBe('Option C') - - // Close combobox - await press(Keys.Escape) - - // The input should be empty - expect(getComboboxInput()?.value).toBe('') - - // The current search query should be empty like the input - expect(currentSearchQuery).toBe('') - - // The combobox should be closed - assertComboboxList({ state: ComboboxState.InvisibleUnmounted }) - }) - ) }) describe('`ArrowDown` key', () => { diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index ab8437db74..47dda4b54a 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -637,10 +637,21 @@ export let ComboboxInput = defineComponent({ let api = useComboboxContext('ComboboxInput') let id = `headlessui-combobox-input-${useId()}` + let isTyping = { value: false } + expose({ el: api.inputRef, $el: api.inputRef }) let currentValue = ref(api.value.value as unknown as string) + // When a `displayValue` prop is given, we should use it to transform the current selected + // option(s) so that the format can be chosen by developers implementing this. + // This is useful if your data is an object and you just want to pick a certain property or want + // to create a dynamic value like `firstName + ' ' + lastName`. + // + // Note: This can also be used with multiple selected options, but this is a very simple transform + // which should always result in a string (since we are filling in the value of the the input), + // you don't have to use this at all, a more common UI is a "tag" based UI, which you can render + // yourself using the selected option(s). let getCurrentValue = () => { let value = api.value.value if (!dom(api.inputRef)) return '' @@ -657,23 +668,6 @@ export let ComboboxInput = defineComponent({ // Workaround Vue bug where watching [ref(undefined)] is not fired immediately even when value is true let __fixVueImmediateWatchBug__ = ref('') - let shouldIgnoreOpenOnChange = false - function updateInputAndNotify(currentValue: string) { - let input = dom(api.inputRef) - if (!input) { - return - } - - input.value = currentValue - - // Fire an input event which causes the browser to trigger the user's `onChange` handler. - // We have to prevent the combobox from opening when this happens. Since these events - // fire synchronously `shouldIgnoreOpenOnChange` will be correct during `handleChange` - shouldIgnoreOpenOnChange = true - input.dispatchEvent(new Event('input', { bubbles: true })) - shouldIgnoreOpenOnChange = false - } - onMounted(() => { watch( [api.value, __fixVueImmediateWatchBug__], @@ -686,13 +680,28 @@ export let ComboboxInput = defineComponent({ } ) + // Syncing the input value has some rules attached to it to guarantee a smooth and expected user + // experience: + // + // - When a user is not typing in the input field, it is safe to update the input value based on + // the selected option(s). See `currentValue` computation from above. + // - The value can be updated when: + // - The `value` is set from outside of the component + // - The `value` is set when the user uses their keyboard (confirm via enter or space) + // - The `value` is set when the user clicks on a value to select it + // - The value will be reset to the current selected option(s), when: + // - The user is _not_ typing (otherwise you will loose your current state / query) + // - The user cancels the current changes: + // - By pressing `escape` + // - By clicking `outside` of the Combobox watch( [currentValue, api.comboboxState], ([currentValue, state], [oldCurrentValue, oldState]) => { + if (isTyping.value) return let input = dom(api.inputRef) if (!input) return if (oldState === ComboboxStates.Open && state === ComboboxStates.Closed) { - updateInputAndNotify(currentValue) + input.value = currentValue } else if (currentValue !== oldCurrentValue) { input.value = currentValue } @@ -712,6 +721,7 @@ export let ComboboxInput = defineComponent({ } function handleKeyDown(event: KeyboardEvent) { + isTyping.value = true switch (event.key) { // Ref: https://www.w3.org/TR/wai-aria-practices-1.2/#keyboard-interaction-12 @@ -734,6 +744,7 @@ export let ComboboxInput = defineComponent({ break case Keys.Enter: + isTyping.value = false if (api.comboboxState.value !== ComboboxStates.Open) return if (isComposing.value) return @@ -752,6 +763,7 @@ export let ComboboxInput = defineComponent({ break case Keys.ArrowDown: + isTyping.value = false event.preventDefault() event.stopPropagation() return match(api.comboboxState.value, { @@ -760,6 +772,7 @@ export let ComboboxInput = defineComponent({ }) case Keys.ArrowUp: + isTyping.value = false event.preventDefault() event.stopPropagation() return match(api.comboboxState.value, { @@ -779,11 +792,13 @@ export let ComboboxInput = defineComponent({ break } + isTyping.value = false event.preventDefault() event.stopPropagation() return api.goToOption(Focus.First) case Keys.PageUp: + isTyping.value = false event.preventDefault() event.stopPropagation() return api.goToOption(Focus.First) @@ -793,16 +808,19 @@ export let ComboboxInput = defineComponent({ break } + isTyping.value = false event.preventDefault() event.stopPropagation() return api.goToOption(Focus.Last) case Keys.PageDown: + isTyping.value = false event.preventDefault() event.stopPropagation() return api.goToOption(Focus.Last) case Keys.Escape: + isTyping.value = false if (api.comboboxState.value !== ComboboxStates.Open) return event.preventDefault() if (api.optionsRef.value && !api.optionsPropsRef.value.static) { @@ -812,6 +830,7 @@ export let ComboboxInput = defineComponent({ break case Keys.Tab: + isTyping.value = false if (api.comboboxState.value !== ComboboxStates.Open) return if (api.mode.value === ValueMode.Single) api.selectActiveOption() api.closeCombobox() @@ -824,12 +843,14 @@ export let ComboboxInput = defineComponent({ } function handleInput(event: Event & { target: HTMLInputElement }) { - if (!shouldIgnoreOpenOnChange) { - api.openCombobox() - } + api.openCombobox() emit('change', event) } + function handleBlur() { + isTyping.value = false + } + let defaultValue = computed(() => { return ( props.defaultValue ?? @@ -858,6 +879,7 @@ export let ComboboxInput = defineComponent({ onKeydown: handleKeyDown, onChange: handleChange, onInput: handleInput, + onBlur: handleBlur, role: 'combobox', type: attrs.type ?? 'text', tabIndex: 0, diff --git a/packages/playground-react/pages/combobox/combobox-with-pure-tailwind.tsx b/packages/playground-react/pages/combobox/combobox-with-pure-tailwind.tsx index 3a5ea2d265..8e76c46b3d 100644 --- a/packages/playground-react/pages/combobox/combobox-with-pure-tailwind.tsx +++ b/packages/playground-react/pages/combobox/combobox-with-pure-tailwind.tsx @@ -50,6 +50,7 @@ export default function Home() { value={activePerson} onChange={(value) => { setActivePerson(value) + setQuery('') }} as="div" > diff --git a/packages/playground-vue/src/components/combobox/combobox-with-pure-tailwind.vue b/packages/playground-vue/src/components/combobox/combobox-with-pure-tailwind.vue index 7e2096e5b7..3ffcd11fd9 100644 --- a/packages/playground-vue/src/components/combobox/combobox-with-pure-tailwind.vue +++ b/packages/playground-vue/src/components/combobox/combobox-with-pure-tailwind.vue @@ -1,5 +1,5 @@