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 use of undefined and displayValue in Combobox #1865

Merged
merged 2 commits into from Sep 19, 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
Expand Up @@ -460,6 +460,46 @@ describe('Rendering', () => {
})
)

it(
'selecting an option puts the display value into Combobox.Input when displayValue is provided (when value is undefined)',
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState(undefined)

return (
<Combobox value={value} onChange={setValue}>
<Combobox.Input
onChange={NOOP}
displayValue={(str?: string) => str?.toUpperCase() ?? ''}
/>
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value="a">Option A</Combobox.Option>
<Combobox.Option value="b">Option B</Combobox.Option>
<Combobox.Option value="c">Option C</Combobox.Option>
</Combobox.Options>
</Combobox>
)
}

render(<Example />)

// Focus the input
await focus(getComboboxInput())

// Type in it
await type(word('A'), getComboboxInput())

// Stop typing (and clear the input)
await press(Keys.Escape, getComboboxInput())

// Focus the body (so the input loses focus)
await focus(document.body)

expect(getComboboxInput()).toHaveValue('')
})
)

it(
'conditionally rendering the input should allow changing the display value',
suppressConsoleLogs(async () => {
Expand Down
4 changes: 3 additions & 1 deletion packages/@headlessui-vue/CHANGELOG.md
Expand Up @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Nothing yet!
### Fixed

- Call `displayValue` with a v-model of `ref(undefined)` on `ComboboxInput` ([#1865](https://github.com/tailwindlabs/headlessui/pull/1865))

## [1.7.2] - 2022-09-15

Expand Down
37 changes: 37 additions & 0 deletions packages/@headlessui-vue/src/components/combobox/combobox.test.ts
Expand Up @@ -493,6 +493,43 @@ describe('Rendering', () => {
})
)

// This really is a bug in Vue but we have a workaround for it
it(
'selecting an option puts the display value into Combobox.Input when displayValue is provided (when v-model is undefined)',
suppressConsoleLogs(async () => {
let Example = defineComponent({
template: html`
<Combobox v-model="value">
<ComboboxInput :displayValue="(str) => str?.toUpperCase() ?? ''" />
<ComboboxButton>Trigger</ComboboxButton>
<ComboboxOptions>
<ComboboxOption value="a">Option A</ComboboxOption>
<ComboboxOption value="b">Option B</ComboboxOption>
<ComboboxOption value="c">Option C</ComboboxOption>
</ComboboxOptions>
</Combobox>
`,
setup: () => ({ value: ref(undefined) }),
})

renderTemplate(Example)

// Focus the input
await focus(getComboboxInput())

// Type in it
await type(word('A'), getComboboxInput())

// Stop typing (and clear the input)
await press(Keys.Escape, getComboboxInput())

// Focus the body (so the input loses focus)
await focus(document.body)

expect(getComboboxInput()).toHaveValue('')
})
)

it('conditionally rendering the input should allow changing the display value', async () => {
let Example = defineComponent({
template: html`
Expand Down
17 changes: 13 additions & 4 deletions packages/@headlessui-vue/src/components/combobox/combobox.ts
Expand Up @@ -629,11 +629,20 @@ export let ComboboxInput = defineComponent({
}
}

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

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

watch(
[currentValue, api.comboboxState],
Expand Down