Combobox and listbox: Save optionKey in x-data to fix usage in Livewi…

…re (#3990) * Save optionKey in x-data When optionKey is not saved in x-data, the value is lost after a Livewire update * Fix typos * refactor * more fixes --------- Co-authored-by: Caleb Porzio <[email protected]>
alpinejs · Jan 22, 2024 · 5fe438f · 5fe438f
1 parent aa3375a
commit 5fe438f
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 15 deletions.
diff --git a/packages/ui/src/combobox.js b/packages/ui/src/combobox.js
@@ -43,19 +43,23 @@ export default function (Alpine) {
     Alpine.magic('comboboxOption', el => {
         let data = Alpine.$data(el)
 
-        let optionEl = Alpine.findClosest(el, i => i.__optionKey)
+        // It's not great depending on the existance of the attribute in the DOM
+        // but it's probably the fastest and most reliable at this point...
+        let optionEl = Alpine.findClosest(el, i => {
+            return i.hasAttribute('x-combobox:option')
+        })
 
         if (! optionEl) throw 'No x-combobox:option directive found...'
 
         return {
             get isActive() {
-                return data.__context.isActiveKey(optionEl.__optionKey)
+                return data.__context.isActiveKey(Alpine.$data(optionEl).__optionKey)
             },
             get isSelected() {
                 return data.__isSelected(optionEl)
             },
             get isDisabled() {
-                return data.__context.isDisabled(optionEl.__optionKey)
+                return data.__context.isDisabled(Alpine.$data(optionEl).__optionKey)
             },
         }
     })
@@ -453,18 +457,20 @@ function handleOption(el, Alpine) {
         // Initialize...
         'x-data'() {
             return {
+                '__optionKey': null,
+
                 init() {
-                    let key = this.$el.__optionKey = (Math.random() + 1).toString(36).substring(7)
+                    this.__optionKey = (Math.random() + 1).toString(36).substring(7)
 
                     let value = Alpine.extractProp(this.$el, 'value')
                     let disabled = Alpine.extractProp(this.$el, 'disabled', false, false)
 
                     // memoize the context as it's not going to change
                     // and calling this.$data on mouse action is expensive
-                    this.__context.registerItem(key, this.$el, value, disabled)
+                    this.__context.registerItem(this.__optionKey, this.$el, value, disabled)
                 },
                 destroy() {
-                    this.__context.unregisterItem(this.$el.__optionKey)
+                    this.__context.unregisterItem(this.__optionKey)
                 }
             }
         },
@@ -498,7 +504,6 @@ function handleOption(el, Alpine) {
     })
 }
 
-
 // Little utility to defer a callback into the microtask queue...
 function microtask(callback) {
     return new Promise(resolve => queueMicrotask(() => resolve(callback())))

diff --git a/packages/ui/src/listbox.js b/packages/ui/src/listbox.js
@@ -48,19 +48,23 @@ export default function (Alpine) {
     Alpine.magic('listboxOption', (el) => {
         let data = Alpine.$data(el)
 
-        let optionEl = Alpine.findClosest(el, i => i.__optionKey)
+        // It's not great depending on the existance of the attribute in the DOM
+        // but it's probably the fastest and most reliable at this point...
+        let optionEl = Alpine.findClosest(el, i => {
+            return i.hasAttribute('x-listbox:option')
+        })
 
         if (! optionEl) throw 'No x-listbox:option directive found...'
 
         return {
             get isActive() {
-                return data.__context.isActiveKey(optionEl.__optionKey)
+                return data.__context.isActiveKey(Alpine.$data(optionEl).__optionKey)
             },
             get isSelected() {
                 return data.__isSelected(optionEl)
             },
             get isDisabled() {
-                return data.__context.isDisabled(optionEl.__optionKey)
+                return data.__context.isDisabled(Alpine.$data(optionEl).__optionKey)
             },
         }
     })
@@ -346,16 +350,18 @@ function handleOption(el, Alpine) {
             // Initialize...
             'x-data'() {
                 return {
+                    '__optionKey': null,
+
                     init() {
-                        let key = el.__optionKey = (Math.random() + 1).toString(36).substring(7)
+                        this.__optionKey = (Math.random() + 1).toString(36).substring(7)
 
                         let value = Alpine.extractProp(el, 'value')
                         let disabled = Alpine.extractProp(el, 'disabled', false, false)
 
-                        this.$data.__context.registerItem(key, el, value, disabled)
+                        this.$data.__context.registerItem(this.__optionKey, el, value, disabled)
                     },
                     destroy() {
-                        this.$data.__context.unregisterItem(this.$el.__optionKey)
+                        this.$data.__context.unregisterItem(this.__optionKey)
                     },
                 }
             },

diff --git a/tests/cypress/integration/plugins/ui/combobox.spec.js b/tests/cypress/integration/plugins/ui/combobox.spec.js
@@ -1,4 +1,4 @@
-import { beVisible, beHidden, haveAttribute, haveClasses, notHaveClasses, haveText, contain, notContain, html, notBeVisible, notHaveAttribute, notExist, haveFocus, test, haveValue, haveLength} from '../../../utils'
+import { beVisible, beHidden, haveAttribute, haveClasses, notHaveClasses, haveText, contain, notContain, html, notBeVisible, notHaveAttribute, notExist, haveFocus, test, haveValue, haveLength, ensureNoConsoleWarns} from '../../../utils'
 
 test('it works with x-model',
     [html`
@@ -1630,3 +1630,39 @@ test('can remove an option without other options getting removed',
         get('[check="3"]').should(notBeVisible())
     },
 );
+
+test('works with morph',
+    [html`
+    <div x-data="{ value: null }">
+        <div x-combobox x-model="value">
+            <button x-combobox:button>Select Framework</button>
+
+            <ul x-combobox:options>
+                <li x-combobox:option value="laravel">Laravel</li>
+            </ul>
+        </div>
+
+        Selected: <span x-text="value"></span>
+    </div>
+    `],
+    ({ get }, reload, window, document) => {
+        let toHtml = html`
+        <div x-data="{ value: null }">
+            <div x-combobox x-model="value">
+                <button x-combobox:button>Select Framework (updated)</button>
+
+                <ul x-combobox:options>
+                    <li x-combobox:option value="laravel">Laravel</li>
+                </ul>
+            </div>
+
+            Selected: <span x-text="value"></span>
+        </div>
+        `
+        ensureNoConsoleWarns()
+
+        get('div').then(([el]) => window.Alpine.morph(el, toHtml))
+
+        get('button').should(haveText('Select Framework (updated)'))
+    },
+)
diff --git a/tests/cypress/integration/plugins/ui/listbox.spec.js b/tests/cypress/integration/plugins/ui/listbox.spec.js
@@ -1,4 +1,4 @@
-import { beVisible, beHidden, haveAttribute, haveClasses, notHaveClasses, haveText, html, notBeVisible, notHaveAttribute, notExist, haveFocus, test} from '../../../utils'
+import { beVisible, beHidden, haveAttribute, haveClasses, notHaveClasses, haveText, html, notBeVisible, notHaveAttribute, notExist, haveFocus, test, ensureNoConsoleWarns} from '../../../utils'
 
 test('it works with x-model',
     [html`
@@ -921,4 +921,40 @@ test('"static" prop',
     },
 )
 
+test('works with morph',
+    [html`
+    <div x-data="{ value: null }">
+        <div x-listbox x-model="value">
+            <button x-listbox:button>Select Framework</button>
+
+            <ul x-listbox:options>
+                <li x-listbox:option value="laravel">Laravel</li>
+            </ul>
+        </div>
+
+        Selected: <span x-text="value"></span>
+    </div>
+    `],
+    ({ get }, reload, window, document) => {
+        let toHtml = html`
+        <div x-data="{ value: null }">
+            <div x-listbox x-model="value">
+                <button x-listbox:button>Select Framework (updated)</button>
+
+                <ul x-listbox:options>
+                    <li x-listbox:option value="laravel">Laravel</li>
+                </ul>
+            </div>
+
+            Selected: <span x-text="value"></span>
+        </div>
+        `
+        ensureNoConsoleWarns()
+
+        get('div').then(([el]) => window.Alpine.morph(el, toHtml))
+
+        get('button').should(haveText('Select Framework (updated)'))
+    },
+)
+
 // test "by" attribute
diff --git a/tests/cypress/utils.js b/tests/cypress/utils.js
@@ -5,6 +5,18 @@ export function html(strings) {
     return strings.raw[0]
 }
 
+export function ensureNoConsoleWarns() {
+    cy.window().then((win) => {
+        let cache = win.console.warn
+
+        win.console.warn = () => { throw new Error('Console warn was triggered') }
+
+        cy.on('window:before:unload', () => {
+            win.console.warn = cache
+        });
+    });
+}
+
 export let test = function (name, template, callback, handleExpectedErrors = false) {
     it(name, () => {
         injectHtmlAndBootAlpine(cy, template, callback, undefined, handleExpectedErrors)