Skip to content
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

add GTM script tag #399

Merged
merged 23 commits into from
Nov 2, 2023
Merged

add GTM script tag #399

merged 23 commits into from
Nov 2, 2023

Conversation

ChisomOguibe11
Copy link
Contributor

What?

  • Setup google tag manager alongside ga4
  • push custom pageLoad events to track page visits

Why?

  • Requested by business for ETA

How?

  • If a GTM tag is present in the service hof-config, the gtm script will run across all pages the form

Testing?

  • Published beta package to test locally and on branch using GTM preview mode

Screenshots (optional)

Anything Else?

@@ -59,3 +59,25 @@
}
</script>
{{/ga4TagId}}

{{#gtmTagId}}
<!-- Data Layer -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean by Data layer? was this comment from you?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's from the snippet code sent from the business. I think it's just a comment to explain what the code is for.

});
return str;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like github actions is complaining about some linting and styling

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aaah thanks ...god I hate linting 😂

@@ -39,22 +49,28 @@ const setupPageMap = routes => {
return pageMap;
};

module.exports = (app, config) => {
module.exports = (app, config) => {
const gaTagId = config.gaTagId;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lots of linting issues

lib/ga-tag.js Outdated
res.locals.gaAllowDebug = config.env === 'development';
res.locals.gaTagId = gaTagId;
res.locals.ga4TagId = ga4TagId;
res.locals.gtmTagId = gtmTagId
res.locals.appName = appName
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need the appName?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was using it before, but no longer need it. Good spot

Copy link
Contributor

@sulthan-ahmed sulthan-ahmed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good just needs some minor changes

- add environmentType template variable for pageLoad event
- add validation that cookies are accepted before running GTM snippets
@ChisomOguibe11 ChisomOguibe11 merged commit 3a2fe40 into master Nov 2, 2023
6 checks passed
@ChisomOguibe11 ChisomOguibe11 deleted the ETA-52 branch November 2, 2023 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants