fix(listbox): on fire model-value-changed when update from children

.changeset/silver-forks-learn.md

@@ -0,0 +1,5 @@
+---
+'@lion/listbox': patch
+---
+
+Only send model-value-changed if the event is caused by one of its children

packages/form-integrations/test/model-value-consistency.test.js

@@ -20,9 +20,10 @@ import '@lion/radio-group/lion-radio.js';
 
 import '@lion/select/lion-select.js';
 
+import '@lion/combobox/lion-combobox.js';
+import '@lion/listbox/lion-listbox.js';
+import '@lion/listbox/lion-option.js';
 import '@lion/select-rich/lion-select-rich.js';
-import '@lion/select-rich/lion-options.js';
-import '@lion/select-rich/lion-option.js';
 
 import '@lion/fieldset/lion-fieldset.js';
 
@@ -206,18 +207,19 @@ describe('lion-select', () => {
   });
 });
 
-describe('lion-select-rich', () => {
+['combobox', 'listbox', 'select-rich'].forEach(chunk => {
+  const tagname = `lion-${chunk}`;
+  const tag = unsafeStatic(tagname);
+  describe(`${tagname}`, () => {
     describe(featureName, () => {
       it(getFirstPaintTitle(firstStampCount), async () => {
         const spy = sinon.spy();
         await fixture(html`
-        <lion-select-rich @model-value-changed="${spy}">
+          <${tag} @model-value-changed="${spy}">
-          <lion-options slot="input">
             <lion-option .choiceValue="${'option1'}"></lion-option>
             <lion-option .choiceValue="${'option2'}"></lion-option>
             <lion-option .choiceValue="${'option3'}"></lion-option>
-          </lion-options>
+          </${tag}>
-        </lion-select-rich>
         `);
 
         expect(spy.callCount).to.equal(firstStampCount);
@@ -226,13 +228,11 @@ describe('lion-select-rich', () => {
       it(getInteractionTitle(interactionCount), async () => {
         const spy = sinon.spy();
         const el = await fixture(html`
-        <lion-select-rich>
+          <${tag}>
-          <lion-options slot="input">
+            <lion-option .choiceValue="${'option1'}">Option 1</lion-option>
-            <lion-option .choiceValue="${'option1'}"></lion-option>
+            <lion-option .choiceValue="${'option2'}">Option 2</lion-option>
-            <lion-option .choiceValue="${'option2'}"></lion-option>
+            <lion-option .choiceValue="${'option3'}">Option 3</lion-option>
-            <lion-option .choiceValue="${'option3'}"></lion-option>
+          </${tag}>
-          </lion-options>
-        </lion-select-rich>
         `);
 
         el.addEventListener('model-value-changed', spy);
@@ -247,6 +247,7 @@ describe('lion-select-rich', () => {
         expect(spy.callCount).to.equal(interactionCount);
       });
     });
+  });
 });
 
 describe('lion-fieldset', () => {

packages/listbox/src/ListboxMixin.js

@@ -1,7 +1,7 @@
-import { ChoiceGroupMixin, FormControlMixin, FormRegistrarMixin } from '@lion/form-core';
+import { css, dedupeMixin, html, ScopedElementsMixin, SlotMixin } from '@lion/core';
-import { css, html, dedupeMixin, ScopedElementsMixin, SlotMixin } from '@lion/core';
-import '@lion/core/src/differentKeyEventNamesShimIE.js';
 import '@lion/core/src/closestPolyfill.js';
+import '@lion/core/src/differentKeyEventNamesShimIE.js';
+import { ChoiceGroupMixin, FormControlMixin, FormRegistrarMixin } from '@lion/form-core';
 import { LionOptions } from './LionOptions.js';
 
 // TODO: extract ListNavigationWithActiveDescendantMixin that can be reused in [role="menu"]
@@ -389,7 +389,7 @@ const ListboxMixinImplementation = superclass =>
       });
 
       this.__proxyChildModelValueChanged(
-        /** @type {Event & { target: LionOption; }} */ ({ target: child }),
+        /** @type {CustomEvent & { target: LionOption; }} */ ({ target: child }),
       );
       this.resetInteractionState();
       /* eslint-enable no-param-reassign */
@@ -674,7 +674,7 @@ const ListboxMixinImplementation = superclass =>
     }
 
     /**
-     * @param {Event & { target: LionOption; }} ev
+     * @param {CustomEvent & { target: LionOption; }} ev
      */
     __proxyChildModelValueChanged(ev) {
       // We need to redispatch the model-value-changed event on 'this', so it will
@@ -687,9 +687,12 @@ const ListboxMixinImplementation = superclass =>
 
       // don't send this.modelValue as oldValue, since it will take modelValue getter which takes it from child elements, which is already the updated value
       this.requestUpdate('modelValue', this.__oldModelValue);
+      // only send model-value-changed if the event is caused by one of its children
+      if (ev.detail && ev.detail.formPath) {
         this.dispatchEvent(
           new CustomEvent('model-value-changed', { detail: { element: ev.target } }),
         );
+      }
       this.__oldModelValue = this.modelValue;
     }
 

packages/listbox/test-suites/ListboxMixin.suite.js

@@ -1,11 +1,11 @@
+import '@lion/core/src/differentKeyEventNamesShimIE.js';
 import { Required } from '@lion/form-core';
-import sinon from 'sinon';
-import { expect, html, fixture as _fixture, unsafeStatic } from '@open-wc/testing';
 import { LionOptions } from '@lion/listbox';
 import '@lion/listbox/lion-option.js';
 import '@lion/listbox/lion-options.js';
+import { expect, fixture as _fixture, html, unsafeStatic } from '@open-wc/testing';
+import sinon from 'sinon';
 import '../lion-listbox.js';
-import '@lion/core/src/differentKeyEventNamesShimIE.js';
 
 /**
  * @typedef {import('../src/LionListbox').LionListbox} LionListbox
@@ -60,6 +60,56 @@ export function runListboxMixinSuite(customConfig = {}) {
         expect(el.modelValue).to.equal('10');
       });
 
+      it('should dispatch model-value-changed 1 time on first paint', async () => {
+        const spy = sinon.spy();
+        await fixture(html`
+          <${tag} @model-value-changed="${spy}">
+            <${optionTag} .choiceValue="${'10'}">Item 1</${optionTag}>
+            <${optionTag} .choiceValue="${'20'}">Item 2</${optionTag}>
+            <${optionTag} .choiceValue="${'30'}">Item 3</${optionTag}>
+          </${tag}>
+        `);
+        expect(spy.callCount).to.equal(1);
+      });
+
+      it('should dispatch model-value-changed 1 time on interaction', async () => {
+        const spy = sinon.spy();
+        const el = await fixture(html`
+          <${tag}>
+            <${optionTag} .choiceValue="${'10'}">Item 1</${optionTag}>
+            <${optionTag} .choiceValue="${'20'}">Item 2</${optionTag}>
+            <${optionTag} .choiceValue="${'30'}">Item 3</${optionTag}>
+          </${tag}>
+        `);
+
+        el.addEventListener('model-value-changed', spy);
+        el.formElements[1].checked = true;
+        expect(spy.callCount).to.equal(1);
+
+        spy.resetHistory();
+
+        el.formElements[2].checked = true;
+        expect(spy.callCount).to.equal(1);
+      });
+
+      it('should not dispatch model-value-changed on reappend checked child', async () => {
+        const spy = sinon.spy();
+        const el = await fixture(html`
+          <${tag}>
+            <${optionTag} .choiceValue="${'10'}">Item 1</${optionTag}>
+            <${optionTag} .choiceValue="${'20'}">Item 2</${optionTag}>
+            <${optionTag} .choiceValue="${'30'}">Item 3</${optionTag}>
+          </${tag}>
+        `);
+
+        el.addEventListener('model-value-changed', spy);
+        el.formElements[1].checked = true;
+        expect(spy.callCount).to.equal(1);
+
+        el.appendChild(el.formElements[1]);
+        expect(spy.callCount).to.equal(1);
+      });
+
       it('automatically sets the name attribute of child checkboxes to its own name', async () => {
         const el = await fixture(html`
         <${tag} name="foo">