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

theme: Update google analytics #1290

Closed
wants to merge 1 commit into from
Closed

Conversation

HaoZeke
Copy link

@HaoZeke HaoZeke commented Jul 27, 2020

The existing implementation no long collects any metrics.

@ehuss
Copy link
Contributor

ehuss commented Jul 27, 2020

Thanks for the PR! Can you share any information on the change to the Global Site Tag, and why the old version does not work? Is there any reason to not switch? I checked my test page, and by entering the Tracking ID, the current google analytics code seems to work fine. Perhaps you have a blocker somewhere preventing it from working?

Also, this removes the check for localhost, which is intended to avoid hits during local testing. Is that not needed anymore? Any reason to remove it?

@HaoZeke
Copy link
Author

HaoZeke commented Jul 27, 2020

Ah, yes, I did later realize I had a blocker configured, but the gtag version is now the only recommended code for tracking site hits. The localhost rule isn't required in the new version (the js handles it).

Given all that, it might be OK to keep the current code for a while, but since it is no longer recommended, it might be depreciated later.

@@ -224,20 +224,13 @@

{{#if google_analytics}}
<!-- Google Analytics Tag -->
<!-- Global site tag (gtag.js) - Google Analytics -->
<script async src="https://www.googletagmanager.com/gtag/js?id=UA-109503488-17"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

It has been a long time since I have used Google analytics, but isn't UA-109503488-17 a personal id that should not be here?

Copy link
Author

Choose a reason for hiding this comment

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

Yup, dunno how I missed that, will replace it (script does replace it)

@ehuss
Copy link
Contributor

ehuss commented Nov 27, 2021

Thanks for the PR! I'm going to close as the google-analytics option is now deprecated per #1675. I suggest placing the appropriate code in the theme/head.hbs file instead. Thanks for the PR, though!

@ehuss ehuss closed this Nov 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants