ayo/lion
1
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

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

Browse source
This commit is contained in:
qa46hx 2020-10-22 14:58:32 +02:00
parent 51732b0e4f
commit 16dd0cecc3
4 changed files with 103 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/silver-forks-learn.md Normal file
View file

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

67
packages/form-integrations/test/model-value-consistency.test.js
View file

@ -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`
<lion-select-rich @model-value-changed="${spy}">
<lion-options slot="input">
['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}">
<lion-option .choiceValue="${'option1'}"></lion-option>
<lion-option .choiceValue="${'option2'}"></lion-option>
<lion-option .choiceValue="${'option3'}"></lion-option>
</lion-options>
</lion-select-rich>
`);
</${tag}>
`);
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`
<lion-select-rich>
<lion-options slot="input">
<lion-option .choiceValue="${'option1'}"></lion-option>
<lion-option .choiceValue="${'option2'}"></lion-option>
<lion-option .choiceValue="${'option3'}"></lion-option>
</lion-options>
</lion-select-rich>
`);
it(getInteractionTitle(interactionCount), async () => {
const spy = sinon.spy();
const el = await fixture(html`
<${tag}>
<lion-option .choiceValue="${'option1'}">Option 1</lion-option>
<lion-option .choiceValue="${'option2'}">Option 2</lion-option>
<lion-option .choiceValue="${'option3'}">Option 3</lion-option>
</${tag}>
`);
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);
});
});
});
});

19
packages/listbox/src/ListboxMixin.js
View file

@ -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;
}

56
packages/listbox/test-suites/ListboxMixin.suite.js
View file

@ -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">