From 1f2de63b402182dc002ec537621b3f04e2c9529c Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 12 Dec 2022 19:16:46 +0100 Subject: [PATCH] Fix `displayValue` syncing when `Combobox.Input` is unmounted and re-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 --- packages/@headlessui-react/CHANGELOG.md | 1 + .../src/components/combobox/combobox.test.tsx | 4 +- .../src/components/combobox/combobox.tsx | 26 ++++++------ packages/@headlessui-vue/CHANGELOG.md | 1 + .../src/components/combobox/combobox.test.ts | 4 +- .../src/components/combobox/combobox.ts | 40 ++++++------------- 6 files changed, 30 insertions(+), 46 deletions(-) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 456689fd55..813d61b29e 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -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 diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index 9c5561d896..a8bcbfa0e6 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -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]) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index ba84793209..f586adda1a 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -696,15 +696,15 @@ 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') { @@ -712,15 +712,13 @@ let Input = forwardRefWithAs(function Input< } 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) @@ -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) diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index ecd90681cb..52c1c24616 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -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 diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts index 1af8cdc24f..13b8febdd3 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts @@ -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]) diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index ac9cc2f629..cb0ffd49f0 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -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 '' @@ -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) @@ -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 }