From 4a8c6ebbed3d878185bb3432dec7f62eda1dc097 Mon Sep 17 00:00:00 2001 From: Mikhail Bashkirov Date: Fri, 12 Jul 2019 14:02:01 +0200 Subject: [PATCH 1/4] fix(button): remove unnecessary instance method bind --- packages/button/src/LionButton.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/button/src/LionButton.js b/packages/button/src/LionButton.js index 347419f26..af4b6ca23 100644 --- a/packages/button/src/LionButton.js +++ b/packages/button/src/LionButton.js @@ -140,7 +140,6 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) { this.disabled = false; this.role = 'button'; this.tabindex = 0; - this.__keydownDelegationHandler = this.__keydownDelegationHandler.bind(this); } connectedCallback() { From b71177f6679c9f69f5e53ceaa7386e7c01f81f4c Mon Sep 17 00:00:00 2001 From: Mikhail Bashkirov Date: Fri, 12 Jul 2019 14:31:41 +0200 Subject: [PATCH 2/4] fix(button): redispatch click event with all original properties --- packages/button/src/LionButton.js | 13 ++-- packages/button/test/lion-button.test.js | 79 +++++++++++++++++++++--- 2 files changed, 81 insertions(+), 11 deletions(-) diff --git a/packages/button/src/LionButton.js b/packages/button/src/LionButton.js index af4b6ca23..349d4e69d 100644 --- a/packages/button/src/LionButton.js +++ b/packages/button/src/LionButton.js @@ -152,9 +152,14 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) { this.__teardownDelegation(); } - __clickDelegationHandler(e) { - e.stopPropagation(); // prevent click on the fake element and cause click on the native button - this.$$slot('_button').click(); + /** + * Prevent click on the fake element and cause click on the native button. + */ + __clickDelegationHandler(oldEvent) { + oldEvent.stopPropagation(); + // replacing `MouseEvent` with `oldEvent.constructor` breaks IE + const newEvent = new MouseEvent(oldEvent.type, oldEvent); + this.$$slot('_button').dispatchEvent(newEvent); } __setupDelegation() { @@ -180,7 +185,7 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) { if (e.keyCode === 32 /* space */ || e.keyCode === 13 /* enter */) { e.preventDefault(); this.shadowRoot.querySelector('.btn').removeAttribute('active'); - this.$$slot('_button').click(); + this.shadowRoot.querySelector('.click-area').click(); } } diff --git a/packages/button/test/lion-button.test.js b/packages/button/test/lion-button.test.js index 76eaca804..44c76af5c 100644 --- a/packages/button/test/lion-button.test.js +++ b/packages/button/test/lion-button.test.js @@ -1,9 +1,20 @@ -import { expect, fixture, html, aTimeout } from '@open-wc/testing'; +import { expect, fixture, html, aTimeout, oneEvent } from '@open-wc/testing'; import sinon from 'sinon'; -import { pressEnter, pressSpace } from '@polymer/iron-test-helpers/mock-interactions.js'; +import { + makeMouseEvent, + pressEnter, + pressSpace, +} from '@polymer/iron-test-helpers/mock-interactions.js'; import '../lion-button.js'; +function getTopElement(el) { + const { left, top } = 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); +} + describe('lion-button', () => { it('behaves like native `button` in terms of a11y', async () => { const el = await fixture(`foo`); @@ -99,11 +110,7 @@ describe('lion-button', () => { `); const button = form.querySelector('lion-button'); - const { left, top } = button.getBoundingClientRect(); - // to support elementFromPoint() in polyfilled browsers we have to use document - const crossBrowserRoot = button.shadowRoot.elementFromPoint ? button.shadowRoot : document; - const shadowClickAreaElement = crossBrowserRoot.elementFromPoint(left, top); - shadowClickAreaElement.click(); + getTopElement(button).click(); expect(formSubmitSpy.called).to.be.true; }); @@ -138,4 +145,62 @@ describe('lion-button', () => { expect(formSubmitSpy.called).to.be.true; }); }); + + describe('click event', () => { + it('is fired once', async () => { + const clickSpy = sinon.spy(); + const el = await fixture( + html` + + `, + ); + + getTopElement(el).click(); + + // trying to wait for other possible redispatched events + await aTimeout(); + await aTimeout(); + + expect(clickSpy.callCount).to.equal(1); + }); + + describe('event after redispatching', async () => { + async function prepareClickEvent(el, host) { + 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 redispatchedEvent; + + before(async () => { + const el = await fixture(''); + hostEvent = await prepareClickEvent(el, true); + redispatchedEvent = await prepareClickEvent(el, false); + }); + + const sameProperties = [ + 'constructor', + 'composed', + 'bubbles', + 'cancelable', + 'clientX', + 'clientY', + ]; + + sameProperties.forEach(property => { + it(`has same value of the property "${property}"`, async () => { + expect(redispatchedEvent[property]).to.equal(hostEvent[property]); + }); + }); + }); + }); }); From 59e456e11f9dce5ca8b83f3b0849f286788d0ffb Mon Sep 17 00:00:00 2001 From: Mikhail Bashkirov Date: Fri, 12 Jul 2019 15:29:56 +0200 Subject: [PATCH 3/4] fix(button): put host element into click event target (fix #89) --- packages/button/src/LionButton.js | 11 +++++++++++ packages/button/test/lion-button.test.js | 1 + 2 files changed, 12 insertions(+) diff --git a/packages/button/src/LionButton.js b/packages/button/src/LionButton.js index 349d4e69d..dc49ef5b3 100644 --- a/packages/button/src/LionButton.js +++ b/packages/button/src/LionButton.js @@ -159,9 +159,20 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) { oldEvent.stopPropagation(); // replacing `MouseEvent` with `oldEvent.constructor` breaks IE const newEvent = new MouseEvent(oldEvent.type, oldEvent); + this.__enforceHostEventTarget(newEvent); this.$$slot('_button').dispatchEvent(newEvent); } + __enforceHostEventTarget(event) { + try { + // this is for IE11 (and works in others), because `Object.defineProperty` does not give any effect there + event.__defineGetter__('target', () => this); // eslint-disable-line no-restricted-properties + } catch (error) { + // in case `__defineGetter__` is removed from the platform + Object.defineProperty(event, 'target', { writable: false, value: this }); + } + } + __setupDelegation() { this.addEventListener('keydown', this.__keydownDelegationHandler); this.addEventListener('keyup', this.__keyupDelegationHandler); diff --git a/packages/button/test/lion-button.test.js b/packages/button/test/lion-button.test.js index 44c76af5c..1ad5334b1 100644 --- a/packages/button/test/lion-button.test.js +++ b/packages/button/test/lion-button.test.js @@ -194,6 +194,7 @@ describe('lion-button', () => { 'cancelable', 'clientX', 'clientY', + 'target', ]; sameProperties.forEach(property => { From b7eb537d1de992a43a4d2fecb0a1115a78c3b1be Mon Sep 17 00:00:00 2001 From: Mikhail Bashkirov Date: Fri, 12 Jul 2019 15:52:28 +0200 Subject: [PATCH 4/4] fix(button): make redispatch method protected --- packages/button/src/LionButton.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/button/src/LionButton.js b/packages/button/src/LionButton.js index dc49ef5b3..196b48008 100644 --- a/packages/button/src/LionButton.js +++ b/packages/button/src/LionButton.js @@ -152,17 +152,21 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) { this.__teardownDelegation(); } - /** - * Prevent click on the fake element and cause click on the native button. - */ - __clickDelegationHandler(oldEvent) { - oldEvent.stopPropagation(); + _redispatchClickEvent(oldEvent) { // replacing `MouseEvent` with `oldEvent.constructor` breaks IE const newEvent = new MouseEvent(oldEvent.type, oldEvent); this.__enforceHostEventTarget(newEvent); this.$$slot('_button').dispatchEvent(newEvent); } + /** + * Prevent click on the fake element and cause click on the native button. + */ + __clickDelegationHandler(e) { + e.stopPropagation(); + this._redispatchClickEvent(e); + } + __enforceHostEventTarget(event) { try { // this is for IE11 (and works in others), because `Object.defineProperty` does not give any effect there