fix(button): remove active when mouse/key up on other element (fix #210)

Author: Mikhail Bashkirov

packages/button/src/LionButton.js

@@ -138,12 +138,12 @@ export class LionButton extends DisabledWithTabIndexMixin(
 
   connectedCallback() {
     super.connectedCallback();
-    this.__setupDelegation();
+    this.__setupEvents();
   }
 
   disconnectedCallback() {
     super.disconnectedCallback();
-    this.__teardownDelegation();
+    this.__teardownEvents();
   }
 
   _redispatchClickEvent(oldEvent) {
@@ -180,42 +180,50 @@ export class LionButton extends DisabledWithTabIndexMixin(
     this.addEventListener('click', this.__clickDelegationHandler, true);
   }
 
-  __setupDelegation() {
-    this.addEventListener('mousedown', this.__mousedownDelegationHandler);
-    this.addEventListener('mouseup', this.__mouseupDelegationHandler);
-    this.addEventListener('keydown', this.__keydownDelegationHandler);
-    this.addEventListener('keyup', this.__keyupDelegationHandler);
+  __setupEvents() {
+    this.addEventListener('mousedown', this.__mousedownHandler);
+    this.addEventListener('keydown', this.__keydownHandler);
+    this.addEventListener('keyup', this.__keyupHandler);
   }
 
-  __teardownDelegation() {
-    this.removeEventListener('mousedown', this.__mousedownDelegationHandler);
-    this.removeEventListener('mouseup', this.__mouseupDelegationHandler);
-    this.removeEventListener('keydown', this.__keydownDelegationHandler);
-    this.removeEventListener('keyup', this.__keyupDelegationHandler);
+  __teardownEvents() {
+    this.removeEventListener('mousedown', this.__mousedownHandler);
+    this.removeEventListener('keydown', this.__keydownHandler);
+    this.removeEventListener('keyup', this.__keyupHandler);
   }
 
-  __mousedownDelegationHandler() {
+  __mousedownHandler() {
     this.active = true;
+    const mouseupHandler = () => {
+      this.active = false;
+      document.removeEventListener('mouseup', mouseupHandler);
+    };
+    document.addEventListener('mouseup', mouseupHandler);
   }
 
-  __mouseupDelegationHandler() {
-    this.active = false;
-  }
-
-  __keydownDelegationHandler(e) {
-    if (e.keyCode === 32 /* space */ || e.keyCode === 13 /* enter */) {
-      e.preventDefault();
-      this.active = true;
+  __keydownHandler(e) {
+    if (!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.active = false;
+  __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/test/lion-button.test.js

@@ -77,6 +77,21 @@ describe('lion-button', () => {
       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);
@@ -92,6 +107,21 @@ describe('lion-button', () => {
       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);
@@ -106,6 +136,21 @@ describe('lion-button', () => {
       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', () => {