From f3303ae014254e6043e1d88cfc809b68cc698d91 Mon Sep 17 00:00:00 2001 From: Mikhail Bashkirov Date: Fri, 26 Jul 2019 16:19:01 +0200 Subject: [PATCH] fix(button): remove active when mouse/key up on other element (fix #210) --- packages/button/src/LionButton.js | 70 +++++++++++++----------- packages/button/test/lion-button.test.js | 45 +++++++++++++++ 2 files changed, 84 insertions(+), 31 deletions(-) diff --git a/packages/button/src/LionButton.js b/packages/button/src/LionButton.js index 23f027799..4f8823c43 100644 --- a/packages/button/src/LionButton.js +++ b/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; - } - - __mouseupDelegationHandler() { - this.active = false; - } - - __keydownDelegationHandler(e) { - if (e.keyCode === 32 /* space */ || e.keyCode === 13 /* enter */) { - e.preventDefault(); - this.active = 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(); + const mouseupHandler = () => { this.active = false; + document.removeEventListener('mouseup', mouseupHandler); + }; + document.addEventListener('mouseup', mouseupHandler); + } + + __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); + } + + __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 */; + } } diff --git a/packages/button/test/lion-button.test.js b/packages/button/test/lion-button.test.js index cb2c15580..0b6792aa6 100644 --- a/packages/button/test/lion-button.test.js +++ b/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(`foo`); + 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(`foo`); 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(`foo`); + 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(`foo`); 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(`foo`); + 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', () => {