From aa9c20751566aa65665410af7888d704868cddb9 Mon Sep 17 00:00:00 2001
From: Jason Beggs <jason@roasted.dev>
Date: Thu, 21 Sep 2023 17:22:01 -0400
Subject: [PATCH 1/7] Account for null values when comparing "by" property

---
 packages/ui/src/combobox.js                   |  6 +-
 .../integration/plugins/ui/combobox.spec.js   | 73 +++++++++++++++++++
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/packages/ui/src/combobox.js b/packages/ui/src/combobox.js
index b0ac15b72..9dfed9d5a 100644
--- a/packages/ui/src/combobox.js
+++ b/packages/ui/src/combobox.js
@@ -246,7 +246,11 @@ function handleRoot(el, Alpine) {
 
                     if (typeof by === 'string') {
                         let property = by
-                        by = (a, b) => a[property] === b[property]
+                        by = (a, b) => {
+                            if (typeof a !== 'object' || typeof b !== 'object') return Alpine.raw(a) === Alpine.raw(b)
+
+                            return a[property] === b[property];
+                        }
                     }
 
                     return by(a, b)
diff --git a/tests/cypress/integration/plugins/ui/combobox.spec.js b/tests/cypress/integration/plugins/ui/combobox.spec.js
index 6092f1326..497e11487 100644
--- a/tests/cypress/integration/plugins/ui/combobox.spec.js
+++ b/tests/cypress/integration/plugins/ui/combobox.spec.js
@@ -708,6 +708,79 @@ test('"multiple" and "name" props together',
     },
 );
 
+test('"by" prop with string value',
+    [html`
+        <div
+            x-data="{
+                people: [
+                    { id: 1, name: 'Wade Cooper' },
+                    { id: 2, name: 'Arlene Mccoy' },
+                    { id: 3, name: 'Devon Webb' },
+                    { id: 4, name: 'Tom Cook' },
+                    { id: 5, name: 'Tanya Fox', disabled: true },
+                    { id: 6, name: 'Hellen Schmidt' },
+                    { id: 7, name: 'Caroline Schultz' },
+                    { id: 8, name: 'Mason Heaney' },
+                    { id: 9, name: 'Claudie Smitham' },
+                    { id: 10, name: 'Emil Schaefer' },
+                ]
+            }"
+            x-combobox
+            by="id"
+        >
+            <label x-combobox:label>Assigned to</label>
+
+            <input x-combobox:input :display-value="(person) => person" type="text">
+            <button x-combobox:button x-text="$combobox.value ? $combobox.value : 'Select People'"></button>
+
+            <ul x-combobox:options>
+                <template x-for="person in people" :key="person.id">
+                    <li
+                        :option="person.id"
+                        x-combobox:option
+                        :value="person.id"
+                        :disabled="person.disabled"
+                        :class="{
+                            'selected': $comboboxOption.isSelected,
+                            'active': $comboboxOption.isActive,
+                        }"
+                    >
+                        <span x-text="person.name"></span>
+                    </li>
+                </template>
+            </ul>
+        </div>
+    `],
+    ({ get }) => {
+        get('ul').should(notBeVisible())
+        get('button').click()
+        get('ul').should(beVisible())
+        get('button').click()
+        get('ul').should(notBeVisible())
+        get('button').click()
+        get('[option="2"]').click()
+        get('ul').should(notBeVisible())
+        get('input').should(haveValue('2'))
+        get('button').should(haveText('2'))
+        get('button').click()
+        get('ul').should(contain('Wade Cooper'))
+            .should(contain('Arlene Mccoy'))
+            .should(contain('Devon Webb'))
+        get('[option="3"]').click()
+        get('ul').should(notBeVisible())
+        get('input').should(haveValue('3'))
+        get('button').should(haveText('3'))
+        get('button').click()
+        get('ul').should(contain('Wade Cooper'))
+            .should(contain('Arlene Mccoy'))
+            .should(contain('Devon Webb'))
+        get('[option="1"]').click()
+        get('ul').should(notBeVisible())
+        get('input').should(haveValue('1'))
+        get('button').should(haveText('1'))
+    },
+);
+
 test('keyboard controls',
     [html`
         <div

From e179f7b37e788c80a98ed3378b68c2af1d1d66f5 Mon Sep 17 00:00:00 2001
From: Jason Beggs <jason@roasted.dev>
Date: Thu, 21 Sep 2023 17:53:52 -0400
Subject: [PATCH 2/7] More fixes

---
 packages/ui/src/combobox.js                   |  5 +-
 .../integration/plugins/ui/combobox.spec.js   | 76 +++++++++++++++++++
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/packages/ui/src/combobox.js b/packages/ui/src/combobox.js
index 9dfed9d5a..e001f95ef 100644
--- a/packages/ui/src/combobox.js
+++ b/packages/ui/src/combobox.js
@@ -247,7 +247,10 @@ function handleRoot(el, Alpine) {
                     if (typeof by === 'string') {
                         let property = by
                         by = (a, b) => {
-                            if (typeof a !== 'object' || typeof b !== 'object') return Alpine.raw(a) === Alpine.raw(b)
+                            if ((! a || typeof a !== 'object') || (! b || typeof b !== 'object')) {
+                                return Alpine.raw(a) === Alpine.raw(b)
+                            }
+
 
                             return a[property] === b[property];
                         }
diff --git a/tests/cypress/integration/plugins/ui/combobox.spec.js b/tests/cypress/integration/plugins/ui/combobox.spec.js
index 497e11487..899604517 100644
--- a/tests/cypress/integration/plugins/ui/combobox.spec.js
+++ b/tests/cypress/integration/plugins/ui/combobox.spec.js
@@ -781,6 +781,82 @@ test('"by" prop with string value',
     },
 );
 
+test('"by" prop with string value and "nullable"',
+    [html`
+        <div
+            x-data="{
+                people: [
+                    { id: 1, name: 'Wade Cooper' },
+                    { id: 2, name: 'Arlene Mccoy' },
+                    { id: 3, name: 'Devon Webb' },
+                    { id: 4, name: 'Tom Cook' },
+                    { id: 5, name: 'Tanya Fox', disabled: true },
+                    { id: 6, name: 'Hellen Schmidt' },
+                    { id: 7, name: 'Caroline Schultz' },
+                    { id: 8, name: 'Mason Heaney' },
+                    { id: 9, name: 'Claudie Smitham' },
+                    { id: 10, name: 'Emil Schaefer' },
+                ]
+            }"
+            x-combobox
+            by="id"
+            default-value="5"
+            nullable
+        >
+            <label x-combobox:label>Assigned to</label>
+
+            <input x-combobox:input :display-value="(person) => person?.name" type="text">
+            <button x-combobox:button x-text="$combobox.value ? $combobox.value.name : 'Select People'"></button>
+
+            <ul x-combobox:options>
+                <template x-for="person in people" :key="person.id">
+                    <li
+                        :option="person.id"
+                        x-combobox:option
+                        :value="person"
+                        :disabled="person.disabled"
+                        :class="{
+                            'selected': $comboboxOption.isSelected,
+                            'active': $comboboxOption.isActive,
+                        }"
+                    >
+                        <span x-text="person.name"></span>
+                    </li>
+                </template>
+            </ul>
+        </div>
+    `],
+    ({ get }) => {
+        get('ul').should(notBeVisible())
+        get('button').click()
+        get('ul').should(beVisible())
+        get('button').click()
+        get('ul').should(notBeVisible())
+        get('button').click()
+        get('[option="2"]').click()
+        get('ul').should(notBeVisible())
+        get('input').should(haveValue('Arlene Mccoy'))
+        get('button').should(haveText('Arlene Mccoy'))
+        get('button').click()
+        get('ul').should(contain('Wade Cooper'))
+            .should(contain('Arlene Mccoy'))
+            .should(contain('Devon Webb'))
+        get('[option="3"]').click()
+        get('ul').should(notBeVisible())
+        get('input').should(haveValue('Devon Webb'))
+        get('button').should(haveText('Devon Webb'))
+        get('button').click()
+        get('ul').should(contain('Wade Cooper'))
+            .should(contain('Arlene Mccoy'))
+            .should(contain('Devon Webb'))
+        get('[option="1"]').click()
+        get('ul').should(notBeVisible())
+        get('input').should(haveValue('Wade Cooper'))
+        get('button').should(haveText('Wade Cooper'))
+    },
+);
+
+
 test('keyboard controls',
     [html`
         <div

From 11e39455abbb4bfd02d8f2565cefe115da60a6fe Mon Sep 17 00:00:00 2001
From: Jason Beggs <jason@roasted.dev>
Date: Sun, 5 Nov 2023 14:49:42 -0500
Subject: [PATCH 3/7] Fix aria-selected attribute

---
 packages/ui/src/combobox.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/ui/src/combobox.js b/packages/ui/src/combobox.js
index e001f95ef..2e1f87eba 100644
--- a/packages/ui/src/combobox.js
+++ b/packages/ui/src/combobox.js
@@ -439,7 +439,7 @@ function handleOption(el, Alpine) {
 
         // Only the active element should have aria-selected="true"...
         'x-effect'() {
-            this.$comboboxOption.isActive
+            this.$comboboxOption.isSelected
                 ? el.setAttribute('aria-selected', true)
                 : el.removeAttribute('aria-selected')
         },

From 2b4399f4c368bca0ee490ef8ce600c59195ab9e0 Mon Sep 17 00:00:00 2001
From: Jason Beggs <jason@roasted.dev>
Date: Sun, 5 Nov 2023 14:50:30 -0500
Subject: [PATCH 4/7] Fix home/end/page up/page down + shift keydown handling

---
 packages/ui/src/list-context.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/packages/ui/src/list-context.js b/packages/ui/src/list-context.js
index fd51f8450..abec6ef0e 100644
--- a/packages/ui/src/list-context.js
+++ b/packages/ui/src/list-context.js
@@ -292,6 +292,8 @@ export function generateContext(Alpine, multiple, orientation, activateSelectedO
                     break;
                 case 'Home':
                 case 'PageUp':
+                    if (e.key == 'Home' && e.shiftKey) return;
+
                     e.preventDefault(); e.stopPropagation()
                     setIsTyping(false)
                     this.reorderKeys(); hasActive = this.hasActive()
@@ -300,6 +302,8 @@ export function generateContext(Alpine, multiple, orientation, activateSelectedO
 
                 case 'End':
                 case 'PageDown':
+                    if (e.key == 'End' && e.shiftKey) return;
+
                     e.preventDefault(); e.stopPropagation()
                     setIsTyping(false)
                     this.reorderKeys(); hasActive = this.hasActive()

From a5e7192eca1c39d3950ab5c2a144739618ece9b0 Mon Sep 17 00:00:00 2001
From: Jason Beggs <jason@roasted.dev>
Date: Sun, 5 Nov 2023 14:50:49 -0500
Subject: [PATCH 5/7] Add some additional tests

---
 .../integration/plugins/ui/combobox.spec.js   | 98 +++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/tests/cypress/integration/plugins/ui/combobox.spec.js b/tests/cypress/integration/plugins/ui/combobox.spec.js
index 899604517..23d780719 100644
--- a/tests/cypress/integration/plugins/ui/combobox.spec.js
+++ b/tests/cypress/integration/plugins/ui/combobox.spec.js
@@ -1452,3 +1452,101 @@ test('active element logic when opening a combobox',
         get('[option="3"]').should(haveAttribute('aria-selected', 'true'))
     }
 )
+
+test.only('can remove an option without other options getting removed',
+    [html`<div
+        x-data="{
+            query: '',
+            selected: [],
+            frameworks: [
+                {
+                    id: 1,
+                    name: 'Laravel',
+                    disabled: false,
+                },
+                {
+                    id: 2,
+                    name: 'Ruby on Rails',
+                    disabled: false,
+                },
+                {
+                    id: 3,
+                    name: 'Django',
+                    disabled: false,
+                },
+            ],
+            get filteredFrameworks() {
+                return this.query === ''
+                    ? this.frameworks
+                    : this.frameworks.filter((framework) => {
+                        return framework.name.toLowerCase().includes(this.query.toLowerCase())
+                    })
+            },
+            remove(framework) {
+                this.selected = this.selected.filter((i) => i !== framework)
+            }
+        }"
+    >
+        <div x-combobox x-model="selected" multiple>
+            <div x-show="selected.length">
+                <template x-for="selectedFramework in selected" :key="selectedFramework.id">
+                    <button x-on:click.prevent="remove(selectedFramework)" :remove-option="selectedFramework.id">
+                        <span x-text="selectedFramework.name"></span>
+                    </button>
+                </template>
+            </div>
+
+            <div>
+                <div>
+                    <input
+                        x-combobox:input
+                        @change="query = $event.target.value;"
+                        placeholder="Search..."
+                    />
+                    <button x-combobox:button>
+                        Show options
+                    </button>
+                </div>
+
+                <div x-combobox:options x-cloak x-transition.out.opacity>
+                    <ul>
+                        <template
+                            x-for="framework in filteredFrameworks"
+                            :key="framework.id"
+                            hidden
+                        >
+                            <li
+                                x-combobox:option
+                                :option="framework.id"
+                                :value="framework"
+                                :disabled="framework.disabled"
+                            >
+                                <span x-text="framework.name"></span>
+
+                                <span x-show="$comboboxOption.isSelected" :check="framework.id">&check;</span>
+                            </li>
+                        </template>
+                    </ul>
+
+                    <p x-show="filteredFrameworks.length == 0">No frameworks match your query.</p>
+                </div>
+            </div>
+        </div>
+    </div>
+    `],
+    ({ get }) => {
+        get('input').type('a').trigger('input')
+        cy.wait(100)
+        get('[option="1"]').click()
+        get('[option="2"]').click()
+        get('[option="3"]').click()
+        get('[remove-option="3"]').click()
+        get('[option="1"]').should(haveAttribute('aria-selected', 'true'))
+        get('[option="2"]').should(haveAttribute('aria-selected', 'true'))
+        get('[option="3"]').should(haveAttribute('aria-selected', 'false'))
+        get('button').click()
+        get('[check="1"]').should(beVisible())
+        get('[check="2"]').should(beVisible())
+        get('[check="3"]').should(notBeVisible())
+    },
+);

From 63a5ffdfa41c147ca31cfe64250c1452ab7c497b Mon Sep 17 00:00:00 2001
From: Jason Beggs <jason@roasted.dev>
Date: Sun, 5 Nov 2023 15:25:56 -0500
Subject: [PATCH 6/7] Fix more aria-selected tests

---
 packages/ui/src/combobox.js                   |  2 +-
 .../integration/plugins/ui/combobox.spec.js   | 24 ++++++++++++-------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/packages/ui/src/combobox.js b/packages/ui/src/combobox.js
index 2e1f87eba..9e3871af5 100644
--- a/packages/ui/src/combobox.js
+++ b/packages/ui/src/combobox.js
@@ -441,7 +441,7 @@ function handleOption(el, Alpine) {
         'x-effect'() {
             this.$comboboxOption.isSelected
                 ? el.setAttribute('aria-selected', true)
-                : el.removeAttribute('aria-selected')
+                : el.setAttribute('aria-selected', false)
         },
 
         ':aria-disabled'() { return this.$comboboxOption.isDisabled },
diff --git a/tests/cypress/integration/plugins/ui/combobox.spec.js b/tests/cypress/integration/plugins/ui/combobox.spec.js
index 7bd4935ac..266341d40 100644
--- a/tests/cypress/integration/plugins/ui/combobox.spec.js
+++ b/tests/cypress/integration/plugins/ui/combobox.spec.js
@@ -1062,13 +1062,13 @@ test('has accessibility attributes',
             .should(haveAttribute('role', 'option'))
             .should(haveAttribute('id', 'alpine-combobox-option-1'))
             .should(haveAttribute('tabindex', '-1'))
-            .should(haveAttribute('aria-selected', 'true'))
+            .should(haveAttribute('aria-selected', 'false'))
 
         get('[option="2"]')
             .should(haveAttribute('role', 'option'))
             .should(haveAttribute('id', 'alpine-combobox-option-2'))
             .should(haveAttribute('tabindex', '-1'))
-            .should(notHaveAttribute('aria-selected'))
+            .should(haveAttribute('aria-selected', 'false'))
 
         get('input')
             .should(haveAttribute('role', 'combobox'))
@@ -1080,9 +1080,13 @@ test('has accessibility attributes',
             .should(haveAttribute('aria-activedescendant', 'alpine-combobox-option-1'))
             .type('{downarrow}')
             .should(haveAttribute('aria-activedescendant', 'alpine-combobox-option-2'))
+            .type('{enter}')
 
         get('[option="2"]')
             .should(haveAttribute('aria-selected', 'true'))
+
+        get('[option="1"]')
+            .should(haveAttribute('aria-selected', 'false'))
     },
 )
 
@@ -1420,6 +1424,7 @@ test('active element logic when opening a combobox',
                                     :option="person.id"
                                     :value="person"
                                     :disabled="person.disabled"
+                                    :class="$comboboxOption.isActive ? 'active' : ''"
                                     x-text="person.name"
                                 >
                                 </li>
@@ -1436,19 +1441,22 @@ test('active element logic when opening a combobox',
         get('button').click()
         // First option is selected on opening if no preselection
         get('ul').should(beVisible())
-        get('[option="1"]').should(haveAttribute('aria-selected', 'true'))
+        get('[option="1"]').should(haveAttribute('aria-selected', 'false'))
+        get('[option="1"]').should(haveClasses(['active']))
         // First match is selected while typing
-        get('[option="4"]').should(notHaveAttribute('aria-selected'))
+        get('[option="4"]').should(haveAttribute('aria-selected', 'false'))
+        get('[option="4"]').should(notHaveClasses(['active']))
         get('input').type('T')
         get('input').trigger('change')
-        get('[option="4"]').should(haveAttribute('aria-selected', 'true'))
+        get('[option="4"]').should(haveAttribute('aria-selected', 'false'))
+        get('[option="4"]').should(haveClasses(['active']))
         // Reset state and select option 3
         get('button').click()
         get('button').click()
         get('[option="3"]').click()
         // Previous selection is selected
         get('button').click()
-        get('[option="4"]').should(notHaveAttribute('aria-selected'))
+        get('[option="4"]').should(haveAttribute('aria-selected', 'false'))
         get('[option="3"]').should(haveAttribute('aria-selected', 'true'))
     }
 )
@@ -1487,7 +1495,7 @@ test.only('can remove an option without other options getting removed',
             }
         }"
     >
-        <div x-combobox x-model="selected" multiple>
+        <div x-combobox x-model="selected" by="id" multiple>
             <div x-show="selected.length">
                 <template x-for="selectedFramework in selected" :key="selectedFramework.id">
                     <button x-on:click.prevent="remove(selectedFramework)" :remove-option="selectedFramework.id">
@@ -1544,7 +1552,7 @@ test.only('can remove an option without other options getting removed',
         get('[option="1"]').should(haveAttribute('aria-selected', 'true'))
         get('[option="2"]').should(haveAttribute('aria-selected', 'true'))
         get('[option="3"]').should(haveAttribute('aria-selected', 'false'))
-        get('button').click()
+        get('input').type('a').trigger('input')
         get('[check="1"]').should(beVisible())
         get('[check="2"]').should(beVisible())
         get('[check="3"]').should(notBeVisible())

From 5d39a077c7e46ec6c9b88ce9dec9a556d1230a81 Mon Sep 17 00:00:00 2001
From: Jason Beggs <jason@roasted.dev>
Date: Sun, 5 Nov 2023 15:28:49 -0500
Subject: [PATCH 7/7] wip

---
 packages/ui/src/combobox.js                           | 1 +
 tests/cypress/integration/plugins/ui/combobox.spec.js | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/packages/ui/src/combobox.js b/packages/ui/src/combobox.js
index 9e3871af5..50739fcc8 100644
--- a/packages/ui/src/combobox.js
+++ b/packages/ui/src/combobox.js
@@ -247,6 +247,7 @@ function handleRoot(el, Alpine) {
                     if (typeof by === 'string') {
                         let property = by
                         by = (a, b) => {
+                            // Handle null values
                             if ((! a || typeof a !== 'object') || (! b || typeof b !== 'object')) {
                                 return Alpine.raw(a) === Alpine.raw(b)
                             }
diff --git a/tests/cypress/integration/plugins/ui/combobox.spec.js b/tests/cypress/integration/plugins/ui/combobox.spec.js
index 266341d40..b3225dfba 100644
--- a/tests/cypress/integration/plugins/ui/combobox.spec.js
+++ b/tests/cypress/integration/plugins/ui/combobox.spec.js
@@ -1461,7 +1461,7 @@ test('active element logic when opening a combobox',
     }
 )
 
-test.only('can remove an option without other options getting removed',
+test('can remove an option without other options getting removed',
     [html`<div
         x-data="{
             query: '',