Skip to content

Commit

Permalink
Fix displayValue syncing when Combobox.Input is unmounted and re-…
Browse files Browse the repository at this point in the history
…mounted in different trees (#2090)

* simplify `currentDisplayValue` calculation

Always calculate the currentDisplayValue, and only apply it if the user
is not typing. In all other cases it can be applied (e.g.: when the
value changes from the outside, inside or on reset)

* update changelog
  • Loading branch information
RobinMalfait committed Dec 12, 2022
1 parent 46754e6 commit 1f2de63
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 46 deletions.
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- Fix regression where `displayValue` crashes ([#2087](https://github.com/tailwindlabs/headlessui/pull/2087))
- Fix `displayValue` syncing when `Combobox.Input` is unmounted and re-mounted in different trees ([#2090](https://github.com/tailwindlabs/headlessui/pull/2090))

## [1.7.5] - 2022-12-08

Expand Down
Expand Up @@ -589,11 +589,11 @@ describe('Rendering', () => {

await click(getByText('Toggle suffix'))

expect(getComboboxInput()).toHaveValue('B no suffix') // No re-sync yet
expect(getComboboxInput()).toHaveValue('B with suffix')

await click(getComboboxButton())

expect(getComboboxInput()).toHaveValue('B no suffix') // No re-sync yet
expect(getComboboxInput()).toHaveValue('B with suffix')

await click(getComboboxOptions()[0])

Expand Down
26 changes: 12 additions & 14 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Expand Up @@ -696,31 +696,29 @@ let Input = forwardRefWithAs(function Input<
let d = useDisposables()

// 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`.
// 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),
// which should always result in a string (since we are filling in the value of the text 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 currentValue = useMemo(() => {
let currentDisplayValue = (function () {
if (typeof displayValue === 'function' && data.value !== undefined) {
return displayValue(data.value as unknown as TType) ?? ''
} else if (typeof data.value === 'string') {
return data.value
} else {
return ''
}

// 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 selected option(s). See `currentDisplayValue` 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)
Expand All @@ -731,16 +729,16 @@ let Input = forwardRefWithAs(function Input<
// - By pressing `escape`
// - By clicking `outside` of the Combobox
useWatch(
([currentValue, state], [oldCurrentValue, oldState]) => {
([currentDisplayValue, state], [oldCurrentDisplayValue, oldState]) => {
if (isTyping.current) return
if (!data.inputRef.current) return
if (oldState === ComboboxState.Open && state === ComboboxState.Closed) {
data.inputRef.current.value = currentValue
} else if (currentValue !== oldCurrentValue) {
data.inputRef.current.value = currentValue
data.inputRef.current.value = currentDisplayValue
} else if (currentDisplayValue !== oldCurrentDisplayValue) {
data.inputRef.current.value = currentDisplayValue
}
},
[currentValue, data.comboboxState]
[currentDisplayValue, data.comboboxState]
)

let isComposing = useRef(false)
Expand Down
1 change: 1 addition & 0 deletions packages/@headlessui-vue/CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- Fix regression where `displayValue` crashes ([#2087](https://github.com/tailwindlabs/headlessui/pull/2087))
- Fix `displayValue` syncing when `Combobox.Input` is unmounted and re-mounted in different trees ([#2090](https://github.com/tailwindlabs/headlessui/pull/2090))

## [1.7.5] - 2022-12-08

Expand Down
Expand Up @@ -629,11 +629,11 @@ describe('Rendering', () => {

await click(getByText('Toggle suffix'))

expect(getComboboxInput()).toHaveValue('B no suffix') // No re-sync yet
expect(getComboboxInput()).toHaveValue('B with suffix')

await click(getComboboxButton())

expect(getComboboxInput()).toHaveValue('B no suffix') // No re-sync yet
expect(getComboboxInput()).toHaveValue('B with suffix')

await click(getComboboxOptions()[0])

Expand Down
40 changes: 12 additions & 28 deletions packages/@headlessui-vue/src/components/combobox/combobox.ts
Expand Up @@ -661,18 +661,16 @@ export let ComboboxInput = defineComponent({

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`.
// 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),
// which should always result in a string (since we are filling in the value of the text 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 currentDisplayValue = computed(() => {
let value = api.value.value
if (!dom(api.inputRef)) return ''

Expand All @@ -683,28 +681,14 @@ export let ComboboxInput = defineComponent({
} else {
return ''
}
}

// Workaround Vue bug where watching [ref(undefined)] is not fired immediately even when value is true
let __fixVueImmediateWatchBug__ = ref('')
})

onMounted(() => {
watch(
[api.value, __fixVueImmediateWatchBug__],
() => {
currentValue.value = getCurrentValue()
},
{
flush: 'sync',
immediate: true,
}
)

// 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 selected option(s). See `currentDisplayValue` 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)
Expand All @@ -715,15 +699,15 @@ export let ComboboxInput = defineComponent({
// - By pressing `escape`
// - By clicking `outside` of the Combobox
watch(
[currentValue, api.comboboxState],
([currentValue, state], [oldCurrentValue, oldState]) => {
[currentDisplayValue, api.comboboxState],
([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 = currentValue
} else if (currentValue !== oldCurrentValue) {
input.value = currentValue
input.value = currentDisplayValue
} else if (currentDisplayValue !== oldCurrentDisplayValue) {
input.value = currentDisplayValue
}
},
{ immediate: true }
Expand Down

0 comments on commit 1f2de63

Please sign in to comment.