Merge pull request #204 from ing-bank/feat/DisabledMixin

feat(button): reduce to minimal css; use DisabledWithTabIndexMixin

packages/button/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@lion/button",
   "version": "0.1.48",
-  "description": "A button that is easily stylable and accessible in all contexts",
+  "description": "A button that is easily styleable and accessible in all contexts",
   "author": "ing-bank",
   "homepage": "https://github.com/ing-bank/lion/",
   "license": "MIT",

packages/button/src/LionButton.js

@@ -1,34 +1,40 @@
-import { css, html, DelegateMixin, SlotMixin } from '@lion/core';
+import { css, html, DelegateMixin, SlotMixin, DisabledWithTabIndexMixin } from '@lion/core';
 import { LionLitElement } from '@lion/core/src/LionLitElement.js';
 
-export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) {
+export class LionButton extends DisabledWithTabIndexMixin(
+  DelegateMixin(SlotMixin(LionLitElement)),
+) {
   static get properties() {
     return {
-      disabled: {
-        type: Boolean,
-        reflect: true,
-      },
       role: {
         type: String,
         reflect: true,
       },
-      tabindex: {
-        type: Number,
-        reflect: true,
-      },
     };
   }
 
   render() {
     return html`
       <div class="btn">
+        ${this._renderBefore()}
         <slot></slot>
+        ${this._renderAfter()}
         <slot name="_button"></slot>
         <div class="click-area" @click="${this.__clickDelegationHandler}"></div>
       </div>
     `;
   }
 
+  // eslint-disable-next-line class-methods-use-this
+  _renderBefore() {
+    return html``;
+  }
+
+  // eslint-disable-next-line class-methods-use-this
+  _renderAfter() {
+    return html``;
+  }
+
   static get styles() {
     return [
       css`
@@ -36,22 +42,20 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) {
           display: inline-block;
           padding-top: 2px;
           padding-bottom: 2px;
-          height: 40px; /* src = https://www.smashingmagazine.com/2012/02/finger-friendly-design-ideal-mobile-touchscreen-target-sizes/ */
+          min-height: 40px; /* src = https://www.smashingmagazine.com/2012/02/finger-friendly-design-ideal-mobile-touchscreen-target-sizes/ */
           outline: 0;
           background-color: transparent;
           box-sizing: border-box;
         }
 
         .btn {
-          height: 24px;
+          min-height: 24px;
           display: flex;
           align-items: center;
           position: relative;
-          border: 1px solid black;
+          background: #eee; /* minimal styling to make it recognizable as btn */
-          border-radius: 8px;
-          background: whitesmoke;
-          color: black;
           padding: 7px 15px;
+          outline: none; /* focus style handled below, else it follows boundaries of click-area */
         }
 
         :host .btn ::slotted(button) {
@@ -69,27 +73,20 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) {
           padding: 0;
         }
 
-        :host(:focus) {
-          outline: none;
-        }
-
         :host(:focus) .btn {
-          border-color: lightblue;
+          /* if you extend, please overwrite */
-          box-shadow: 0 0 8px lightblue, 0 0 0 1px lightblue;
+          outline: 2px solid #bde4ff;
         }
 
         :host(:hover) .btn {
-          background: black;
+          /* if you extend, please overwrite */
-          color: whitesmoke;
+          background: #f4f6f7;
-        }
-
-        :host(:hover) .btn ::slotted(lion-icon) {
-          fill: whitesmoke;
         }
 
         :host(:active) .btn,
         .btn[active] {
-          background: grey;
+          /* if you extend, please overwrite */
+          background: gray;
         }
 
         :host([disabled]) {
@@ -97,22 +94,15 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) {
         }
 
         :host([disabled]) .btn {
+          /* if you extend, please overwrite */
           background: lightgray;
-          color: gray;
+          color: #adadad;
-          fill: gray;
+          fill: #adadad;
-          border-color: gray;
         }
       `,
     ];
   }
 
-  _requestUpdate(name, oldValue) {
-    super._requestUpdate(name, oldValue);
-    if (name === 'disabled') {
-      this.__onDisabledChanged(oldValue);
-    }
-  }
-
   get delegations() {
     return {
       ...super.delegations,
@@ -137,9 +127,7 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) {
 
   constructor() {
     super();
-    this.disabled = false;
     this.role = 'button';
-    this.tabindex = 0;
   }
 
   connectedCallback() {
@@ -203,13 +191,4 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) {
       this.shadowRoot.querySelector('.click-area').click();
     }
   }
-
-  __onDisabledChanged() {
-    if (this.disabled) {
-      this.__originalTabIndex = this.tabindex;
-      this.tabindex = -1;
-    } else {
-      this.tabindex = this.__originalTabIndex;
-    }
-  }
 }

packages/core/index.js

@@ -51,3 +51,5 @@ export { DelegateMixin } from './src/DelegateMixin.js';
 export { DomHelpersMixin } from './src/DomHelpersMixin.js';
 export { LionSingleton } from './src/LionSingleton.js';
 export { SlotMixin } from './src/SlotMixin.js';
+export { DisabledMixin } from './src/DisabledMixin.js';
+export { DisabledWithTabIndexMixin } from './src/DisabledWithTabIndexMixin.js';

packages/core/src/DisabledMixin.js

@@ -0,0 +1,64 @@
+import { dedupeMixin } from './dedupeMixin.js';
+
+/**
+ * #DisabledMixin
+ *
+ * @polymerMixin
+ * @mixinFunction
+ */
+export const DisabledMixin = dedupeMixin(
+  superclass =>
+    // eslint-disable-next-line no-shadow
+    class DisabledMixin extends superclass {
+      static get properties() {
+        return {
+          disabled: {
+            type: Boolean,
+            reflect: true,
+          },
+        };
+      }
+
+      constructor() {
+        super();
+        this.__requestedToBeDisabled = false;
+        this.__isUserSettingDisabled = true;
+
+        this.__restoreDisabledTo = false;
+        this.disabled = false;
+      }
+
+      makeRequestToBeDisabled() {
+        if (this.__requestedToBeDisabled === false) {
+          this.__requestedToBeDisabled = true;
+          this.__restoreDisabledTo = this.disabled;
+          this.__internalSetDisabled(true);
+        }
+      }
+
+      retractRequestToBeDisabled() {
+        if (this.__requestedToBeDisabled === true) {
+          this.__requestedToBeDisabled = false;
+          this.__internalSetDisabled(this.__restoreDisabledTo);
+        }
+      }
+
+      __internalSetDisabled(value) {
+        this.__isUserSettingDisabled = false;
+        this.disabled = value;
+        this.__isUserSettingDisabled = true;
+      }
+
+      _requestUpdate(name, oldValue) {
+        super._requestUpdate(name, oldValue);
+        if (name === 'disabled') {
+          if (this.__isUserSettingDisabled) {
+            this.__restoreDisabledTo = this.disabled;
+          }
+          if (this.disabled === false && this.__requestedToBeDisabled === true) {
+            this.__internalSetDisabled(true);
+          }
+        }
+      }
+    },
+);

packages/core/src/DisabledWithTabIndexMixin.js

@@ -0,0 +1,86 @@
+import { dedupeMixin } from './dedupeMixin.js';
+import { DisabledMixin } from './DisabledMixin.js';
+
+/**
+ * #DisabledWithTabIndexMixin
+ *
+ * @polymerMixin
+ * @mixinFunction
+ */
+export const DisabledWithTabIndexMixin = dedupeMixin(
+  superclass =>
+    // eslint-disable-next-line no-shadow
+    class DisabledWithTabIndexMixin extends DisabledMixin(superclass) {
+      static get properties() {
+        return {
+          // we use a property here as if we use the native tabIndex we can not set a default value
+          // in the constructor as it synchronously sets the attribute which is not allowed in the
+          // constructor phase
+          tabIndex: {
+            type: Number,
+            reflect: true,
+            attribute: 'tabindex',
+          },
+        };
+      }
+
+      constructor() {
+        super();
+        this.__isUserSettingTabIndex = true;
+
+        this.__restoreTabIndexTo = 0;
+        this.__internalSetTabIndex(0);
+      }
+
+      makeRequestToBeDisabled() {
+        super.makeRequestToBeDisabled();
+        if (this.__requestedToBeDisabled === false) {
+          this.__restoreTabIndexTo = this.tabIndex;
+        }
+      }
+
+      retractRequestToBeDisabled() {
+        super.retractRequestToBeDisabled();
+        if (this.__requestedToBeDisabled === true) {
+          this.__internalSetTabIndex(this.__restoreTabIndexTo);
+        }
+      }
+
+      __internalSetTabIndex(value) {
+        this.__isUserSettingTabIndex = false;
+        this.tabIndex = value;
+        this.__isUserSettingTabIndex = true;
+      }
+
+      _requestUpdate(name, oldValue) {
+        super._requestUpdate(name, oldValue);
+
+        if (name === 'disabled') {
+          if (this.disabled) {
+            this.__internalSetTabIndex(-1);
+          } else {
+            this.__internalSetTabIndex(this.__restoreTabIndexTo);
+          }
+        }
+
+        if (name === 'tabIndex') {
+          if (this.__isUserSettingTabIndex) {
+            this.__restoreTabIndexTo = this.tabIndex;
+          }
+
+          if (this.tabIndex !== -1 && this.__requestedToBeDisabled === true) {
+            this.__internalSetTabIndex(-1);
+          }
+        }
+      }
+
+      firstUpdated(changedProperties) {
+        super.firstUpdated(changedProperties);
+        // for ShadyDom the timing is a little different and we need to make sure
+        // the tabindex gets correctly updated here
+        if (this.disabled) {
+          this.__internalSetTabIndex(-1);
+        }
+      }
+    },
+);

packages/core/test/DisabledMixin.test.js

@@ -0,0 +1,86 @@
+import { expect, fixture, html } from '@open-wc/testing';
+
+import { LitElement } from '../index.js';
+import { DisabledMixin } from '../src/DisabledMixin.js';
+
+describe('DisabledMixin', () => {
+  before(() => {
+    class CanBeDisabled extends DisabledMixin(LitElement) {}
+    customElements.define('can-be-disabled', CanBeDisabled);
+  });
+
+  it('reflects disabled to attribute', async () => {
+    const el = await fixture(html`
+      <can-be-disabled></can-be-disabled>
+    `);
+    expect(el.hasAttribute('disabled')).to.be.false;
+    el.disabled = true;
+    await el.updateComplete;
+    expect(el.hasAttribute('disabled')).to.be.true;
+  });
+
+  it('can be requested to be disabled', async () => {
+    const el = await fixture(html`
+      <can-be-disabled></can-be-disabled>
+    `);
+    el.makeRequestToBeDisabled();
+    expect(el.disabled).to.be.true;
+    await el.updateComplete;
+    expect(el.hasAttribute('disabled')).to.be.true;
+  });
+
+  it('will not allow to become enabled after makeRequestToBeDisabled()', async () => {
+    const el = await fixture(html`
+      <can-be-disabled></can-be-disabled>
+    `);
+    el.makeRequestToBeDisabled();
+    expect(el.disabled).to.be.true;
+
+    el.disabled = false;
+    expect(el.disabled).to.be.true;
+  });
+
+  it('will stay disabled after retractRequestToBeDisabled() if it was disabled before', async () => {
+    const el = await fixture(html`
+      <can-be-disabled disabled></can-be-disabled>
+    `);
+    el.makeRequestToBeDisabled();
+    el.retractRequestToBeDisabled();
+    expect(el.disabled).to.be.true;
+  });
+
+  it('will become enabled after retractRequestToBeDisabled() if it was enabled before', async () => {
+    const el = await fixture(html`
+      <can-be-disabled></can-be-disabled>
+    `);
+    el.makeRequestToBeDisabled();
+    expect(el.disabled).to.be.true;
+    el.retractRequestToBeDisabled();
+    expect(el.disabled).to.be.false;
+  });
+
+  it('may allow multiple calls to makeRequestToBeDisabled()', async () => {
+    const el = await fixture(html`
+      <can-be-disabled></can-be-disabled>
+    `);
+    el.makeRequestToBeDisabled();
+    el.makeRequestToBeDisabled();
+    el.retractRequestToBeDisabled();
+    expect(el.disabled).to.be.false;
+  });
+
+  it('will restore last state after retractRequestToBeDisabled()', async () => {
+    const el = await fixture(html`
+      <can-be-disabled></can-be-disabled>
+    `);
+    el.makeRequestToBeDisabled();
+    el.disabled = true;
+    el.retractRequestToBeDisabled();
+    expect(el.disabled).to.be.true;
+
+    el.makeRequestToBeDisabled();
+    el.disabled = false;
+    el.retractRequestToBeDisabled();
+    expect(el.disabled).to.be.false;
+  });
+});

packages/core/test/DisabledWithTabIndexMixin.test.js

@@ -0,0 +1,107 @@
+import { expect, fixture, html } from '@open-wc/testing';
+
+import { LitElement } from '../index.js';
+import { DisabledWithTabIndexMixin } from '../src/DisabledWithTabIndexMixin.js';
+
+describe('DisabledWithTabIndexMixin', () => {
+  before(() => {
+    class WithTabIndex extends DisabledWithTabIndexMixin(LitElement) {}
+    customElements.define('can-be-disabled-with-tab-index', WithTabIndex);
+  });
+
+  it('has an initial tabIndex of 0', async () => {
+    const el = await fixture(html`
+      <can-be-disabled-with-tab-index></can-be-disabled-with-tab-index>
+    `);
+    expect(el.tabIndex).to.equal(0);
+    expect(el.getAttribute('tabindex')).to.equal('0');
+  });
+
+  it('sets tabIndex to -1 if disabled', async () => {
+    const el = await fixture(html`
+      <can-be-disabled-with-tab-index></can-be-disabled-with-tab-index>
+    `);
+    el.disabled = true;
+    expect(el.tabIndex).to.equal(-1);
+    await el.updateComplete;
+    expect(el.getAttribute('tabindex')).to.equal('-1');
+  });
+
+  it('disabled does not override user provided tabindex', async () => {
+    const el = await fixture(html`
+      <can-be-disabled-with-tab-index tabindex="5" disabled></can-be-disabled-with-tab-index>
+    `);
+    expect(el.getAttribute('tabindex')).to.equal('-1');
+    el.disabled = false;
+    await el.updateComplete;
+    expect(el.getAttribute('tabindex')).to.equal('5');
+  });
+
+  it('can be disabled imperatively', async () => {
+    const el = await fixture(html`
+      <can-be-disabled-with-tab-index disabled></can-be-disabled-with-tab-index>
+    `);
+    expect(el.getAttribute('tabindex')).to.equal('-1');
+
+    el.disabled = false;
+    await el.updateComplete;
+    expect(el.getAttribute('tabindex')).to.equal('0');
+    expect(el.hasAttribute('disabled')).to.equal(false);
+
+    el.disabled = true;
+    await el.updateComplete;
+    expect(el.getAttribute('tabindex')).to.equal('-1');
+    expect(el.hasAttribute('disabled')).to.equal(true);
+  });
+
+  it('will not allow to change tabIndex after makeRequestToBeDisabled()', async () => {
+    const el = await fixture(html`
+      <can-be-disabled-with-tab-index></can-be-disabled-with-tab-index>
+    `);
+    el.makeRequestToBeDisabled();
+
+    el.tabIndex = 5;
+    expect(el.tabIndex).to.equal(-1);
+    await el.updateComplete;
+    expect(el.getAttribute('tabindex')).to.equal('-1');
+  });
+
+  it('will restore last tabIndex after retractRequestToBeDisabled()', async () => {
+    const el = await fixture(html`
+      <can-be-disabled-with-tab-index tabindex="5"></can-be-disabled-with-tab-index>
+    `);
+    el.makeRequestToBeDisabled();
+    expect(el.tabIndex).to.equal(-1);
+    await el.updateComplete;
+    expect(el.getAttribute('tabindex')).to.equal('-1');
+    el.retractRequestToBeDisabled();
+    expect(el.tabIndex).to.equal(5);
+    await el.updateComplete;
+    expect(el.getAttribute('tabindex')).to.equal('5');
+
+    el.makeRequestToBeDisabled();
+    el.tabIndex = 12;
+    el.retractRequestToBeDisabled();
+    expect(el.tabIndex).to.equal(12);
+    await el.updateComplete;
+    expect(el.getAttribute('tabindex')).to.equal('12');
+
+    el.makeRequestToBeDisabled();
+    el.tabIndex = 13;
+    el.tabIndex = 14;
+    el.retractRequestToBeDisabled();
+    expect(el.tabIndex).to.equal(14);
+    await el.updateComplete;
+    expect(el.getAttribute('tabindex')).to.equal('14');
+  });
+
+  it('may allow multiple calls to retractRequestToBeDisabled', async () => {
+    const el = await fixture(html`
+      <can-be-disabled-with-tab-index disabled></can-be-disabled-with-tab-index>
+    `);
+    el.retractRequestToBeDisabled();
+    el.retractRequestToBeDisabled();
+    expect(el.disabled).to.be.true;
+    expect(el.tabIndex).to.be.equal(-1);
+  });
+});