Merge pull request #208 from ing-bank/fix/buttonTwiceClickIE11

[button] Fix IE11 bugs or bugs introduced by fixes for IE11

packages/button/src/LionButton.js

@@ -10,6 +10,10 @@ export class LionButton extends DisabledWithTabIndexMixin(
         type: String,
         reflect: true,
       },
+      active: {
+        type: Boolean,
+        reflect: true,
+      },
     };
   }
 
@@ -20,7 +24,7 @@ export class LionButton extends DisabledWithTabIndexMixin(
         <slot></slot>
         ${this._renderAfter()}
         <slot name="_button"></slot>
-        <div class="click-area" @click="${this.__clickDelegationHandler}"></div>
+        <div class="click-area"></div>
       </div>
     `;
   }
@@ -83,8 +87,8 @@ export class LionButton extends DisabledWithTabIndexMixin(
           background: #f4f6f7;
         }
 
-        :host(:active) .btn,
+        :host(:active) .btn, /* keep native :active to render quickly where possible */
-        .btn[active] {
+        :host([active]) .btn /* use custom [active] to fix IE11 */ {
           /* if you extend, please overwrite */
           background: gray;
         }
@@ -128,32 +132,37 @@ export class LionButton extends DisabledWithTabIndexMixin(
   constructor() {
     super();
     this.role = 'button';
+    this.active = false;
+    this.__setupDelegationInConstructor();
   }
 
   connectedCallback() {
     super.connectedCallback();
-    this.__setupDelegation();
+    this.__setupEvents();
   }
 
   disconnectedCallback() {
     super.disconnectedCallback();
-    this.__teardownDelegation();
+    this.__teardownEvents();
   }
 
   _redispatchClickEvent(oldEvent) {
     // replacing `MouseEvent` with `oldEvent.constructor` breaks IE
     const newEvent = new MouseEvent(oldEvent.type, oldEvent);
+    newEvent.__isRedispatchedOnNativeButton = true;
     this.__enforceHostEventTarget(newEvent);
     this.$$slot('_button').dispatchEvent(newEvent);
   }
 
   /**
-   * Prevent click on the fake element and cause click on the native button.
+   * Prevent normal click and redispatch click on the native button unless already redispatched.
    */
   __clickDelegationHandler(e) {
-    e.stopPropagation();
+    if (!e.__isRedispatchedOnNativeButton) {
+      e.stopImmediatePropagation();
       this._redispatchClickEvent(e);
     }
+  }
 
   __enforceHostEventTarget(event) {
     try {
@@ -165,30 +174,56 @@ export class LionButton extends DisabledWithTabIndexMixin(
     }
   }
 
-  __setupDelegation() {
+  __setupDelegationInConstructor() {
-    this.addEventListener('keydown', this.__keydownDelegationHandler);
+    // do not move to connectedCallback, otherwise IE11 breaks
-    this.addEventListener('keyup', this.__keyupDelegationHandler);
+    // more info: https://github.com/ing-bank/lion/issues/179#issuecomment-511763835
+    this.addEventListener('click', this.__clickDelegationHandler, true);
   }
 
-  __teardownDelegation() {
+  __setupEvents() {
-    this.removeEventListener('keydown', this.__keydownDelegationHandler);
+    this.addEventListener('mousedown', this.__mousedownHandler);
-    this.removeEventListener('keyup', this.__keyupDelegationHandler);
+    this.addEventListener('keydown', this.__keydownHandler);
+    this.addEventListener('keyup', this.__keyupHandler);
   }
 
-  __keydownDelegationHandler(e) {
+  __teardownEvents() {
-    if (e.keyCode === 32 /* space */ || e.keyCode === 13 /* enter */) {
+    this.removeEventListener('mousedown', this.__mousedownHandler);
-      e.preventDefault();
+    this.removeEventListener('keydown', this.__keydownHandler);
-      this.shadowRoot.querySelector('.btn').setAttribute('active', '');
+    this.removeEventListener('keyup', this.__keyupHandler);
-    }
   }
 
-  __keyupDelegationHandler(e) {
+  __mousedownHandler() {
-    // Makes the real button the trigger in forms (will submit form, as opposed to paper-button)
+    this.active = true;
-    // and make click handlers on button work on space and enter
+    const mouseupHandler = () => {
-    if (e.keyCode === 32 /* space */ || e.keyCode === 13 /* enter */) {
+      this.active = false;
-      e.preventDefault();
+      document.removeEventListener('mouseup', mouseupHandler);
-      this.shadowRoot.querySelector('.btn').removeAttribute('active');
+    };
+    document.addEventListener('mouseup', mouseupHandler);
+  }
+
+  __keydownHandler(e) {
+    if (this.active || !this.__isKeyboardClickEvent(e)) {
+      return;
+    }
+    this.active = true;
+    const keyupHandler = keyupEvent => {
+      if (this.__isKeyboardClickEvent(keyupEvent)) {
+        this.active = false;
+        document.removeEventListener('keyup', keyupHandler, true);
+      }
+    };
+    document.addEventListener('keyup', keyupHandler, true);
+  }
+
+  __keyupHandler(e) {
+    if (this.__isKeyboardClickEvent(e)) {
+      // redispatch click
       this.shadowRoot.querySelector('.click-area').click();
     }
   }
+
+  // eslint-disable-next-line class-methods-use-this
+  __isKeyboardClickEvent(e) {
+    return e.keyCode === 32 /* space */ || e.keyCode === 13 /* enter */;
+  }
 }

packages/button/stories/index.stories.js

@@ -25,7 +25,9 @@ storiesOf('Buttons|Button', module)
         <lion-button><lion-icon .svg="${bug12}"></lion-icon>Debug</lion-button>
         <lion-button type="submit">Submit</lion-button>
         <lion-button aria-label="Debug"><lion-icon .svg="${bug12}"></lion-icon></lion-button>
-        <lion-button onclick="alert('clicked/spaced/entered')">click/space/enter me</lion-button>
+        <lion-button @click="${e => console.log('clicked/spaced/entered', e)}">
+          click/space/enter me and see log
+        </lion-button>
         <lion-button disabled>Disabled</lion-button>
       </div>
     `,

packages/button/test/lion-button.test.js

@@ -4,15 +4,20 @@ import {
   makeMouseEvent,
   pressEnter,
   pressSpace,
+  down,
+  up,
+  keyDownOn,
+  keyUpOn,
 } from '@polymer/iron-test-helpers/mock-interactions.js';
 
 import '../lion-button.js';
 
 function getTopElement(el) {
-  const { left, top } = el.getBoundingClientRect();
+  const { left, top, width, height } = el.getBoundingClientRect();
   // to support elementFromPoint() in polyfilled browsers we have to use document
-  const crossBrowserRoot = el.shadowRoot.elementFromPoint ? el.shadowRoot : document;
+  const crossBrowserRoot =
-  return crossBrowserRoot.elementFromPoint(left, top);
+    el.shadowRoot && el.shadowRoot.elementFromPoint ? el.shadowRoot : document;
+  return crossBrowserRoot.elementFromPoint(left + width / 2, top + height / 2);
 }
 
 describe('lion-button', () => {
@@ -56,6 +61,98 @@ describe('lion-button', () => {
     expect(el.hasAttribute('disabled')).to.equal(true);
   });
 
+  describe('active', () => {
+    it('updates "active" attribute on host when mousedown/mouseup on button', async () => {
+      const el = await fixture(`<lion-button>foo</lion-button>`);
+      const topEl = getTopElement(el);
+
+      down(topEl);
+      expect(el.active).to.be.true;
+      await el.updateComplete;
+      expect(el.hasAttribute('active')).to.be.true;
+
+      up(topEl);
+      expect(el.active).to.be.false;
+      await el.updateComplete;
+      expect(el.hasAttribute('active')).to.be.false;
+    });
+
+    it('updates "active" attribute on host when mousedown on button and mouseup anywhere else', async () => {
+      const el = await fixture(`<lion-button>foo</lion-button>`);
+      const topEl = getTopElement(el);
+
+      down(topEl);
+      expect(el.active).to.be.true;
+      await el.updateComplete;
+      expect(el.hasAttribute('active')).to.be.true;
+
+      up(document.body);
+      expect(el.active).to.be.false;
+      await el.updateComplete;
+      expect(el.hasAttribute('active')).to.be.false;
+    });
+
+    it('updates "active" attribute on host when space keydown/keyup on button', async () => {
+      const el = await fixture(`<lion-button>foo</lion-button>`);
+      const topEl = getTopElement(el);
+
+      keyDownOn(topEl, 32);
+      expect(el.active).to.be.true;
+      await el.updateComplete;
+      expect(el.hasAttribute('active')).to.be.true;
+
+      keyUpOn(topEl, 32);
+      expect(el.active).to.be.false;
+      await el.updateComplete;
+      expect(el.hasAttribute('active')).to.be.false;
+    });
+
+    it('updates "active" attribute on host when space keydown on button and space keyup anywhere else', async () => {
+      const el = await fixture(`<lion-button>foo</lion-button>`);
+      const topEl = getTopElement(el);
+
+      keyDownOn(topEl, 32);
+      expect(el.active).to.be.true;
+      await el.updateComplete;
+      expect(el.hasAttribute('active')).to.be.true;
+
+      keyUpOn(document.body, 32);
+      expect(el.active).to.be.false;
+      await el.updateComplete;
+      expect(el.hasAttribute('active')).to.be.false;
+    });
+
+    it('updates "active" attribute on host when enter keydown/keyup on button', async () => {
+      const el = await fixture(`<lion-button>foo</lion-button>`);
+      const topEl = getTopElement(el);
+
+      keyDownOn(topEl, 13);
+      expect(el.active).to.be.true;
+      await el.updateComplete;
+      expect(el.hasAttribute('active')).to.be.true;
+
+      keyUpOn(topEl, 13);
+      expect(el.active).to.be.false;
+      await el.updateComplete;
+      expect(el.hasAttribute('active')).to.be.false;
+    });
+
+    it('updates "active" attribute on host when enter keydown on button and space keyup anywhere else', async () => {
+      const el = await fixture(`<lion-button>foo</lion-button>`);
+      const topEl = getTopElement(el);
+
+      keyDownOn(topEl, 13);
+      expect(el.active).to.be.true;
+      await el.updateComplete;
+      expect(el.hasAttribute('active')).to.be.true;
+
+      keyUpOn(document.body, 13);
+      expect(el.active).to.be.false;
+      await el.updateComplete;
+      expect(el.hasAttribute('active')).to.be.false;
+    });
+  });
+
   describe('a11y', () => {
     it('has a role="button" by default', async () => {
       const el = await fixture(`<lion-button>foo</lion-button>`);
@@ -151,7 +248,7 @@ describe('lion-button', () => {
       const clickSpy = sinon.spy();
       const el = await fixture(
         html`
-          <lion-button @click="${clickSpy}"></lion-button>
+          <lion-button @click="${clickSpy}">foo</lion-button>
         `,
       );
 
@@ -164,27 +261,22 @@ describe('lion-button', () => {
       expect(clickSpy.callCount).to.equal(1);
     });
 
-    describe('event after redispatching', async () => {
+    describe('native button behavior', async () => {
-      async function prepareClickEvent(el, host) {
+      async function prepareClickEvent(el) {
         setTimeout(() => {
-          if (host) {
-            // click on host like in native button
-            makeMouseEvent('click', { x: 11, y: 11 }, el);
-          } else {
-            // click on click-area which is then redispatched
           makeMouseEvent('click', { x: 11, y: 11 }, getTopElement(el));
-          }
         });
         return oneEvent(el, 'click');
       }
 
-      let hostEvent;
+      let nativeButtonEvent;
-      let redispatchedEvent;
+      let lionButtonEvent;
 
       before(async () => {
-        const el = await fixture('<lion-button></lion-button>');
+        const nativeButtonEl = await fixture('<button>foo</button>');
-        hostEvent = await prepareClickEvent(el, true);
+        const lionButtonEl = await fixture('<lion-button>foo</lion-button>');
-        redispatchedEvent = await prepareClickEvent(el, false);
+        nativeButtonEvent = await prepareClickEvent(nativeButtonEl);
+        lionButtonEvent = await prepareClickEvent(lionButtonEl);
       });
 
       const sameProperties = [
@@ -194,13 +286,18 @@ describe('lion-button', () => {
         'cancelable',
         'clientX',
         'clientY',
-        'target',
       ];
 
       sameProperties.forEach(property => {
-        it(`has same value of the property "${property}"`, async () => {
+        it(`has same value of the property "${property}" as in native button event`, () => {
-          expect(redispatchedEvent[property]).to.equal(hostEvent[property]);
+          expect(lionButtonEvent[property]).to.equal(nativeButtonEvent[property]);
-        });
+        });
+      });
+
+      it('has host in the target property', async () => {
+        const el = await fixture('<lion-button>foo</lion-button>');
+        const event = await prepareClickEvent(el);
+        expect(event.target).to.equal(el);
       });
     });
   });

packages/popup/test/lion-popup.test.js

@@ -22,12 +22,11 @@ describe('lion-popup', () => {
         </lion-popup>
       `);
       const invoker = el.querySelector('[slot="invoker"]');
-      const eventOnClick = new Event('click');
+      invoker.click();
-      invoker.dispatchEvent(eventOnClick);
       await el.updateComplete;
 
       expect(el.querySelector('[slot="content"]').style.display).to.be.equal('inline-block');
-      invoker.dispatchEvent(eventOnClick);
+      invoker.click();
       await el.updateComplete;
       expect(el.querySelector('[slot="content"]').style.display).to.be.equal('none');
     });