@lion/choice-input - npm Package Compare versions

Comparing version 0.1.22 to 0.2.0

CHANGELOG.md

@@ -6,2 +6,13 @@ # Change Log
 
+# [0.2.0](https://github.com/ing-bank/lion/compare/@lion/choice-input@0.1.22...@lion/choice-input@0.2.0) (2019-06-25)
+
+
+### Features
+
+* **choice-input:** api normalization and cleanup ([2870e0d](https://github.com/ing-bank/lion/commit/2870e0d))
+
+
+
+
+
 ## [0.1.22](https://github.com/ing-bank/lion/compare/@lion/choice-input@0.1.21...@lion/choice-input@0.1.22) (2019-06-24)
@@ -8,0 +19,0 @@

package.json

 {
   "name": "@lion/choice-input",
-  "version": "0.1.22",
+  "version": "0.2.0",
   "description": "Base for all choise inputs like checkbox/radio",
@@ -41,5 +41,6 @@ "author": "ing-bank",
     "@open-wc/demoing-storybook": "^0.2.0",
-    "@open-wc/testing": "^0.12.5"
+    "@open-wc/testing": "^0.12.5",
+    "sinon": "^7.2.2"
   },
-  "gitHead": "48d2e547d6ad7ec89a16d9de731862d874fc5d20"
+  "gitHead": "45252784c60ce5b357892788e539f607060b0352"
 }

src/ChoiceInputMixin.js

 /* 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';
@@ -9,43 +8,74 @@
   // 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'],
-      };
+    get choiceValue() {
+      return this.modelValue.value;
     }
 
-    static get asyncObservers() {
-      return {
-        ...super.asyncObservers,
-        _reflectCheckedToCssClass: ['modelValue'],
-      };
+    set choiceValue(value) {
+      this.requestUpdate('choiceValue', this.choiceValue);
+      if (this.modelValue.value !== value) {
+        this.modelValue = { value, checked: this.modelValue.checked };
+      }
     }
 
-    get choiceChecked() {
-      return this.modelValue.checked;
-    }
+    _requestUpdate(name, oldValue) {
+      super._requestUpdate(name, oldValue);
 
-    set choiceChecked(checked) {
-      if (this.modelValue.checked !== checked) {
-        this.modelValue = { value: this.modelValue.value, checked };
+      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);
+        }
       }
     }
 
-    get choiceValue() {
-      return this.modelValue.value;
+    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();
+      }
     }
 
-    set choiceValue(value) {
-      if (this.modelValue.value !== value) {
-        this.modelValue = { value, checked: this.modelValue.checked };
+    updated(c) {
+      super.updated(c);
+      if (c.has('modelValue')) {
+        this._reflectCheckedToCssClass({ modelValue: this.modelValue });
+        this.__syncCheckedToInputElement();
       }
@@ -60,11 +90,5 @@ }
     /**
-     * @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() {
@@ -84,2 +108,6 @@ return [
 
+    /**
+     * Template for [input=radio] and [input=checkbox] wrappers.
+     * For [role=option] extensions, please override completely
+     */
     render() {
@@ -102,4 +130,4 @@ return html`
     connectedCallback() {
-      if (super.connectedCallback) super.connectedCallback();
-      this.addEventListener('user-input-changed', this._toggleChecked);
+      super.connectedCallback();
+      this.addEventListener('user-input-changed', this.__toggleChecked);
       this._reflectCheckedToCssClass();
@@ -109,16 +137,38 @@ }
     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;
+    }
+
+    /**
+     * @override
      * This method is overridden from FormatMixin. It originally fired the normalizing
@@ -134,28 +184,11 @@ * 'user-input-changed' event after listening to the native 'input' event.
      * @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');
-    }
-
     /**
@@ -180,3 +213,3 @@ * @override
      * @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.
      */
@@ -188,2 +221,21 @@ __isRequired(modelValue) {
     }
+
+    /**
+     * @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');
+    }
   };

test/ChoiceInputMixin.test.js

 import { expect, fixture } from '@open-wc/testing';
 import { html } from '@lion/core';
+import sinon from 'sinon';
 
@@ -53,7 +54,7 @@ import { LionInput } from '@lion/input';
 
-    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';
@@ -91,3 +92,3 @@ el.modelValue = { value: 'foo', checked: true };
     expect(el.error.required).to.be.true;
-    el.choiceChecked = true;
+    el.checked = true;
     expect(el.error.required).to.be.undefined;
@@ -99,8 +100,10 @@ });
       const el = await fixture(`<choice-input></choice-input>`);
-      expect(el.choiceChecked).to.equal(false, 'initially unchecked');
+      expect(el.checked).to.equal(false, 'initially unchecked');
 
       const precheckedElementAttr = await fixture(html`
-        <choice-input .choiceChecked=${true}></choice-input>
+        <choice-input .checked=${true}></choice-input>
       `);
-      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');
     });
@@ -110,5 +113,8 @@
       const el = await fixture(`<choice-input></choice-input>`);
-      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;
     });
@@ -119,5 +125,5 @@
       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;
     });
@@ -129,3 +135,3 @@
       `);
-      expect(el.choiceChecked).to.be.false;
+      expect(el.checked).to.be.false;
       expect(el.modelValue).to.deep.equal({
@@ -135,4 +141,4 @@ checked: false,
       });
-      el.choiceChecked = true;
-      expect(el.choiceChecked).to.be.true;
+      el.checked = true;
+      expect(el.checked).to.be.true;
       expect(el.modelValue).to.deep.equal({
@@ -144,7 +150,80 @@ checked: true,
 
-    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`
+        <choice-input .choiceValue=${'foo'}></choice-input>
+      `);
+      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(`<choice-input></choice-input>`);
+      const elChecked = await fixture(html`
+        <choice-input .checked=${true}>
+          <input slot="input" />
+        </choice-input>
+      `);
+
+      // 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(`<choice-input></choice-input>`);
       const elChecked = await fixture(html`
-        <choice-input .choiceChecked=${true}></choice-input>
+        <choice-input .checked=${true}>
+          <input slot="input" />
+        </choice-input>
       `);
@@ -157,4 +236,5 @@
       // Programmatically via checked
-      el.choiceChecked = true;
-      elChecked.choiceChecked = false;
+      el.checked = true;
+      elChecked.checked = false;
+
       await el.updateComplete;
@@ -165,4 +245,4 @@ expect(hasClass(el)).to.equal(true, 'programmatically checked');
       // reset
-      el.choiceChecked = false;
-      elChecked.choiceChecked = true;
+      el.checked = false;
+      elChecked.checked = true;
 
@@ -177,4 +257,4 @@ // Via user interaction
       // reset
-      el.choiceChecked = false;
-      elChecked.choiceChecked = true;
+      el.checked = false;
+      elChecked.checked = true;
 
@@ -188,2 +268,17 @@ // Programmatically via modelValue
     });
+
+    it('[deprecated] uses choiceChecked to set checked state', async () => {
+      const el = await fixture(html`
+        <choice-input .choiceValue=${'foo'}></choice-input>
+      `);
+      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;
+    });
   });
@@ -199,3 +294,3 @@
       const elChecked = await fixture(html`
-        <choice-input .choiceValue=${'foo'} .choiceChecked=${true}></choice-input>
+        <choice-input .choiceValue=${'foo'} .checked=${true}></choice-input>
       `);
@@ -219,3 +314,3 @@ expect(elChecked.modelValue).deep.equal({ value: 'foo', checked: true });
       const el = await fixture(html`
-        <choice-input .choiceChecked=${true}></choice-input>
+        <choice-input .checked=${true}></choice-input>
       `);
@@ -222,0 +317,0 @@ expect(el.prefilled).equal(true, 'checked element not considered prefilled');

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

25336

increased by26.43%

Lines of code

467

increased by37.35%

Worsened metrics

Dev dependency count

4

increased by33.33%

No dependency changes