From 04fc6cfa0659423fb94e3fb848f367f928651896 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 30 Jun 2023 12:58:44 +0200 Subject: [PATCH] Ensure the caret is in a consistent position when syncing the `Combobox.Input` value (#2568) * ensure the caret is at the end of the input, after syncing the value This will ensure the caret is always in a consistent location once the input value has synced. We will _not_ do this while the user is typing because changing the position while typing will result in odd results. Safari already kept the caret at the end, Chrome put the caret at the end but once you synced the value once the caret was in front of the text. * update changelog * add selection guards Ensure that we only move the caret to the end, if the selection didn't change in the meantime yet. For example when you have code like this: ```js e.target.select()} /> ``` This will select all the text in the input field, if we just move the caret position without keeping this into account then we would undo this behaviour. * add tests --- packages/@headlessui-react/CHANGELOG.md | 4 +- .../src/components/combobox/combobox.test.tsx | 72 +++++++++++++++++++ .../src/components/combobox/combobox.tsx | 29 +++++++- packages/@headlessui-vue/CHANGELOG.md | 4 +- .../src/components/combobox/combobox.test.ts | 67 +++++++++++++++++ .../src/components/combobox/combobox.ts | 22 ++++++ 6 files changed, 193 insertions(+), 5 deletions(-) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index b2b6646006..3a00b99538 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- Nothing yet! +### Fixed + +- Ensure the caret is in a consistent position when syncing the `Combobox.Input` value ([#2568](https://github.com/tailwindlabs/headlessui/pull/2568)) ## [1.7.15] - 2023-06-01 diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index fe7e1e3c93..d9a18eb928 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -690,6 +690,78 @@ describe('Rendering', () => { expect(getComboboxInput()).toHaveValue('charlie - closed') }) ) + + it( + 'should move the caret to the end of the input when syncing the value', + suppressConsoleLogs(async () => { + function Example() { + return ( + + + + + + alice + bob + charlie + + + ) + } + + render() + + // Open the combobox + await click(getComboboxButton()) + + // Choose charlie + await click(getComboboxOptions()[2]) + expect(getComboboxInput()).toHaveValue('charlie') + + // Ensure the selection is in the correct position + expect(getComboboxInput()?.selectionStart).toBe('charlie'.length) + expect(getComboboxInput()?.selectionEnd).toBe('charlie'.length) + }) + ) + + // Skipped because JSDOM doesn't implement this properly: https://github.com/jsdom/jsdom/issues/3524 + xit( + 'should not move the caret to the end of the input when syncing the value if a custom selection is made', + suppressConsoleLogs(async () => { + function Example() { + return ( + + { + e.target.select() + e.target.setSelectionRange(0, e.target.value.length) + }} + /> + + + + alice + bob + charlie + + + ) + } + + render() + + // Open the combobox + await click(getComboboxButton()) + + // Choose charlie + await click(getComboboxOptions()[2]) + expect(getComboboxInput()).toHaveValue('charlie') + + // Ensure the selection is in the correct position + expect(getComboboxInput()?.selectionStart).toBe(0) + expect(getComboboxInput()?.selectionEnd).toBe('charlie'.length) + }) + ) }) describe('Combobox.Label', () => { diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index d7a508ea14..f0fbbb7a3a 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -779,12 +779,35 @@ function InputFn< useWatch( ([currentDisplayValue, state], [oldCurrentDisplayValue, oldState]) => { if (isTyping.current) return - if (!data.inputRef.current) return + let input = data.inputRef.current + + if (!input) return + if (oldState === ComboboxState.Open && state === ComboboxState.Closed) { - data.inputRef.current.value = currentDisplayValue + input.value = currentDisplayValue } else if (currentDisplayValue !== oldCurrentDisplayValue) { - data.inputRef.current.value = currentDisplayValue + input.value = currentDisplayValue } + + // Once we synced the input value, we want to make sure the cursor is at the end of the input + // field. This makes it easier to continue typing and append to the query. We will bail out if + // the user is currently typing, because we don't want to mess with the cursor position while + // typing. + requestAnimationFrame(() => { + if (isTyping.current) return + if (!input) return + + let { selectionStart, selectionEnd } = input + + // A custom selection is used, no need to move the caret + if (Math.abs((selectionEnd ?? 0) - (selectionStart ?? 0)) !== 0) return + + // A custom caret position is used, no need to move the caret + if (selectionStart !== 0) return + + // Move the caret to the end + input.setSelectionRange(input.value.length, input.value.length) + }) }, [currentDisplayValue, data.comboboxState] ) diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 03780da5e9..cb0a52fb6b 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- Nothing yet! +### Fixed + +- Ensure the caret is in a consistent position when syncing the `Combobox.Input` value ([#2568](https://github.com/tailwindlabs/headlessui/pull/2568)) ## [1.7.14] - 2023-06-01 diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts index 1306252d2f..daf4cb17a6 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts @@ -721,6 +721,73 @@ describe('Rendering', () => { expect(getComboboxInput()).toHaveValue('charlie - closed') }) ) + + it( + 'should move the caret to the end of the input when syncing the value', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + + + + + alice + bob + charlie + + + `, + }) + + await nextFrame() + + // Open the combobox + await click(getComboboxButton()) + + // Choose charlie + await click(getComboboxOptions()[2]) + expect(getComboboxInput()).toHaveValue('charlie') + + // Ensure the selection is in the correct position + expect(getComboboxInput()?.selectionStart).toBe('charlie'.length) + expect(getComboboxInput()?.selectionEnd).toBe('charlie'.length) + }) + ) + + // Skipped because JSDOM doesn't implement this properly: https://github.com/jsdom/jsdom/issues/3524 + xit( + 'should not move the caret to the end of the input when syncing the value if a custom selection is made', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + + + + + alice + bob + charlie + + + `, + }) + + await nextFrame() + + // Open the combobox + await click(getComboboxButton()) + + // Choose charlie + await click(getComboboxOptions()[2]) + expect(getComboboxInput()).toHaveValue('charlie') + + // Ensure the selection is in the correct position + expect(getComboboxInput()?.selectionStart).toBe(0) + expect(getComboboxInput()?.selectionEnd).toBe('charlie'.length) + }) + ) }) describe('ComboboxLabel', () => { diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index 7cf476ee3b..596822c5ab 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -732,12 +732,34 @@ export let ComboboxInput = defineComponent({ ([currentDisplayValue, state], [oldCurrentDisplayValue, oldState]) => { if (isTyping.value) return let input = dom(api.inputRef) + if (!input) return + if (oldState === ComboboxStates.Open && state === ComboboxStates.Closed) { input.value = currentDisplayValue } else if (currentDisplayValue !== oldCurrentDisplayValue) { input.value = currentDisplayValue } + + // Once we synced the input value, we want to make sure the cursor is at the end of the + // input field. This makes it easier to continue typing and append to the query. We will + // bail out if the user is currently typing, because we don't want to mess with the cursor + // position while typing. + requestAnimationFrame(() => { + if (isTyping.value) return + if (!input) return + + let { selectionStart, selectionEnd } = input + + // A custom selection is used, no need to move the caret + if (Math.abs((selectionEnd ?? 0) - (selectionStart ?? 0)) !== 0) return + + // A custom caret position is used, no need to move the caret + if (selectionStart !== 0) return + + // Move the caret to the end + input.setSelectionRange(input.value.length, input.value.length) + }) }, { immediate: true } )