diff --git a/.changeset/silver-forks-learn.md b/.changeset/silver-forks-learn.md new file mode 100644 index 000000000..d092a94d6 --- /dev/null +++ b/.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 diff --git a/packages/form-integrations/test/model-value-consistency.test.js b/packages/form-integrations/test/model-value-consistency.test.js index 4fc842629..6c252b374 100644 --- a/packages/form-integrations/test/model-value-consistency.test.js +++ b/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,45 +207,45 @@ describe('lion-select', () => { }); }); -describe('lion-select-rich', () => { - describe(featureName, () => { - it(getFirstPaintTitle(firstStampCount), async () => { - const spy = sinon.spy(); - await fixture(html` - - +['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` + <${tag} @model-value-changed="${spy}"> - - - `); + + `); - expect(spy.callCount).to.equal(firstStampCount); - }); + expect(spy.callCount).to.equal(firstStampCount); + }); - it(getInteractionTitle(interactionCount), async () => { - const spy = sinon.spy(); - const el = await fixture(html` - - - - - - - - `); + it(getInteractionTitle(interactionCount), async () => { + const spy = sinon.spy(); + const el = await fixture(html` + <${tag}> + Option 1 + Option 2 + Option 3 + + `); - el.addEventListener('model-value-changed', spy); - const option2 = el.querySelector('lion-option:nth-child(2)'); - option2.checked = true; - expect(spy.callCount).to.equal(interactionCount); + el.addEventListener('model-value-changed', spy); + const option2 = el.querySelector('lion-option:nth-child(2)'); + option2.checked = true; + expect(spy.callCount).to.equal(interactionCount); - spy.resetHistory(); + spy.resetHistory(); - const option3 = el.querySelector('lion-option:nth-child(3)'); - option3.checked = true; - expect(spy.callCount).to.equal(interactionCount); + const option3 = el.querySelector('lion-option:nth-child(3)'); + option3.checked = true; + expect(spy.callCount).to.equal(interactionCount); + }); }); }); }); diff --git a/packages/listbox/src/ListboxMixin.js b/packages/listbox/src/ListboxMixin.js index 157044ae2..4fe0ca26f 100644 --- a/packages/listbox/src/ListboxMixin.js +++ b/packages/listbox/src/ListboxMixin.js @@ -1,7 +1,7 @@ -import { ChoiceGroupMixin, FormControlMixin, FormRegistrarMixin } from '@lion/form-core'; -import { css, html, dedupeMixin, ScopedElementsMixin, SlotMixin } from '@lion/core'; -import '@lion/core/src/differentKeyEventNamesShimIE.js'; +import { css, dedupeMixin, html, ScopedElementsMixin, SlotMixin } from '@lion/core'; 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); - this.dispatchEvent( - new CustomEvent('model-value-changed', { detail: { element: ev.target } }), - ); + // 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; } diff --git a/packages/listbox/test-suites/ListboxMixin.suite.js b/packages/listbox/test-suites/ListboxMixin.suite.js index 1f7a2608d..59816fd60 100644 --- a/packages/listbox/test-suites/ListboxMixin.suite.js +++ b/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} .choiceValue="${'20'}">Item 2 + <${optionTag} .choiceValue="${'30'}">Item 3 + + `); + 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} .choiceValue="${'20'}">Item 2 + <${optionTag} .choiceValue="${'30'}">Item 3 + + `); + + 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} .choiceValue="${'20'}">Item 2 + <${optionTag} .choiceValue="${'30'}">Item 3 + + `); + + 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">