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

Allow seoGoogleTagManager property to be overridden from settings in data/local.js #51

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shaunhurley
Copy link

Sample solution to close #50

Allow seoGoogleTagManager property to be overridden from settings in data/local.js such that specific Google Tag Managers can be enforced in specific environments i.e. dev/test

I've set this up to use a generic 'settings' module as discussed at here but it could presumably just as easily be included in an apostrophe-global section (in line with the existing directives) or an apostrophe-seo section to keep the functionality together?

@abea abea self-requested a review May 18, 2021 21:25
Copy link
Contributor

@abea abea left a comment

Choose a reason for hiding this comment

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

Thanks for this @shaunhurley. Apologies for my taking so long to review it.

Besides the implementation note, this would need documentation in the README. We'd also need the same for the other Google properties, but you probably planned that. This looks like a draft implementation.

I think this would be useful not only for your case but to allow the Google properties to be hard coded in the environment if desired, keeping it out of the UI altogether. It's not the best for everyone, but could be useful for some organizations.

@boutell Do you have thoughts on the general idea?

@@ -1,7 +1,9 @@
{% if data.global.seoGoogleTagManager %}
{% set seoOverRides = apos.settings.getOption('seoOverRides') %}
Copy link
Contributor

Choose a reason for hiding this comment

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

This would need to be on a module we know will be there and that has an alias to reference this way. With that in mind I'd use the SEO module and assign it an alias such as aposSeo.

I'm not sure it needs the seoOverRides layer. You could simply add the option directly on the module. That also saves you having to write an additional conditional below to see if seoOverRides exists.

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.

Environment specific Google Analytics IDs and/or Google Tag Manager IDs
2 participants