[add-resource] New Plugin - to add Google Tag Manager Container code to the Grav CMS

[jaymurphy1997]

Hello:

I would like to add my new plugin, which ads the Google Tag Manager Container code to the head and foot positions of a Grav website without and code changes in the template or pages.

The project lives here https://github.com/jaymurphy1997/grav-plugin-gtm-plugin

I plan to make further updates - as both Grav CMS and Google Tag Manager will be part of my business going forward.

Best regards,
Jay Murphy

[rhukster]

Added to GPM

[rhukster]

Won't show up until you do a proper release though: https://learn.getgrav.org/17/advanced/grav-development#theme-plugin-release-process

[pamtbaau]

Took the liberty to have a look at the plugin.

A small and simple plugin which nonetheless could benefit from some improvements:

• The inline code is added correctly.
• The <noscript> tag is not being inserted.

Code suggestions:

• Stick to the layout of the file generated by bin/plugin devtools
  • Add extra events in onPluginsInitialized, as shown in the generated plugin skeleton and also as shown in the docs.
    When adding events to getSubscribedEvents unexpected side effects could occur.
• You've created a comment "// Don't proceed if there is no GTM Container ID", but the code does not test for an empty GTM containder Id.
• During event onAssetsInitialized Grav has not yet generated any output. This is why the <noscript> is not being inserted.
  • Did you test/debug it?
• Take a look at the docs about Event Hooks and Grav Lifecycle to learn when the output has been generated.

Readme:

• The Configuration example does not match the gtm-plugin.yaml file.
• The Usage section might be expanded a bit by explaining how and where the container Id should be added (file vs Admin).
  There are many unexperienced users and users who only use Admin.

[jaymurphy1997]

Thanks for your comments. I did not realize this was live. I will address these comments. The iframe piece in particular, but the thorough review and comment are very helpful. Thank you

…

On Fri, Aug 23, 2024, 10:54 AM pamtbaau ***@***.***> wrote: Took the liberty to have a look at the plugin. A small and simple plugin which nonetheless could benefit from some improvements: - The inline code is added correctly. - The <noscript> tag is not being inserted. Code suggestions: - Stick to the layout of the file generated by bin/plugin devtools - Add extra events in onPluginsInitialized, as shown in the generated plugin skeleton and also as shown in the docs <https://learn.getgrav.org/17/plugins/plugin-tutorial>. When adding events to getSubscribedEvents unexpected side effects could occur. - You've created a comment "// Don't proceed if there is no GTM Container ID", but the code does not test for an empty GTM containder Id. - During event onAssetsInitialized Grav has not yet generated any output. This is why the <noscript> is not being inserted. - Did you test/debug it? - Take a look at the docs about Event Hooks <https://learn.getgrav.org/17/plugins/event-hooks> and Grav Lifecycle <https://learn.getgrav.org/17/plugins/grav-lifecycle> to learn when the output has been generated. Readme: - The Configuration example does not match the gtm-plugin.yaml file. - The Usage section might be expanded a bit by explaining how and where the container Id should be added (file vs Admin). There are many unexperienced users and users who only use Admin. — Reply to this email directly, view it on GitHub <#3845 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABPUAA5QRIZPRAV4PLRAVQLZS5EJZAVCNFSM6AAAAABL64DEUOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBXGI2TSMZUG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[pamtbaau]

I did not realize this was live.

It isn't live until you create a proper release as mentioned above. I just cloned your repo.

[jaymurphy1997]

I have addressed all the comments by @pamtbaau from above and created a "0.1.0-alpha" release. I plan to add more functionality over time as well as address any issues that arise.