From b380d03ccc2d5502de6430d1644be7b9aeb1199c Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Fri, 28 Jul 2023 11:46:33 -0400 Subject: [PATCH] Use correct value when resetting `` and `` (#2626) * Fix bug with non-controlled, multiple combobox in Vue It thought it was always controlled which broke things * Use correct value when resetting `` and `` * Update changelog --- packages/@headlessui-react/CHANGELOG.md | 4 +- .../src/components/combobox/combobox.test.tsx | 44 ++++++++++++++++++ .../src/components/combobox/combobox.tsx | 4 +- .../src/components/listbox/listbox.test.tsx | 44 ++++++++++++++++++ .../src/components/listbox/listbox.tsx | 4 +- packages/@headlessui-vue/CHANGELOG.md | 5 +- .../src/components/combobox/combobox.test.ts | 46 +++++++++++++++++++ .../src/components/combobox/combobox.ts | 20 ++++---- .../src/components/listbox/listbox.test.tsx | 44 ++++++++++++++++++ .../src/components/listbox/listbox.ts | 27 +++++++---- 10 files changed, 217 insertions(+), 25 deletions(-) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 6240d00c5f..4eb2e44888 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- Nothing yet! +### Fixed + +- Use correct value when resetting `` and `` ([#2626](https://github.com/tailwindlabs/headlessui/pull/2626)) ## [1.7.16] - 2023-07-27 diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index 3642b6edb5..eb57a61e5c 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -1448,6 +1448,50 @@ describe('Rendering', () => { assertActiveComboboxOption(getComboboxOptions()[1]) }) + it('should be possible to reset to the default value in multiple mode', async () => { + let handleSubmission = jest.fn() + let data = ['alice', 'bob', 'charlie'] + + render( +
{ + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }} + > + + {({ value }) => value.join(', ') || 'Trigger'} + + {data.map((person) => ( + + {person} + + ))} + + + + +
+ ) + + await click(document.getElementById('submit')) + + // Bob is the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ + 'assignee[0]': 'bob', + }) + + await click(document.getElementById('reset')) + await click(document.getElementById('submit')) + + // Bob is still the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ + 'assignee[0]': 'bob', + }) + }) + it('should still call the onChange listeners when choosing new values', async () => { let handleChange = jest.fn() diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index c7839d33fb..4181ff26c2 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -642,9 +642,9 @@ function ComboboxFn { - onChange(defaultValue) + theirOnChange?.(defaultValue) }) - }, [form, onChange /* Explicitly ignoring `defaultValue` */]) + }, [form, theirOnChange /* Explicitly ignoring `defaultValue` */]) return ( diff --git a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx index 9eca6f2119..521172f336 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx @@ -1125,6 +1125,50 @@ describe('Rendering', () => { assertActiveListboxOption(getListboxOptions()[1]) }) + it('should be possible to reset to the default value in multiple mode', async () => { + let handleSubmission = jest.fn() + let data = ['alice', 'bob', 'charlie'] + + render( +
{ + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }} + > + + {({ value }) => value.join(', ') || 'Trigger'} + + {data.map((person) => ( + + {person} + + ))} + + + + +
+ ) + + await click(document.getElementById('submit')) + + // Bob is the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ + 'assignee[0]': 'bob', + }) + + await click(document.getElementById('reset')) + await click(document.getElementById('submit')) + + // Bob is still the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ + 'assignee[0]': 'bob', + }) + }) + it('should still call the onChange listeners when choosing new values', async () => { let handleChange = jest.fn() diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index 8b11271b79..c633ad5cd9 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -537,9 +537,9 @@ function ListboxFn< if (defaultValue === undefined) return d.addEventListener(form.current, 'reset', () => { - onChange(defaultValue) + theirOnChange?.(defaultValue) }) - }, [form, onChange /* Explicitly ignoring `defaultValue` */]) + }, [form, theirOnChange /* Explicitly ignoring `defaultValue` */]) return ( diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 575e02896f..faf9e47560 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -7,7 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- Nothing yet! +### Fixed + +- Fix form elements for uncontrolled `` and `` ([#2626](https://github.com/tailwindlabs/headlessui/pull/2626)) +- Use correct value when resetting `` and `` ([#2626](https://github.com/tailwindlabs/headlessui/pull/2626)) ## [1.7.15] - 2023-07-27 diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts index 6d294d3957..a66d0930a9 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts @@ -1553,6 +1553,52 @@ describe('Rendering', () => { }) ) + it('should be possible to reset to the default value in multiple mode', async () => { + let data = ['alice', 'bob', 'charlie'] + let handleSubmission = jest.fn() + + renderTemplate({ + template: html` +
+ + {{ value.join(', ') || 'Trigger' }} + + + {{ person }} + + + + + +
+ `, + setup: () => ({ + data, + handleSubmit(e: SubmitEvent) { + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }, + }), + }) + + await click(document.getElementById('submit')) + + // Bob is the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ + 'assignee[0]': 'bob', + }) + + await click(document.getElementById('reset')) + await click(document.getElementById('submit')) + + // Bob is still the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ + 'assignee[0]': 'bob', + }) + }) + it('should still call the onChange listeners when choosing new values', async () => { let handleChange = jest.fn() diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index 8b4b6e6a0f..88e0fa2ff4 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -189,19 +189,21 @@ export let Combobox = defineComponent({ let mode = computed(() => (props.multiple ? ValueMode.Multi : ValueMode.Single)) let nullable = computed(() => props.nullable) - let [value, theirOnChange] = useControllable( - computed(() => - props.modelValue === undefined - ? match(mode.value, { - [ValueMode.Multi]: [], - [ValueMode.Single]: undefined, - }) - : props.modelValue - ), + let [directValue, theirOnChange] = useControllable( + computed(() => props.modelValue), (value: unknown) => emit('update:modelValue', value), computed(() => props.defaultValue) ) + let value = computed(() => + directValue.value === undefined + ? match(mode.value, { + [ValueMode.Multi]: [], + [ValueMode.Single]: undefined, + }) + : directValue.value + ) + let goToOptionRaf: ReturnType | null = null let orderOptionsRaf: ReturnType | null = null diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx index 56a8ab8b0a..9052ffd35a 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx @@ -1236,6 +1236,50 @@ describe('Rendering', () => { }) ) + it('should be possible to reset to the default value in multiple mode', async () => { + let data = ['alice', 'bob', 'charlie'] + let handleSubmission = jest.fn() + + renderTemplate({ + template: html` +
+ + {{ value.join(', ') || 'Trigger' }} + + + {{ person }} + + + + + +
+ `, + setup: () => ({ + data, + handleSubmit(e: SubmitEvent) { + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }, + }), + }) + + await click(document.getElementById('submit')) + + // Bob is the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ + 'assignee[0]': 'bob', + }) + + await click(document.getElementById('reset')) + await click(document.getElementById('submit')) + + // Bob is still the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ + 'assignee[0]': 'bob', + }) + }) + it('should still call the onChange listeners when choosing new values', async () => { let handleChange = jest.fn() diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.ts b/packages/@headlessui-vue/src/components/listbox/listbox.ts index 89283dcd85..34d9a8f480 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.ts +++ b/packages/@headlessui-vue/src/components/listbox/listbox.ts @@ -181,19 +181,22 @@ export let Listbox = defineComponent({ } let mode = computed(() => (props.multiple ? ValueMode.Multi : ValueMode.Single)) - let [value, theirOnChange] = useControllable( - computed(() => - props.modelValue === undefined - ? match(mode.value, { - [ValueMode.Multi]: [], - [ValueMode.Single]: undefined, - }) - : props.modelValue - ), + + let [directValue, theirOnChange] = useControllable( + computed(() => props.modelValue), (value: unknown) => emit('update:modelValue', value), computed(() => props.defaultValue) ) + let value = computed(() => + directValue.value === undefined + ? match(mode.value, { + [ValueMode.Multi]: [], + [ValueMode.Single]: undefined, + }) + : directValue.value + ) + let api = { listboxState, value, @@ -300,6 +303,10 @@ export let Listbox = defineComponent({ activeOptionIndex.value = adjustedState.activeOptionIndex activationTrigger.value = ActivationTrigger.Other }, + theirOnChange(value: unknown) { + if (props.disabled) return + theirOnChange(value) + }, select(value: unknown) { if (props.disabled) return theirOnChange( @@ -357,7 +364,7 @@ export let Listbox = defineComponent({ if (props.defaultValue === undefined) return function handle() { - api.select(props.defaultValue) + api.theirOnChange(props.defaultValue) } form.value.addEventListener('reset', handle)