-
-
Notifications
You must be signed in to change notification settings - Fork 160
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
Google Tag Manager dataLayer fix #179
Conversation
According to the GTM dev guide (https://developers.google.com/tag-manager/devguide) the dataLayer should be set as a global variable and Google will be in charge of retriving such variable with their gtm.js iframe. Fixes adopted-ember-addons#127
…aLayerProp name for google tag manager adapter
is this going to be merged eventually? |
Any chance to see this incorporated? |
Rebased through github interface so needs a test but seems ok now |
const envParamsString = envParams ? `&${envParams}`: ''; | ||
|
||
assert(`[ember-metrics] You must pass a valid \`id\` to the ${this.toString()} adapter`, id); | ||
|
||
set(this, 'dataLayer', dataLayer); | ||
window[dataLayerProp] = dataLayerValue; |
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.
@bugduino I'm new to this but I've just noticed that it seems like you should not use the window if you are not sure you canUseDom
. I would suggest to move it down a bit (put it below the canUseDom
) to prevent unseen problems.
According to the GTM dev guide (https://developers.google.com/tag-manager/devguide) the dataLayer should be set as a global variable and Google will be in charge of retriving such variable with their gtm.js iframe.
Fixes #127