-
Couldn't load subscription status.
- Fork 19
feat(consent): track consent banner close events #373
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
base: main
Are you sure you want to change the base?
Conversation
|
This PR will trigger a minor release when merged. |
| return; | ||
| } | ||
| // eslint-disable-next-line no-param-reassign | ||
| sampleRUM.oneTrustTrackingSet = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need this exposed on the object? we could just use a local variable, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure, would a local variable cut it? what happens if addCookieConsentTracking(sampleRUM) called twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be only 1 sampleRUM instance… so it would be the same problem.
You could bail out with a conditional if it is called a 2nd time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ekremney
couple of comments:
- with this new approach if a visitor who already closed the the banner before in a previous visit, leaves the page before OneTrust is loaded, we will not track the
hiddencheckpoint. - A cosmetic issue. We are defining an object
window.OneTrustin sites where OneTrust is not installed, right?
|
Changes
Instead of reading cookies directly, now we use the OneTrust Banner SDK to catch the OneTrust events.
Motivation
We currently don't track the "banner closed" (if) happens after "banner shown". This is especially important to calculate the bounces caused by the cookie consent banner robustly.
this PR introduces a new target
closeto theconsentcheckpoint.Note: if we decide to go with this impl, I will adjust the tests.
Here's an alternative implementation for the
closeevent keeping everything else the same: #374