Use correct value when resetting <Listbox multiple> and `<Combobox …

…multiple>` (#2626) * Fix bug with non-controlled, multiple combobox in Vue It thought it was always controlled which broke things * Use correct value when resetting `<Listbox multiple>` and `<Combobox multiple>` * Update changelog
tailwindlabs · Jul 28, 2023 · b380d03 · b380d03 · vercel · Jul 28, 2023
1 parent 9b42daf
commit b380d03
Show file tree

Hide file tree

Showing 10 changed files with 217 additions and 25 deletions.
diff --git 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 `<Listbox multiple>` and `<Combobox multiple>` ([#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
@@ -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(
+        <form
+          onSubmit={(e) => {
+            e.preventDefault()
+            handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement)))
+          }}
+        >
+          <Combobox name="assignee" defaultValue={['bob'] as string[]} multiple>
+            <Combobox.Button>{({ value }) => value.join(', ') || 'Trigger'}</Combobox.Button>
+            <Combobox.Options>
+              {data.map((person) => (
+                <Combobox.Option key={person} value={person}>
+                  {person}
+                </Combobox.Option>
+              ))}
+            </Combobox.Options>
+          </Combobox>
+          <button id="submit">submit</button>
+          <button type="reset" id="reset">
+            reset
+          </button>
+        </form>
+      )
+
+      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
@@ -642,9 +642,9 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
     if (defaultValue === undefined) return
 
     d.addEventListener(form.current, 'reset', () => {
-      onChange(defaultValue)
+      theirOnChange?.(defaultValue)
     })
-  }, [form, onChange /* Explicitly ignoring `defaultValue` */])
+  }, [form, theirOnChange /* Explicitly ignoring `defaultValue` */])
 
   return (
     <ComboboxActionsContext.Provider value={actions}>

diff --git 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(
+        <form
+          onSubmit={(e) => {
+            e.preventDefault()
+            handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement)))
+          }}
+        >
+          <Listbox name="assignee" defaultValue={['bob'] as string[]} multiple>
+            <Listbox.Button>{({ value }) => value.join(', ') || 'Trigger'}</Listbox.Button>
+            <Listbox.Options>
+              {data.map((person) => (
+                <Listbox.Option key={person} value={person}>
+                  {person}
+                </Listbox.Option>
+              ))}
+            </Listbox.Options>
+          </Listbox>
+          <button id="submit">submit</button>
+          <button type="reset" id="reset">
+            reset
+          </button>
+        </form>
+      )
+
+      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
@@ -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 (
     <ListboxActionsContext.Provider value={actions}>

diff --git 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 `<Listbox multiple>` and `<Combobox multiple>` ([#2626](https://github.com/tailwindlabs/headlessui/pull/2626))
+- Use correct value when resetting `<Listbox multiple>` and `<Combobox multiple>` ([#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
@@ -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`
+          <form @submit="handleSubmit">
+            <Combobox name="assignee" :defaultValue="['bob']" multiple>
+              <ComboboxButton v-slot="{ value }"
+                >{{ value.join(', ') || 'Trigger' }}</ComboboxButton
+              >
+              <ComboboxOptions>
+                <ComboboxOption v-for="person in data" :key="person" :value="person">
+                  {{ person }}
+                </ComboboxOption>
+              </ComboboxOptions>
+            </Combobox>
+            <button id="submit">submit</button>
+            <button type="reset" id="reset">reset</button>
+          </form>
+        `,
+        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
@@ -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<typeof requestAnimationFrame> | null = null
     let orderOptionsRaf: ReturnType<typeof requestAnimationFrame> | null = null
 

diff --git 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`
+          <form @submit="handleSubmit">
+            <Listbox name="assignee" :defaultValue="['bob']" multiple>
+              <ListboxButton v-slot="{ value }">{{ value.join(', ') || 'Trigger' }}</ListboxButton>
+              <ListboxOptions>
+                <ListboxOption v-for="person in data" :key="person" :value="person">
+                  {{ person }}
+                </ListboxOption>
+              </ListboxOptions>
+            </Listbox>
+            <button id="submit">submit</button>
+            <button type="reset" id="reset">reset</button>
+          </form>
+        `,
+        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
@@ -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)