Skip to content

Commit 1f2de63

Browse files
RobinMalfaitRobinMalfait
authored
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
1 parent 46754e6 commit 1f2de63

File tree

6 files changed

+30
-46
lines changed

6 files changed

+30
-46
lines changed

‎packages/@headlessui-react/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Fixed
1111

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

1415
## [1.7.5] - 2022-12-08
1516

‎packages/@headlessui-react/src/components/combobox/combobox.test.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,11 +589,11 @@ describe('Rendering', () => {
589589

590590
await click(getByText('Toggle suffix'))
591591

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

594594
await click(getComboboxButton())
595595

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

598598
await click(getComboboxOptions()[0])
599599

‎packages/@headlessui-react/src/components/combobox/combobox.tsx‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -696,31 +696,29 @@ let Input = forwardRefWithAs(function Input<
696696
let d = useDisposables()
697697

698698
// When a `displayValue` prop is given, we should use it to transform the current selected
699-
// option(s) so that the format can be chosen by developers implementing this.
700-
// This is useful if your data is an object and you just want to pick a certain property or want
701-
// to create a dynamic value like `firstName + ' ' + lastName`.
699+
// option(s) so that the format can be chosen by developers implementing this. This is useful if
700+
// your data is an object and you just want to pick a certain property or want to create a dynamic
701+
// value like `firstName + ' ' + lastName`.
702702
//
703703
// Note: This can also be used with multiple selected options, but this is a very simple transform
704-
// which should always result in a string (since we are filling in the value of the the input),
704+
// which should always result in a string (since we are filling in the value of the text input),
705705
// you don't have to use this at all, a more common UI is a "tag" based UI, which you can render
706706
// yourself using the selected option(s).
707-
let currentValue = useMemo(() => {
707+
let currentDisplayValue = (function () {
708708
if (typeof displayValue === 'function' && data.value !== undefined) {
709709
return displayValue(data.value as unknown as TType) ?? ''
710710
} else if (typeof data.value === 'string') {
711711
return data.value
712712
} else {
713713
return ''
714714
}
715-
716-
// displayValue is intentionally left out
717-
}, [data.value])
715+
})()
718716

719717
// Syncing the input value has some rules attached to it to guarantee a smooth and expected user
720718
// experience:
721719
//
722720
// - When a user is not typing in the input field, it is safe to update the input value based on
723-
// the selected option(s). See `currentValue` computation from above.
721+
// the selected option(s). See `currentDisplayValue` computation from above.
724722
// - The value can be updated when:
725723
// - The `value` is set from outside of the component
726724
// - The `value` is set when the user uses their keyboard (confirm via enter or space)
@@ -731,16 +729,16 @@ let Input = forwardRefWithAs(function Input<
731729
// - By pressing `escape`
732730
// - By clicking `outside` of the Combobox
733731
useWatch(
734-
([currentValue, state], [oldCurrentValue, oldState]) => {
732+
([currentDisplayValue, state], [oldCurrentDisplayValue, oldState]) => {
735733
if (isTyping.current) return
736734
if (!data.inputRef.current) return
737735
if (oldState === ComboboxState.Open && state === ComboboxState.Closed) {
738-
data.inputRef.current.value = currentValue
739-
} else if (currentValue !== oldCurrentValue) {
740-
data.inputRef.current.value = currentValue
736+
data.inputRef.current.value = currentDisplayValue
737+
} else if (currentDisplayValue !== oldCurrentDisplayValue) {
738+
data.inputRef.current.value = currentDisplayValue
741739
}
742740
},
743-
[currentValue, data.comboboxState]
741+
[currentDisplayValue, data.comboboxState]
744742
)
745743

746744
let isComposing = useRef(false)

‎packages/@headlessui-vue/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Fixed
1111

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

1415
## [1.7.5] - 2022-12-08
1516

‎packages/@headlessui-vue/src/components/combobox/combobox.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -629,11 +629,11 @@ describe('Rendering', () => {
629629

630630
await click(getByText('Toggle suffix'))
631631

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

634634
await click(getComboboxButton())
635635

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

638638
await click(getComboboxOptions()[0])
639639

‎packages/@headlessui-vue/src/components/combobox/combobox.ts‎

Lines changed: 12 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -661,18 +661,16 @@ export let ComboboxInput = defineComponent({
661661

662662
expose({ el: api.inputRef, $el: api.inputRef })
663663

664-
let currentValue = ref(api.value.value as unknown as string)
665-
666664
// When a `displayValue` prop is given, we should use it to transform the current selected
667-
// option(s) so that the format can be chosen by developers implementing this.
668-
// This is useful if your data is an object and you just want to pick a certain property or want
669-
// to create a dynamic value like `firstName + ' ' + lastName`.
665+
// option(s) so that the format can be chosen by developers implementing this. This is useful if
666+
// your data is an object and you just want to pick a certain property or want to create a dynamic
667+
// value like `firstName + ' ' + lastName`.
670668
//
671669
// Note: This can also be used with multiple selected options, but this is a very simple transform
672-
// which should always result in a string (since we are filling in the value of the the input),
670+
// which should always result in a string (since we are filling in the value of the text input),
673671
// you don't have to use this at all, a more common UI is a "tag" based UI, which you can render
674672
// yourself using the selected option(s).
675-
let getCurrentValue = () => {
673+
let currentDisplayValue = computed(() => {
676674
let value = api.value.value
677675
if (!dom(api.inputRef)) return ''
678676

@@ -683,28 +681,14 @@ export let ComboboxInput = defineComponent({
683681
} else {
684682
return ''
685683
}
686-
}
687-
688-
// Workaround Vue bug where watching [ref(undefined)] is not fired immediately even when value is true
689-
let __fixVueImmediateWatchBug__ = ref('')
684+
})
690685

691686
onMounted(() => {
692-
watch(
693-
[api.value, __fixVueImmediateWatchBug__],
694-
() => {
695-
currentValue.value = getCurrentValue()
696-
},
697-
{
698-
flush: 'sync',
699-
immediate: true,
700-
}
701-
)
702-
703687
// Syncing the input value has some rules attached to it to guarantee a smooth and expected user
704688
// experience:
705689
//
706690
// - When a user is not typing in the input field, it is safe to update the input value based on
707-
// the selected option(s). See `currentValue` computation from above.
691+
// the selected option(s). See `currentDisplayValue` computation from above.
708692
// - The value can be updated when:
709693
// - The `value` is set from outside of the component
710694
// - The `value` is set when the user uses their keyboard (confirm via enter or space)
@@ -715,15 +699,15 @@ export let ComboboxInput = defineComponent({
715699
// - By pressing `escape`
716700
// - By clicking `outside` of the Combobox
717701
watch(
718-
[currentValue, api.comboboxState],
719-
([currentValue, state], [oldCurrentValue, oldState]) => {
702+
[currentDisplayValue, api.comboboxState],
703+
([currentDisplayValue, state], [oldCurrentDisplayValue, oldState]) => {
720704
if (isTyping.value) return
721705
let input = dom(api.inputRef)
722706
if (!input) return
723707
if (oldState === ComboboxStates.Open && state === ComboboxStates.Closed) {
724-
input.value = currentValue
725-
} else if (currentValue !== oldCurrentValue) {
726-
input.value = currentValue
708+
input.value = currentDisplayValue
709+
} else if (currentDisplayValue !== oldCurrentDisplayValue) {
710+
input.value = currentDisplayValue
727711
}
728712
},
729713
{ immediate: true }

0 commit comments

Comments
 (0)