Skip to content

Conversation

ddenisyuk
Copy link
Contributor

Removing of event listener with in a way like: element.onclick = null;, stores null as a handler.

@CLAassistant
Copy link

CLAassistant commented Jun 30, 2023

CLA assistant check
All committers have signed the CLA.

@ddenisyuk ddenisyuk force-pushed the fix-unsubscription branch from d0c4362 to 0da4f4a Compare June 30, 2023 21:37
Copy link
Collaborator

@powerivq powerivq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. LGTM otherwise.

@@ -22,7 +22,9 @@ export const appendGlobalEventProperties = (keys: Array<string>): void => {
if (stored) {
this.removeEventListener(normalizedKey, stored);
}
this.addEventListener(normalizedKey, value);
if (typeof value === 'function') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do value instanceof Function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, no problem

@ddenisyuk ddenisyuk requested a review from powerivq August 22, 2023 11:59
@powerivq powerivq merged commit 36ed6a9 into ampproject:main Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants