From a21121b14bc45cdf2c5732bbd028c1a01caff71d Mon Sep 17 00:00:00 2001 From: Pierre-Marie Dartus Date: Tue, 22 May 2018 16:53:36 -0700 Subject: [PATCH] fix: event retargeting issue with nested component (#338) * fix: event retrageting issue with nested component * test(engine): integration test for shadow root retargeting * docs: add back removed comment --- .../src/framework/__tests__/events.spec.ts | 38 +++++++++++++++++++ packages/lwc-engine/src/framework/root.ts | 35 +++++++++++++++-- .../root-listener-event-target.spec.js | 17 +++++++++ .../root-listener-event-target.html | 3 ++ .../root-listener-event-target.js | 5 +++ .../x-child/x-child.html | 4 ++ .../x-child/x-child.js | 11 ++++++ .../x-grand-child/x-grand-child.html | 3 ++ .../x-grand-child/x-grand-child.js | 5 +++ 9 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 packages/lwc-integration/src/components/events/test-root-listener-event-target/root-listener-event-target.spec.js create mode 100644 packages/lwc-integration/src/components/events/test-root-listener-event-target/root-listener-event-target/root-listener-event-target.html create mode 100644 packages/lwc-integration/src/components/events/test-root-listener-event-target/root-listener-event-target/root-listener-event-target.js create mode 100644 packages/lwc-integration/src/components/events/test-root-listener-event-target/x-child/x-child.html create mode 100644 packages/lwc-integration/src/components/events/test-root-listener-event-target/x-child/x-child.js create mode 100644 packages/lwc-integration/src/components/events/test-root-listener-event-target/x-grand-child/x-grand-child.html create mode 100644 packages/lwc-integration/src/components/events/test-root-listener-event-target/x-grand-child/x-grand-child.js diff --git a/packages/lwc-engine/src/framework/__tests__/events.spec.ts b/packages/lwc-engine/src/framework/__tests__/events.spec.ts index 5db42a3fd0..2b696f2d49 100644 --- a/packages/lwc-engine/src/framework/__tests__/events.spec.ts +++ b/packages/lwc-engine/src/framework/__tests__/events.spec.ts @@ -746,6 +746,44 @@ describe('Shadow Root events', () => { elm.clickChildDiv(); }); }); + + it('should retarget properly event listener attached on non-root components', () => { + expect.assertions(2); + + class GrandChild extends Element {} + + class Child extends Element { + connectedCallback() { + this.template.addEventListener('click', evt => { + expect(evt.target.tagName).toBe('X-GRAND-CHILD'); + expect(evt.currentTarget.tagName).toBe('X-CHILD'); + }); + } + + render() { + return $api => { + return [ + $api.c('x-grand-child', GrandChild, {}) + ]; + }; + } + } + + class Root extends Element { + render() { + return $api => { + return [ + $api.c('x-child', Child, {}) + ]; + }; + } + } + + const elm = createElement('x-root', { is: Root }); + document.body.appendChild(elm); + + HTMLElement.prototype.querySelector.call(elm, 'x-grand-child').click(); + }); }); describe('Removing events from shadowroot', () => { diff --git a/packages/lwc-engine/src/framework/root.ts b/packages/lwc-engine/src/framework/root.ts index c58aae3056..9cd44b455d 100644 --- a/packages/lwc-engine/src/framework/root.ts +++ b/packages/lwc-engine/src/framework/root.ts @@ -283,23 +283,52 @@ register({ // will kick in and return the cmp, which is not the intent. return callback(pierce(value)); case 'target': + // For event listener attached: + // * on the component => `target` must always equal to `currentTarget` + // * on the root => `target` is retargeted + // * on an element inside template => `target` is retargeted + // * on the root component of the component tree via addEventListener + // => don't do any retargeting + // + // Slotted events should not be handled, since those are part of the light DOM. + const { currentTarget } = event; + // Executing event listener on component, target is always currentTarget if (componentEventListenerType === EventListenerContext.COMPONENT_LISTENER) { return callback(pierce(currentTarget)); } - // Event is coming from an slotted element + // Handle events is coming from an slotted elements. + // TODO: Add more information why we need to handle the light DOM events here. if (isChildNode(getRootNode.call(value, GET_ROOT_NODE_CONFIG_FALSE), currentTarget as Element)) { return; } - // target is owned by the VM - const vm = currentTarget ? getElementOwnerVM(currentTarget as Element) : undefined; + + let vm: VM | undefined; + if (componentEventListenerType === EventListenerContext.ROOT_LISTENER) { + // If we are in an event listener attached on the shadow root, then we do not want to look + // for the currentTarget owner VM because the currentTarget owner VM would be the VM which + // rendered the component (parent component). + // + // Instead, we want to get the custom element's VM because that VM owns the shadow root itself. + vm = getCustomElementVM(currentTarget as HTMLElement); + } else if (!isUndefined(currentTarget)) { + // TODO: When does currentTarget can be undefined + vm = getElementOwnerVM(currentTarget as Element); + } + + // Handle case when VM is not present for example when attaching an event listener + // on the root component of the component tree. if (!isUndefined(vm)) { let node = value; + + // Let's climb up the node tree starting from the original event target + // up until finding the first node being rendered by the current VM. while (!isNull(node) && vm.uid !== node[OwnerKey]) { node = node.parentNode; } + return callback(pierce(node)); } } diff --git a/packages/lwc-integration/src/components/events/test-root-listener-event-target/root-listener-event-target.spec.js b/packages/lwc-integration/src/components/events/test-root-listener-event-target/root-listener-event-target.spec.js new file mode 100644 index 0000000000..6b954a55a9 --- /dev/null +++ b/packages/lwc-integration/src/components/events/test-root-listener-event-target/root-listener-event-target.spec.js @@ -0,0 +1,17 @@ +const assert = require('assert'); + +describe('Event target in slot elements', () => { + const URL = 'http://localhost:4567/root-listener-event-target/'; + + before(() => { + browser.url(URL); + }); + + it('should receive event with correct target', function () { + const element = browser.element('x-grand-child'); + element.click(); + + const verifyElement = browser.element('.event-target-correct'); + assert.strictEqual(verifyElement.getText(), 'Event Target is correct element'); + }); +}); diff --git a/packages/lwc-integration/src/components/events/test-root-listener-event-target/root-listener-event-target/root-listener-event-target.html b/packages/lwc-integration/src/components/events/test-root-listener-event-target/root-listener-event-target/root-listener-event-target.html new file mode 100644 index 0000000000..bbabacf0e3 --- /dev/null +++ b/packages/lwc-integration/src/components/events/test-root-listener-event-target/root-listener-event-target/root-listener-event-target.html @@ -0,0 +1,3 @@ + diff --git a/packages/lwc-integration/src/components/events/test-root-listener-event-target/root-listener-event-target/root-listener-event-target.js b/packages/lwc-integration/src/components/events/test-root-listener-event-target/root-listener-event-target/root-listener-event-target.js new file mode 100644 index 0000000000..bf76aeee4e --- /dev/null +++ b/packages/lwc-integration/src/components/events/test-root-listener-event-target/root-listener-event-target/root-listener-event-target.js @@ -0,0 +1,5 @@ +import { Element } from 'engine'; + +export default class RootListenerEventTarget extends Element { + +} diff --git a/packages/lwc-integration/src/components/events/test-root-listener-event-target/x-child/x-child.html b/packages/lwc-integration/src/components/events/test-root-listener-event-target/x-child/x-child.html new file mode 100644 index 0000000000..05ab71cfcd --- /dev/null +++ b/packages/lwc-integration/src/components/events/test-root-listener-event-target/x-child/x-child.html @@ -0,0 +1,4 @@ + diff --git a/packages/lwc-integration/src/components/events/test-root-listener-event-target/x-child/x-child.js b/packages/lwc-integration/src/components/events/test-root-listener-event-target/x-child/x-child.js new file mode 100644 index 0000000000..1fd8eca083 --- /dev/null +++ b/packages/lwc-integration/src/components/events/test-root-listener-event-target/x-child/x-child.js @@ -0,0 +1,11 @@ +import { Element, track } from 'engine'; + +export default class Child extends Element { + @track eventTargetIsCorrect = false; + + connectedCallback() { + this.template.addEventListener('click', (evt) => { + this.eventTargetIsCorrect = evt.target.tagName === 'X-GRAND-CHILD'; + }); + } +} diff --git a/packages/lwc-integration/src/components/events/test-root-listener-event-target/x-grand-child/x-grand-child.html b/packages/lwc-integration/src/components/events/test-root-listener-event-target/x-grand-child/x-grand-child.html new file mode 100644 index 0000000000..a0927f9075 --- /dev/null +++ b/packages/lwc-integration/src/components/events/test-root-listener-event-target/x-grand-child/x-grand-child.html @@ -0,0 +1,3 @@ + diff --git a/packages/lwc-integration/src/components/events/test-root-listener-event-target/x-grand-child/x-grand-child.js b/packages/lwc-integration/src/components/events/test-root-listener-event-target/x-grand-child/x-grand-child.js new file mode 100644 index 0000000000..35062ca3f2 --- /dev/null +++ b/packages/lwc-integration/src/components/events/test-root-listener-event-target/x-grand-child/x-grand-child.js @@ -0,0 +1,5 @@ +import { Element } from 'engine'; + +export default class GrandChild extends Element { + +}