-
Notifications
You must be signed in to change notification settings - Fork 5
Core 1151 fix cookie yes gtm integration #2768
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
Conversation
fdb52f0 to
8d32910
Compare
TomWoodward
left a comment
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.
this looks fine, looking at the diff i'm wondering if pardot and facebook also need to be updated
8d32910 to
7e829c7
Compare
Is GTM configured to load them as well? If so, it sounds like we can take them out of here at let it do its thing. |
7e829c7 to
dca5d10
Compare
|
We would need to establish if they are currently loaded or not, but for them to be correctly not loaded based on the users consent settings we'd need to move them to gtm What do those vendor scripts do? |
9b2055d to
8cdc978
Compare
TomWoodward
left a comment
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.
you're good to remove the pardot one as well
CORE-1151