Skip to content

Commit

Permalink
Ensure the caret is in a consistent position when syncing the `Combob…
Browse files Browse the repository at this point in the history
…ox.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
<Combobox.Input onFocus={e => 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
  • Loading branch information
RobinMalfait committed Jun 30, 2023
1 parent 11157a8 commit 04fc6cf
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 5 deletions.
4 changes: 3 additions & 1 deletion packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<Combobox>
<Combobox.Input />
<Combobox.Button />

<Combobox.Options>
<Combobox.Option value="alice">alice</Combobox.Option>
<Combobox.Option value="bob">bob</Combobox.Option>
<Combobox.Option value="charlie">charlie</Combobox.Option>
</Combobox.Options>
</Combobox>
)
}

render(<Example />)

// 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 (
<Combobox>
<Combobox.Input
onFocus={(e) => {
e.target.select()
e.target.setSelectionRange(0, e.target.value.length)
}}
/>
<Combobox.Button />

<Combobox.Options>
<Combobox.Option value="alice">alice</Combobox.Option>
<Combobox.Option value="bob">bob</Combobox.Option>
<Combobox.Option value="charlie">charlie</Combobox.Option>
</Combobox.Options>
</Combobox>
)
}

render(<Example />)

// 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', () => {
Expand Down
29 changes: 26 additions & 3 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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]
)
Expand Down
4 changes: 3 additions & 1 deletion packages/@headlessui-vue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
67 changes: 67 additions & 0 deletions packages/@headlessui-vue/src/components/combobox/combobox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
<Combobox>
<ComboboxInput />
<ComboboxButton />
<ComboboxOptions>
<ComboboxOption value="alice">alice</ComboboxOption>
<ComboboxOption value="bob">bob</ComboboxOption>
<ComboboxOption value="charlie">charlie</ComboboxOption>
</ComboboxOptions>
</Combobox>
`,
})

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`
<Combobox>
<ComboboxInput />
<ComboboxButton />
<ComboboxOptions>
<ComboboxOption value="alice">alice</ComboboxOption>
<ComboboxOption value="bob">bob</ComboboxOption>
<ComboboxOption value="charlie">charlie</ComboboxOption>
</ComboboxOptions>
</Combobox>
`,
})

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', () => {
Expand Down
22 changes: 22 additions & 0 deletions packages/@headlessui-vue/src/components/combobox/combobox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
)
Expand Down

2 comments on commit 04fc6cf

@vercel
Copy link

@vercel vercel bot commented on 04fc6cf Jun 30, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

headlessui-vue – ./packages/playground-vue

headlessui-vue.vercel.app
headlessui-vue-tailwindlabs.vercel.app
headlessui-vue-git-main-tailwindlabs.vercel.app

@vercel
Copy link

@vercel vercel bot commented on 04fc6cf Jun 30, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

headlessui-react – ./packages/playground-react

headlessui-react-git-main-tailwindlabs.vercel.app
headlessui-react.vercel.app
headlessui-react-tailwindlabs.vercel.app

Please sign in to comment.