Skip to content

Commit

Permalink
🐛 Subscriptions: Stops stopping propagation of click events (#26702)
Browse files Browse the repository at this point in the history
* Stops stopping propagation of click events

* Add test

* Creates const and uses array accessor for surrogate property on click events
  • Loading branch information
ChrisAntaki committed Feb 11, 2020
1 parent 28587a0 commit 315d430
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ import {UrlBuilder} from './url-builder';
import {closestAncestorElementBySelector} from '../../../src/dom';
import {dev, userAssert} from '../../../src/log';

/**
* Surrogate property added to click events marking them as handled by the
* amp-subscriptions extension.
*/
const CLICK_HANDLED_EVENT_PROPERTY = '_AMP_SUBSCRIPTIONS_CLICK_HANDLED';

/**
* This implements the methods to interact with various subscription platforms.
*
Expand Down Expand Up @@ -109,27 +115,24 @@ export class LocalSubscriptionBasePlatform {
* @protected
*/
initializeListeners_() {
const handleClickAndStopEventPropagation = e => {
e.stopPropagation();
const handleClickOncePerEvent = e => {
if (e[CLICK_HANDLED_EVENT_PROPERTY]) {
return;
}
e[CLICK_HANDLED_EVENT_PROPERTY] = true;

const element = closestAncestorElementBySelector(
dev().assertElement(e.target),
'[subscriptions-action]'
);
this.handleClick_(element);
};
this.rootNode_.addEventListener(
'click',
handleClickAndStopEventPropagation
);
this.rootNode_.addEventListener('click', handleClickOncePerEvent);

// If the root node has a `body` property, listen to events on that too,
// to fix an iOS shadow DOM bug (https://github.com/ampproject/amphtml/issues/25754).
if (this.rootNode_.body) {
this.rootNode_.body.addEventListener(
'click',
handleClickAndStopEventPropagation
);
this.rootNode_.body.addEventListener('click', handleClickOncePerEvent);
}
}

Expand Down
11 changes: 11 additions & 0 deletions extensions/amp-subscriptions/0.1/test/test-local-subscriptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,17 @@ describes.fakeWin('LocalSubscriptionsPlatform', {amp: true}, env => {
expect(domStub.getCall(0).args[0]).to.be.equals('click');
});

it('initializeListeners_ should handle clicks once per event', () => {
const handleClickStub = env.sandbox.stub(
localSubscriptionPlatform,
'handleClick_'
);

localSubscriptionPlatform.initializeListeners_();
localSubscriptionPlatform.rootNode_.body.click();
expect(handleClickStub).calledOnce;
});

it('should return baseScore', () => {
expect(localSubscriptionPlatform.getBaseScore()).to.be.equal(99);
});
Expand Down

0 comments on commit 315d430

Please sign in to comment.