Fix nullable by prop for combobox and radio group (#1815)

* Fix nullable by prop for combobox and radio group * Update changelog
tailwindlabs · Sep 2, 2022 · 13463f0 · 13463f0
1 parent 5165be5
commit 13463f0
Show file tree

Hide file tree

Showing 10 changed files with 151 additions and 6 deletions.
diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-- Add `by` prop for `Listbox`, `Combobox` and `RadioGroup` ([#1482](https://github.com/tailwindlabs/headlessui/pull/1482), [#1717](https://github.com/tailwindlabs/headlessui/pull/1717), [#1814](https://github.com/tailwindlabs/headlessui/pull/1814))
+- Add `by` prop for `Listbox`, `Combobox` and `RadioGroup` ([#1482](https://github.com/tailwindlabs/headlessui/pull/1482), [#1717](https://github.com/tailwindlabs/headlessui/pull/1717), [#1814](https://github.com/tailwindlabs/headlessui/pull/1814), [#1815](https://github.com/tailwindlabs/headlessui/pull/1815))
 - Add `@headlessui/tailwindcss` plugin ([#1487](https://github.com/tailwindlabs/headlessui/pull/1487))
 
 ### Fixed

diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx
@@ -209,6 +209,44 @@ describe('Rendering', () => {
         })
       )
 
+      it(
+        'should be possible to compare null values by a field',
+        suppressConsoleLogs(async () => {
+          render(
+            <Combobox value={null} onChange={console.log} by="id">
+              <Combobox.Button>Trigger</Combobox.Button>
+              <Combobox.Options>
+                {options.map((option) => (
+                  <Combobox.Option
+                    key={option.id}
+                    value={option}
+                    className={(info) => JSON.stringify(info)}
+                  >
+                    {option.name}
+                  </Combobox.Option>
+                ))}
+              </Combobox.Options>
+            </Combobox>
+          )
+
+          await click(getComboboxButton())
+
+          let [alice, bob, charlie] = getComboboxOptions()
+          expect(alice).toHaveAttribute(
+            'class',
+            JSON.stringify({ active: true, selected: false, disabled: false })
+          )
+          expect(bob).toHaveAttribute(
+            'class',
+            JSON.stringify({ active: false, selected: false, disabled: false })
+          )
+          expect(charlie).toHaveAttribute(
+            'class',
+            JSON.stringify({ active: false, selected: false, disabled: false })
+          )
+        })
+      )
+
       it(
         'should be possible to compare objects by a field',
         suppressConsoleLogs(async () => {

diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx
@@ -417,7 +417,7 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
     typeof by === 'string'
       ? (a, z) => {
           let property = by as unknown as keyof TValue
-          return a[property] === z[property]
+          return a?.[property] === z?.[property]
         }
       : by
   )

diff --git a/packages/@headlessui-react/src/components/radio-group/radio-group.test.tsx b/packages/@headlessui-react/src/components/radio-group/radio-group.test.tsx
@@ -404,6 +404,39 @@ describe('Rendering', () => {
       })
     )
 
+    it(
+      'should be possible to compare null values by a field',
+      suppressConsoleLogs(async () => {
+        render(
+          <RadioGroup value={null} onChange={console.log} by="id">
+            {options.map((option) => (
+              <RadioGroup.Option
+                key={option.id}
+                value={option}
+                className={(info) => JSON.stringify(info)}
+              >
+                {option.name}
+              </RadioGroup.Option>
+            ))}
+          </RadioGroup>
+        )
+
+        let [alice, bob, charlie] = getRadioGroupOptions()
+        expect(alice).toHaveAttribute(
+          'class',
+          JSON.stringify({ checked: false, disabled: false, active: false })
+        )
+        expect(bob).toHaveAttribute(
+          'class',
+          JSON.stringify({ checked: false, disabled: false, active: false })
+        )
+        expect(charlie).toHaveAttribute(
+          'class',
+          JSON.stringify({ checked: false, disabled: false, active: false })
+        )
+      })
+    )
+
     it(
       'should be possible to compare objects by a field',
       suppressConsoleLogs(async () => {

diff --git a/packages/@headlessui-react/src/components/radio-group/radio-group.tsx b/packages/@headlessui-react/src/components/radio-group/radio-group.tsx
@@ -140,7 +140,7 @@ let RadioGroupRoot = forwardRefWithAs(function RadioGroup<
     typeof by === 'string'
       ? (a: TType, z: TType) => {
           let property = by as unknown as keyof TType
-          return a[property] === z[property]
+          return a?.[property] === z?.[property]
         }
       : by
   )

diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-- Add `by` prop for `Listbox`, `Combobox` and `RadioGroup` ([#1482](https://github.com/tailwindlabs/headlessui/pull/1482), [#1717](https://github.com/tailwindlabs/headlessui/pull/1717), [#1814](https://github.com/tailwindlabs/headlessui/pull/1814))
+- Add `by` prop for `Listbox`, `Combobox` and `RadioGroup` ([#1482](https://github.com/tailwindlabs/headlessui/pull/1482), [#1717](https://github.com/tailwindlabs/headlessui/pull/1717), [#1814](https://github.com/tailwindlabs/headlessui/pull/1814), [#1815](https://github.com/tailwindlabs/headlessui/pull/1815))
 - Add `@headlessui/tailwindcss` plugin ([#1487](https://github.com/tailwindlabs/headlessui/pull/1487))
 
 ### Fixed

diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts
@@ -243,6 +243,45 @@ describe('Rendering', () => {
         })
       )
 
+      it(
+        'should be possible to compare null values by a field',
+        suppressConsoleLogs(async () => {
+          renderTemplate({
+            template: html`
+              <Combobox v-model="value" by="id">
+                <ComboboxButton>Trigger</ComboboxButton>
+                <ComboboxOptions>
+                  <ComboboxOption
+                    v-for="option in options"
+                    :key="option.id"
+                    :value="option"
+                    v-slot="data"
+                    >{{ JSON.stringify(data) }}</ComboboxOption
+                  >
+                </ComboboxOptions>
+              </Combobox>
+            `,
+            setup: () => {
+              let value = ref(null)
+              return { options, value }
+            },
+          })
+
+          await click(getComboboxButton())
+
+          let [alice, bob, charlie] = getComboboxOptions()
+          expect(alice).toHaveTextContent(
+            JSON.stringify({ active: true, selected: false, disabled: false })
+          )
+          expect(bob).toHaveTextContent(
+            JSON.stringify({ active: false, selected: false, disabled: false })
+          )
+          expect(charlie).toHaveTextContent(
+            JSON.stringify({ active: false, selected: false, disabled: false })
+          )
+        })
+      )
+
       it(
         'should be possible to compare objects by a field',
         suppressConsoleLogs(async () => {

diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts
@@ -187,7 +187,7 @@ export let Combobox = defineComponent({
       compare(a: any, z: any) {
         if (typeof props.by === 'string') {
           let property = props.by as unknown as any
-          return a[property] === z[property]
+          return a?.[property] === z?.[property]
         }
         return props.by(a, z)
       },

diff --git a/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts b/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts
@@ -532,6 +532,41 @@ describe('Rendering', () => {
       })
     )
 
+    it(
+      'should be possible to compare null values by a field',
+      suppressConsoleLogs(async () => {
+        renderTemplate({
+          template: html`
+            <RadioGroup v-model="value" by="id">
+              <RadioGroupButton>Trigger</RadioGroupButton>
+              <RadioGroupOption
+                v-for="option in options"
+                :key="option.id"
+                :value="option"
+                v-slot="data"
+                >{{ JSON.stringify(data) }}</RadioGroupOption
+              >
+            </RadioGroup>
+          `,
+          setup: () => {
+            let value = ref(null)
+            return { options, value }
+          },
+        })
+
+        let [alice, bob, charlie] = getRadioGroupOptions()
+        expect(alice).toHaveTextContent(
+          JSON.stringify({ checked: false, disabled: false, active: false })
+        )
+        expect(bob).toHaveTextContent(
+          JSON.stringify({ checked: false, disabled: false, active: false })
+        )
+        expect(charlie).toHaveTextContent(
+          JSON.stringify({ checked: false, disabled: false, active: false })
+        )
+      })
+    )
+
     it(
       'should be possible to compare objects by a field',
       suppressConsoleLogs(async () => {

diff --git a/packages/@headlessui-vue/src/components/radio-group/radio-group.ts b/packages/@headlessui-vue/src/components/radio-group/radio-group.ts
@@ -115,7 +115,7 @@ export let RadioGroup = defineComponent({
       compare(a: any, z: any) {
         if (typeof props.by === 'string') {
           let property = props.by as unknown as any
-          return a[property] === z[property]
+          return a?.[property] === z?.[property]
         }
         return props.by(a, z)
       },