Conversation
disable neos google analytics injections and add own tag manager logic
add GTM descriptions to APM section
Sebobo
left a comment
Sebobo
left a comment
There was a problem hiding this comment.
Hey, thanks for this change!
I left some feedback
| <script async custom-element="amp-analytics" src="https://cdn.ampproject.org/v0/amp-analytics-0.1.js"></script> | ||
| <amp-analytics config={'https://www.googletagmanager.com/amp.json?id=' + Configuration.setting('Shel.Blog.analytics.tagManager.id') + '>m.url=SOURCE_URL'} data-credentials="include"></amp-analytics> | ||
| ` | ||
| @if.isset = ${Configuration.setting('Shel.Blog.amp.analytics.tagManager.id')} |
There was a problem hiding this comment.
Wouldn't it make sense to take the setting from the Analytics package setting path?
Then it's not necessary to define it twice.
And did you use a Join because people can add more to it?
There was a problem hiding this comment.
according this post, Inside google tag manager you have to create your own container type for amp pages, which then has its own container ID. Perhaps the settings name could be better, something like ampContainerID.
I've used a Join to get access to the @position method.
if you have a better idea i would be happy if you give me some idea how i can do it better
There was a problem hiding this comment.
You can always use the @position. I think in this case I wouldn't even use AFX but just a Neos.Fusion:Tag instead of the join and define the attributes.
Sebobo
left a comment
Sebobo
left a comment
There was a problem hiding this comment.
The configuration didn't match the setting anymore, so I added some suggested changes :)
| main = afx` | ||
| <amp-analytics config={'https://www.googletagmanager.com/amp.json?id=' + Configuration.setting('Shel.Blog.analytics.tagManager.id') + '>m.url=SOURCE_URL'} data-credentials="include"></amp-analytics> | ||
| ` | ||
| @if.isset = ${Configuration.setting('Shel.Blog.analytics.tagManager.id')} |
There was a problem hiding this comment.
| @if.isset = ${Configuration.setting('Shel.Blog.analytics.tagManager.id')} | |
| @if.isset = ${Configuration.setting('Shel.Blog.analytics.tagManager.ampContainerId')} |
| } | ||
| googleTrackingCustomElement = Neos.Fusion:Join { | ||
| main = afx` | ||
| <amp-analytics config={'https://www.googletagmanager.com/amp.json?id=' + Configuration.setting('Shel.Blog.analytics.tagManager.id') + '>m.url=SOURCE_URL'} data-credentials="include"></amp-analytics> |
There was a problem hiding this comment.
| <amp-analytics config={'https://www.googletagmanager.com/amp.json?id=' + Configuration.setting('Shel.Blog.analytics.tagManager.id') + '>m.url=SOURCE_URL'} data-credentials="include"></amp-analytics> | |
| <amp-analytics config={'https://www.googletagmanager.com/amp.json?id=' + Configuration.setting('Shel.Blog.analytics.tagManager.ampContainerId') + '>m.url=SOURCE_URL'} data-credentials="include"></amp-analytics> |
| main = afx` | ||
| <script async custom-element="amp-analytics" src="https://cdn.ampproject.org/v0/amp-analytics-0.1.js"></script> | ||
| ` | ||
| @if.isset = ${Configuration.setting('Shel.Blog.analytics.tagManager.id')} |
There was a problem hiding this comment.
| @if.isset = ${Configuration.setting('Shel.Blog.analytics.tagManager.id')} | |
| @if.isset = ${Configuration.setting('Shel.Blog.analytics.tagManager.ampContainerId')} |
2 participants
In the AMP specifications it is not allowed to add custom Javascript.
If you have installed the neos/googleanalytics package aside with shel/blog, the analytics package injects a tag with js after the head tag.
This pull request remove this tag and adds the possibility to add an AMP confirmative version of a google tag manager according to this answer