-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat: add google analytics script tag #5437
Conversation
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for novu-design ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
apps/web/public/index.html
Outdated
j.async = true; | ||
j.src = 'https://www.googletagmanager.com/gtm.js?id=' + i + dl; | ||
f.parentNode.insertBefore(j, f); | ||
})(window, document, 'script', 'dataLayer', 'GTM-KXMC4XP2'); |
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.
GTM identifier should be taken from the env variables passed from the pipelines. It will be different per environment (dev/staging/prod).
Also are we sure that we want to use the same identifier for both EU and US regions?
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.
As per Justin's message in slack, it will be the same for all environments.
- Added conditions and support of environment variables. I can remove these conditions and variables if you think it is not needed anymore.
What changed? Why was the change needed?
Ref:- INF-344
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer