diff --git a/packages/choice-input/package.json b/packages/choice-input/package.json
index 8cd109cfc..03038ea50 100644
--- a/packages/choice-input/package.json
+++ b/packages/choice-input/package.json
@@ -38,6 +38,7 @@
"devDependencies": {
"@lion/input": "^0.1.22",
"@open-wc/demoing-storybook": "^0.2.0",
- "@open-wc/testing": "^0.12.5"
+ "@open-wc/testing": "^0.12.5",
+ "sinon": "^7.2.2"
}
}
diff --git a/packages/choice-input/src/ChoiceInputMixin.js b/packages/choice-input/src/ChoiceInputMixin.js
index e85dfa002..37a0346a4 100644
--- a/packages/choice-input/src/ChoiceInputMixin.js
+++ b/packages/choice-input/src/ChoiceInputMixin.js
@@ -1,70 +1,94 @@
/* eslint-disable class-methods-use-this */
import { html, css, nothing } from '@lion/core';
-import { ObserverMixin } from '@lion/core/src/ObserverMixin.js';
import { FormatMixin } from '@lion/field';
export const ChoiceInputMixin = superclass =>
// eslint-disable-next-line
- class ChoiceInputMixin extends FormatMixin(ObserverMixin(superclass)) {
- get delegations() {
+ class ChoiceInputMixin extends FormatMixin(superclass) {
+ static get properties() {
return {
- ...super.delegations,
- target: () => this.inputElement,
- properties: [...super.delegations.properties, 'checked'],
- attributes: [...super.delegations.attributes, 'checked'],
+ ...super.properties,
+ /**
+ * Boolean indicating whether or not this element is checked by the end user.
+ */
+ checked: {
+ type: Boolean,
+ reflect: true,
+ },
+ /**
+ * Whereas 'normal' `.modelValue`s usually store a complex/typed version
+ * of a view value, choice inputs have a slightly different approach.
+ * In order to remain their Single Source of Truth characteristic, choice inputs
+ * store both the value and 'checkedness', in the format { value: 'x', checked: true }
+ * Different from the platform, this also allows to serialize the 'non checkedness',
+ * allowing to restore form state easily and inform the server about unchecked options.
+ */
+ modelValue: {
+ type: Object,
+ hasChanged: (nw, old = {}) => nw.value !== old.value || nw.checked !== old.checked,
+ },
+ /**
+ * The value property of the modelValue. It provides an easy inteface for storing
+ * (complex) values in the modelValue
+ */
+ choiceValue: {
+ type: Object,
+ },
};
}
- static get syncObservers() {
- return {
- ...super.syncObservers,
- _syncModelValueToChecked: ['modelValue'],
- };
- }
-
- static get asyncObservers() {
- return {
- ...super.asyncObservers,
- _reflectCheckedToCssClass: ['modelValue'],
- };
- }
-
- get choiceChecked() {
- return this.modelValue.checked;
- }
-
- set choiceChecked(checked) {
- if (this.modelValue.checked !== checked) {
- this.modelValue = { value: this.modelValue.value, checked };
- }
- }
-
get choiceValue() {
return this.modelValue.value;
}
set choiceValue(value) {
+ this.requestUpdate('choiceValue', this.choiceValue);
if (this.modelValue.value !== value) {
this.modelValue = { value, checked: this.modelValue.checked };
}
}
+ _requestUpdate(name, oldValue) {
+ super._requestUpdate(name, oldValue);
+
+ if (name === 'modelValue') {
+ if (this.modelValue.checked !== this.checked) {
+ this.__syncModelCheckedToChecked(this.modelValue.checked);
+ }
+ } else if (name === 'checked') {
+ if (this.modelValue.checked !== this.checked) {
+ this.__syncCheckedToModel(this.checked);
+ }
+ }
+ }
+
+ firstUpdated(c) {
+ super.firstUpdated(c);
+ if (c.has('checked')) {
+ // Here we set the initial value for our [slot=input] content,
+ // which has been set by our SlotMixin
+ this.__syncCheckedToInputElement();
+ }
+ }
+
+ updated(c) {
+ super.updated(c);
+ if (c.has('modelValue')) {
+ this._reflectCheckedToCssClass({ modelValue: this.modelValue });
+ this.__syncCheckedToInputElement();
+ }
+ }
+
constructor() {
super();
this.modelValue = { value: '', checked: false };
}
/**
- * @override
- * Override InteractionStateMixin
- * 'prefilled' should be false when modelValue is { checked: false }, which would return
- * true in original method (since non-empty objects are considered prefilled by default).
+ * Styles for [input=radio] and [input=checkbox] wrappers.
+ * For [role=option] extensions, please override completely
*/
- static _isPrefilled(modelValue) {
- return modelValue.checked;
- }
-
static get styles() {
return [
css`
@@ -79,6 +103,10 @@ export const ChoiceInputMixin = superclass =>
];
}
+ /**
+ * Template for [input=radio] and [input=checkbox] wrappers.
+ * For [role=option] extensions, please override completely
+ */
render() {
return html`
@@ -96,22 +124,44 @@ export const ChoiceInputMixin = superclass =>
}
connectedCallback() {
- if (super.connectedCallback) super.connectedCallback();
- this.addEventListener('user-input-changed', this._toggleChecked);
+ super.connectedCallback();
+ this.addEventListener('user-input-changed', this.__toggleChecked);
this._reflectCheckedToCssClass();
}
disconnectedCallback() {
- if (super.disconnectedCallback) super.disconnectedCallback();
- this.removeEventListener('user-input-changed', this._toggleChecked);
+ super.disconnectedCallback();
+ this.removeEventListener('user-input-changed', this.__toggleChecked);
}
- _toggleChecked() {
- this.choiceChecked = !this.choiceChecked;
+ __toggleChecked() {
+ this.checked = !this.checked;
}
- _syncModelValueToChecked({ modelValue }) {
- this.checked = !!modelValue.checked;
+ __syncModelCheckedToChecked(checked) {
+ this.checked = checked;
+ }
+
+ __syncCheckedToModel(checked) {
+ this.modelValue = { value: this.choiceValue, checked };
+ }
+
+ __syncCheckedToInputElement() {
+ // .inputElement might not be available yet(slot content)
+ // or at all (no reliance on platform construct, in case of [role=option])
+ if (this.inputElement) {
+ this.inputElement.checked = this.checked;
+ }
+ }
+
+ /**
+ * @override
+ * Override InteractionStateMixin
+ * 'prefilled' should be false when modelValue is { checked: false }, which would return
+ * true in original method (since non-empty objects are considered prefilled by default).
+ */
+ static _isPrefilled(modelValue) {
+ return modelValue.checked;
}
/**
@@ -126,30 +176,13 @@ export const ChoiceInputMixin = superclass =>
/**
* @override
- * Override FormatMixin default dispatching of model-value-changed as it only does a simple
- * comparision which is not enough in js because
- * { value: 'foo', checked: true } !== { value: 'foo', checked: true }
- * We do our own "deep" comparision.
- *
- * @param {object} modelValue
- * @param {object} modelValue the old one
+ * hasChanged is designed for async (updated) callback, also check for sync
+ * (_requestUpdate) callback
*/
- // TODO: consider making a generic option inside FormatMixin for deep object comparisons when
- // modelValue is an object
- _dispatchModelValueChangedEvent({ modelValue }, { modelValue: old }) {
- let changed = true;
- if (old) {
- changed = modelValue.value !== old.value || modelValue.checked !== old.checked;
+ _onModelValueChanged({ modelValue }, { modelValue: old }) {
+ if (this.constructor._classProperties.get('modelValue').hasChanged(modelValue, old)) {
+ super._onModelValueChanged({ modelValue });
}
- if (changed) {
- this.dispatchEvent(
- new CustomEvent('model-value-changed', { bubbles: true, composed: true }),
- );
- }
- }
-
- _reflectCheckedToCssClass() {
- this.classList[this.choiceChecked ? 'add' : 'remove']('state-checked');
}
/**
@@ -171,11 +204,30 @@ export const ChoiceInputMixin = superclass =>
/**
* @override
- * Overridden from ValidateMixin, since a different modelValue is used for choice inputs.
+ * Overridden from Field, since a different modelValue is used for choice inputs.
*/
__isRequired(modelValue) {
return {
required: !!modelValue.checked,
};
}
+
+ /**
+ * @deprecated use .checked
+ */
+ get choiceChecked() {
+ return this.checked;
+ }
+
+ /**
+ * @deprecated use .checked
+ */
+ set choiceChecked(c) {
+ this.checked = c;
+ }
+
+ /** @deprecated for styling purposes, use [checked] attribute */
+ _reflectCheckedToCssClass() {
+ this.classList[this.checked ? 'add' : 'remove']('state-checked');
+ }
};
diff --git a/packages/choice-input/test/ChoiceInputMixin.test.js b/packages/choice-input/test/ChoiceInputMixin.test.js
index 34f5ede3c..3d41e2caf 100644
--- a/packages/choice-input/test/ChoiceInputMixin.test.js
+++ b/packages/choice-input/test/ChoiceInputMixin.test.js
@@ -1,5 +1,6 @@
import { expect, fixture } from '@open-wc/testing';
import { html } from '@lion/core';
+import sinon from 'sinon';
import { LionInput } from '@lion/input';
import { ChoiceInputMixin } from '../src/ChoiceInputMixin.js';
@@ -50,11 +51,11 @@ describe('ChoiceInputMixin', () => {
`);
expect(counter).to.equal(1); // undefined to set value
- el.choiceChecked = true;
+ el.checked = true;
expect(counter).to.equal(2);
// no change means no event
- el.choiceChecked = true;
+ el.checked = true;
el.choiceValue = 'foo';
el.modelValue = { value: 'foo', checked: true };
expect(counter).to.equal(2);
@@ -87,58 +88,136 @@ describe('ChoiceInputMixin', () => {
`);
expect(el.error.required).to.be.true;
- el.choiceChecked = true;
+ el.checked = true;
expect(el.error.required).to.be.undefined;
});
describe('Checked state synchronization', () => {
it('synchronizes checked state initially (via attribute or property)', async () => {
const el = await fixture(``);
- expect(el.choiceChecked).to.equal(false, 'initially unchecked');
+ expect(el.checked).to.equal(false, 'initially unchecked');
const precheckedElementAttr = await fixture(html`
-
+
`);
- expect(precheckedElementAttr.choiceChecked).to.equal(true, 'initially checked via attribute');
+ el._syncValueUpwards = () => {}; // We need to disable the method for the test to pass
+
+ expect(precheckedElementAttr.checked).to.equal(true, 'initially checked via attribute');
});
it('can be checked and unchecked programmatically', async () => {
const el = await fixture(``);
- expect(el.choiceChecked).to.be.false;
- el.choiceChecked = true;
- expect(el.choiceChecked).to.be.true;
+ expect(el.checked).to.be.false;
+ el.checked = true;
+ expect(el.checked).to.be.true;
+
+ await el.updateComplete;
+ expect(el.inputElement.checked).to.be.true;
});
it('can be checked and unchecked via user interaction', async () => {
const el = await fixture(``);
el.inputElement.click();
- expect(el.choiceChecked).to.be.true;
+ expect(el.checked).to.be.true;
el.inputElement.click();
- expect(el.choiceChecked).to.be.false;
+ expect(el.checked).to.be.false;
});
it('synchronizes modelValue to checked state and vice versa', async () => {
const el = await fixture(html`
`);
- expect(el.choiceChecked).to.be.false;
+ expect(el.checked).to.be.false;
expect(el.modelValue).to.deep.equal({
checked: false,
value: 'foo',
});
- el.choiceChecked = true;
- expect(el.choiceChecked).to.be.true;
+ el.checked = true;
+ expect(el.checked).to.be.true;
expect(el.modelValue).to.deep.equal({
checked: true,
value: 'foo',
});
});
- it('synchronizes checked state to class "state-checked" for styling purposes', async () => {
+ it('ensures optimal synchronize performance by preventing redundant computation steps', async () => {
+ /* we are checking private apis here to make sure we do not have cyclical updates
+ which can be quite common for these type of connected data */
+ const el = await fixture(html`
+
+ `);
+ expect(el.checked).to.be.false;
+
+ const spyModelCheckedToChecked = sinon.spy(el, '__syncModelCheckedToChecked');
+ const spyCheckedToModel = sinon.spy(el, '__syncCheckedToModel');
+ el.checked = true;
+ expect(el.modelValue.checked).to.be.true;
+ expect(spyModelCheckedToChecked.callCount).to.equal(0);
+ expect(spyCheckedToModel.callCount).to.equal(1);
+
+ el.modelValue = { value: 'foo', checked: false };
+ expect(el.checked).to.be.false;
+ expect(spyModelCheckedToChecked.callCount).to.equal(1);
+ expect(spyCheckedToModel.callCount).to.equal(1);
+
+ // not changeing values should not trigger any updates
+ el.checked = false;
+ el.modelValue = { value: 'foo', checked: false };
+ expect(spyModelCheckedToChecked.callCount).to.equal(1);
+ expect(spyCheckedToModel.callCount).to.equal(1);
+ });
+
+ it('synchronizes checked state to [checked] attribute for styling purposes', async () => {
+ const hasAttr = el => el.hasAttribute('checked');
+ const el = await fixture(``);
+ const elChecked = await fixture(html`
+
+
+
+ `);
+
+ // Initial values
+ expect(hasAttr(el)).to.equal(false, 'inital unchecked element');
+ expect(hasAttr(elChecked)).to.equal(true, 'inital checked element');
+
+ // Programmatically via checked
+ el.checked = true;
+ elChecked.checked = false;
+
+ await el.updateComplete;
+ expect(hasAttr(el)).to.equal(true, 'programmatically checked');
+ expect(hasAttr(elChecked)).to.equal(false, 'programmatically unchecked');
+
+ // reset
+ el.checked = false;
+ elChecked.checked = true;
+
+ // Via user interaction
+ el.inputElement.click();
+ elChecked.inputElement.click();
+ await el.updateComplete;
+ expect(hasAttr(el)).to.equal(true, 'user click checked');
+ expect(hasAttr(elChecked)).to.equal(false, 'user click unchecked');
+
+ // reset
+ el.checked = false;
+ elChecked.checked = true;
+
+ // Programmatically via modelValue
+ el.modelValue = { value: '', checked: true };
+ elChecked.modelValue = { value: '', checked: false };
+ await el.updateComplete;
+ expect(hasAttr(el)).to.equal(true, 'modelValue checked');
+ expect(hasAttr(elChecked)).to.equal(false, 'modelValue unchecked');
+ });
+
+ it('[deprecated] synchronizes checked state to class "state-checked" for styling purposes', async () => {
const hasClass = el => [].slice.call(el.classList).indexOf('state-checked') > -1;
const el = await fixture(``);
const elChecked = await fixture(html`
-
+
+
+
`);
// Initial values
@@ -146,15 +225,16 @@ describe('ChoiceInputMixin', () => {
expect(hasClass(elChecked)).to.equal(true, 'inital checked element');
// Programmatically via checked
- el.choiceChecked = true;
- elChecked.choiceChecked = false;
+ el.checked = true;
+ elChecked.checked = false;
+
await el.updateComplete;
expect(hasClass(el)).to.equal(true, 'programmatically checked');
expect(hasClass(elChecked)).to.equal(false, 'programmatically unchecked');
// reset
- el.choiceChecked = false;
- elChecked.choiceChecked = true;
+ el.checked = false;
+ elChecked.checked = true;
// Via user interaction
el.inputElement.click();
@@ -164,8 +244,8 @@ describe('ChoiceInputMixin', () => {
expect(hasClass(elChecked)).to.equal(false, 'user click unchecked');
// reset
- el.choiceChecked = false;
- elChecked.choiceChecked = true;
+ el.checked = false;
+ elChecked.checked = true;
// Programmatically via modelValue
el.modelValue = { value: '', checked: true };
@@ -174,6 +254,21 @@ describe('ChoiceInputMixin', () => {
expect(hasClass(el)).to.equal(true, 'modelValue checked');
expect(hasClass(elChecked)).to.equal(false, 'modelValue unchecked');
});
+
+ it('[deprecated] uses choiceChecked to set checked state', async () => {
+ const el = await fixture(html`
+
+ `);
+ expect(el.choiceChecked).to.be.false;
+ el.choiceChecked = true;
+ expect(el.checked).to.be.true;
+ expect(el.modelValue).to.deep.equal({
+ checked: true,
+ value: 'foo',
+ });
+ await el.updateComplete;
+ expect(el.inputElement.checked).to.be.true;
+ });
});
describe('Format/parse/serialize loop', () => {
@@ -184,7 +279,7 @@ describe('ChoiceInputMixin', () => {
expect(el.modelValue).deep.equal({ value: 'foo', checked: false });
const elChecked = await fixture(html`
-
+
`);
expect(elChecked.modelValue).deep.equal({ value: 'foo', checked: true });
});
@@ -203,7 +298,7 @@ describe('ChoiceInputMixin', () => {
describe('Interaction states', () => {
it('is considered prefilled when checked and not considered prefilled when unchecked', async () => {
const el = await fixture(html`
-
+
`);
expect(el.prefilled).equal(true, 'checked element not considered prefilled');