search

swup / ga-plugin

A swup plugin for integrating Google Analytics 📈
https://swup.js.org/plugins/google-analytics-plugin
MIT License
2 stars 2 forks source link

Adding new functionality for modern gtag integration #4

Closed studio-blueboat closed 3 years ago

studio-blueboat commented 4 years ago

This PR adds a new function to check for the presence of the new gtag standard integration of analytics, while preserving the use of the old standard.

Unfortunately the new gtag standard requires a measurement ID for virtual page views, so i've integrated an options argument to handle this.

Full disclosure this is literally my first ever PR for open source software, I've copied the options syntax from one of your other plugins and I haven't added to the readme.md file in case I'm totally wrong. Please let me know if this is a good solution or if I'm totally mental.

joshuaHallee commented 4 years ago

Congrats on your first PR for open source software!

I've taken a look at this PR and the options syntax appears to be correct. However, i am going to have to question the if/else if/else statement that checks for ga/gtag functions.

If a site uses strictly gtag.js as their analytics tool, the ga function is still present which means the else if will never occur due to if being true. I cannot find this in the ga/gtag documentation to support my claim, but i have been able to find both functions present in a site that i know only uses gtag.

studio-blueboat commented 4 years ago

@joshuaHallee thanks for the feedback! Would this just be a case of flipping the if/elseif order?

Assuming a site only uses ga, it'll fail the first check and move to the second, but if it contains both by virtue of using gtag only as you suggest, it'll resolve the first condition.

joshuaHallee commented 4 years ago

@studio-blueboat That's a good question, logically i would have to agree that it would work if you flipped the conditions in the if/else statement. I am making an assumption though that if a site only uses ga, the gtag function is not present.

studio-blueboat commented 4 years ago

@joshuaHallee In my admittedly small scale tests (4 production sites), I couldn't find an instance of a ga site with the gtag function present so I'm assuming our logic holds.

I have made a commit to flip that order.

gmrchk commented 3 years ago

Hi! I apologize for the huge delay. This has slipped through somehow.

Awesome stuff! I've merged the PR, added few tiny changes, and update the readme. These changes should are available in v 1.1.0 and will be updated on the docs site in no time.

I did not have an opportunity to test it, so please let me know if there are any issues. 🙏