@lion/button - npm Package Compare versions

Comparing version 0.2.0 to 0.3.0

CHANGELOG.md

@@ -6,2 +6,20 @@ # Change Log
 
+# [0.3.0](https://github.com/ing-bank/lion/compare/@lion/button@0.2.0...@lion/button@0.3.0) (2019-07-26)
+
+
+### Bug Fixes
+
+* **button:** click event fired twice in IE11 (fix [#179](https://github.com/ing-bank/lion/issues/179)) ([e269b5d](https://github.com/ing-bank/lion/commit/e269b5d))
+* **button:** prevent unnecessary keydown/keyup handlers ([06124e0](https://github.com/ing-bank/lion/commit/06124e0))
+* **button:** remove active when mouse/key up on other element (fix [#210](https://github.com/ing-bank/lion/issues/210)) ([f3303ae](https://github.com/ing-bank/lion/commit/f3303ae))
+
+
+### Features
+
+* **button:** move active to host for cross-browser support (fix [#188](https://github.com/ing-bank/lion/issues/188)) ([471d662](https://github.com/ing-bank/lion/commit/471d662))
+
+
+
+
+
 # [0.2.0](https://github.com/ing-bank/lion/compare/@lion/button@0.1.48...@lion/button@0.2.0) (2019-07-25)
@@ -8,0 +26,0 @@

package.json

 {
   "name": "@lion/button",
-  "version": "0.2.0",
+  "version": "0.3.0",
   "description": "A button that is easily styleable and accessible in all contexts",
@@ -46,3 +46,3 @@ "author": "ing-bank",
   },
-  "gitHead": "33652e5e494b694181f75ef4d6f3256c1af43419"
+  "gitHead": "69d5945c688b99c2e05634fbe4b81f6cc2965d2c"
 }

src/LionButton.js

@@ -13,2 +13,6 @@ import { css, html, DelegateMixin, SlotMixin, DisabledWithTabIndexMixin } from '@lion/core';
       },
+      active: {
+        type: Boolean,
+        reflect: true,
+      },
     };
@@ -24,3 +28,3 @@ }
         <slot name="_button"></slot>
-        <div class="click-area" @click="${this.__clickDelegationHandler}"></div>
+        <div class="click-area"></div>
       </div>
@@ -88,4 +92,4 @@ `;
 
-        :host(:active) .btn,
-        .btn[active] {
+        :host(:active) .btn, /* keep native :active to render quickly where possible */
+        :host([active]) .btn /* use custom [active] to fix IE11 */ {
           /* if you extend, please overwrite */
@@ -134,2 +138,4 @@ background: gray;
     this.role = 'button';
+    this.active = false;
+    this.__setupDelegationInConstructor();
   }
@@ -139,3 +145,3 @@
     super.connectedCallback();
-    this.__setupDelegation();
+    this.__setupEvents();
   }
@@ -145,3 +151,3 @@
     super.disconnectedCallback();
-    this.__teardownDelegation();
+    this.__teardownEvents();
   }
@@ -152,2 +158,3 @@
     const newEvent = new MouseEvent(oldEvent.type, oldEvent);
+    newEvent.__isRedispatchedOnNativeButton = true;
     this.__enforceHostEventTarget(newEvent);
@@ -158,7 +165,9 @@ 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();
-    this._redispatchClickEvent(e);
+    if (!e.__isRedispatchedOnNativeButton) {
+      e.stopImmediatePropagation();
+      this._redispatchClickEvent(e);
+    }
   }
@@ -176,28 +185,54 @@
 
-  __setupDelegation() {
-    this.addEventListener('keydown', this.__keydownDelegationHandler);
-    this.addEventListener('keyup', this.__keyupDelegationHandler);
+  __setupDelegationInConstructor() {
+    // do not move to connectedCallback, otherwise IE11 breaks
+    // more info: https://github.com/ing-bank/lion/issues/179#issuecomment-511763835
+    this.addEventListener('click', this.__clickDelegationHandler, true);
   }
 
-  __teardownDelegation() {
-    this.removeEventListener('keydown', this.__keydownDelegationHandler);
-    this.removeEventListener('keyup', this.__keyupDelegationHandler);
+  __setupEvents() {
+    this.addEventListener('mousedown', this.__mousedownHandler);
+    this.addEventListener('keydown', this.__keydownHandler);
+    this.addEventListener('keyup', this.__keyupHandler);
   }
 
-  __keydownDelegationHandler(e) {
-    if (e.keyCode === 32 /* space */ || e.keyCode === 13 /* enter */) {
-      e.preventDefault();
-      this.shadowRoot.querySelector('.btn').setAttribute('active', '');
+  __teardownEvents() {
+    this.removeEventListener('mousedown', this.__mousedownHandler);
+    this.removeEventListener('keydown', this.__keydownHandler);
+    this.removeEventListener('keyup', this.__keyupHandler);
+  }
+
+  __mousedownHandler() {
+    this.active = true;
+    const mouseupHandler = () => {
+      this.active = false;
+      document.removeEventListener('mouseup', mouseupHandler);
+    };
+    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);
   }
 
-  __keyupDelegationHandler(e) {
-    // Makes the real button the trigger in forms (will submit form, as opposed to paper-button)
-    // and make click handlers on button work on space and enter
-    if (e.keyCode === 32 /* space */ || e.keyCode === 13 /* enter */) {
-      e.preventDefault();
-      this.shadowRoot.querySelector('.btn').removeAttribute('active');
+  __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 */;
+  }
 }

stories/index.stories.js

@@ -28,3 +28,5 @@ import { storiesOf, html } from '@open-wc/demoing-storybook';
         <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>
@@ -31,0 +33,0 @@ </div>

test/lion-button.test.js

@@ -7,2 +7,6 @@ import { expect, fixture, html, aTimeout, oneEvent } from '@open-wc/testing';
   pressSpace,
+  down,
+  up,
+  keyDownOn,
+  keyUpOn,
 } from '@polymer/iron-test-helpers/mock-interactions.js';
@@ -13,6 +17,7 @@
 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;
-  return crossBrowserRoot.elementFromPoint(left, top);
+  const crossBrowserRoot =
+    el.shadowRoot && el.shadowRoot.elementFromPoint ? el.shadowRoot : document;
+  return crossBrowserRoot.elementFromPoint(left + width / 2, top + height / 2);
 }
@@ -61,2 +66,94 @@
 
+  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', () => {
@@ -157,3 +254,3 @@ it('has a role="button" by default', async () => {
         html`
-          <lion-button @click="${clickSpy}"></lion-button>
+          <lion-button @click="${clickSpy}">foo</lion-button>
         `,
@@ -171,12 +268,6 @@ );
 
-    describe('event after redispatching', async () => {
-      async function prepareClickEvent(el, host) {
+    describe('native button behavior', async () => {
+      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));
-          }
+          makeMouseEvent('click', { x: 11, y: 11 }, getTopElement(el));
         });
@@ -186,9 +277,10 @@ return oneEvent(el, 'click');
 
-      let hostEvent;
-      let redispatchedEvent;
+      let nativeButtonEvent;
+      let lionButtonEvent;
 
       before(async () => {
-        const el = await fixture('<lion-button></lion-button>');
-        hostEvent = await prepareClickEvent(el, true);
-        redispatchedEvent = await prepareClickEvent(el, false);
+        const nativeButtonEl = await fixture('<button>foo</button>');
+        const lionButtonEl = await fixture('<lion-button>foo</lion-button>');
+        nativeButtonEvent = await prepareClickEvent(nativeButtonEl);
+        lionButtonEvent = await prepareClickEvent(lionButtonEl);
       });
@@ -203,12 +295,17 @@
         'clientY',
-        'target',
       ];
 
       sameProperties.forEach(property => {
-        it(`has same value of the property "${property}"`, async () => {
-          expect(redispatchedEvent[property]).to.equal(hostEvent[property]);
+        it(`has same value of the property "${property}" as in native button event`, () => {
+          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);
+      });
     });
   });
 });

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

32835

increased by18.58%

Lines of code

502

increased by28.72%

No dependency changes