Skip to content

Prevent double marks in IAB without unregistering the eventlistener #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Suven
Copy link

@Suven Suven commented Oct 28, 2022

Hey,

thanks for your inspiring talk today at perfnow.nl! This recommendation came in really handy for me as I am using a SourcePoint but simply was missing out on this opportunity.

From my experience, publishers often already have code listening to that event because they need to process custom vendors, want to trigger reloads, or many other things. I imagine if someone stumbles upon your snippet and just adds the inner code to their existing callback, they might run into issues when unregistering the event handler. That's why I am suggesting this alternative.

On the other side, you could argue that this is the developers responsibility, or that they just could register two event listeners and thus, that this is a workaround for a problem that might not even exits in the first place.

In that case, I am not disappointed in you rejecting this PR. Take it as feedback/heads up in that case (:

@Suven Suven changed the title Patch 1 Prevent double marks in iAB without unregistering the eventlistener Oct 28, 2022
@Suven Suven changed the title Prevent double marks in iAB without unregistering the eventlistener Prevent double marks in IAB without unregistering the eventlistener Oct 28, 2022
@andydavies
Copy link
Owner

Thanks, I’ll take a look at this once I’ve caught up with a few other things I need to do

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.

2 participants