Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix displayValue syncing when Combobox.Input is unmounted and re-mounted in different trees #2090

Merged
merged 2 commits into from Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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