Skip to content

Commit 6bbab87

Browse files
ChrisAntakierwinmombay
authored andcommitted
🐛 Subscriptions: Stops stopping propagation of click events (ampproject#26702)
* Stops stopping propagation of click events * Add test * Creates const and uses array accessor for surrogate property on click events
1 parent d0c1725 commit 6bbab87

File tree

2 files changed

+24
-10
lines changed

2 files changed

+24
-10
lines changed

extensions/amp-subscriptions/0.1/local-subscription-platform-base.js

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ import {UrlBuilder} from './url-builder';
2121
import {closestAncestorElementBySelector} from '../../../src/dom';
2222
import {dev, userAssert} from '../../../src/log';
2323

24+
/**
25+
* Surrogate property added to click events marking them as handled by the
26+
* amp-subscriptions extension.
27+
*/
28+
const CLICK_HANDLED_EVENT_PROPERTY = '_AMP_SUBSCRIPTIONS_CLICK_HANDLED';
29+
2430
/**
2531
* This implements the methods to interact with various subscription platforms.
2632
*
@@ -109,27 +115,24 @@ export class LocalSubscriptionBasePlatform {
109115
* @protected
110116
*/
111117
initializeListeners_() {
112-
const handleClickAndStopEventPropagation = e => {
113-
e.stopPropagation();
118+
const handleClickOncePerEvent = e => {
119+
if (e[CLICK_HANDLED_EVENT_PROPERTY]) {
120+
return;
121+
}
122+
e[CLICK_HANDLED_EVENT_PROPERTY] = true;
114123

115124
const element = closestAncestorElementBySelector(
116125
dev().assertElement(e.target),
117126
'[subscriptions-action]'
118127
);
119128
this.handleClick_(element);
120129
};
121-
this.rootNode_.addEventListener(
122-
'click',
123-
handleClickAndStopEventPropagation
124-
);
130+
this.rootNode_.addEventListener('click', handleClickOncePerEvent);
125131

126132
// If the root node has a `body` property, listen to events on that too,
127133
// to fix an iOS shadow DOM bug (https://github.com/ampproject/amphtml/issues/25754).
128134
if (this.rootNode_.body) {
129-
this.rootNode_.body.addEventListener(
130-
'click',
131-
handleClickAndStopEventPropagation
132-
);
135+
this.rootNode_.body.addEventListener('click', handleClickOncePerEvent);
133136
}
134137
}
135138

extensions/amp-subscriptions/0.1/test/test-local-subscriptions.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,17 @@ describes.fakeWin('LocalSubscriptionsPlatform', {amp: true}, env => {
116116
expect(domStub.getCall(0).args[0]).to.be.equals('click');
117117
});
118118

119+
it('initializeListeners_ should handle clicks once per event', () => {
120+
const handleClickStub = env.sandbox.stub(
121+
localSubscriptionPlatform,
122+
'handleClick_'
123+
);
124+
125+
localSubscriptionPlatform.initializeListeners_();
126+
localSubscriptionPlatform.rootNode_.body.click();
127+
expect(handleClickStub).calledOnce;
128+
});
129+
119130
it('should return baseScore', () => {
120131
expect(localSubscriptionPlatform.getBaseScore()).to.be.equal(99);
121132
});

0 commit comments

Comments
 (0)