Problem/Motivation
This issue has been closed as duplicate. The discussion is moved to #1813014: Dynamically switch visibility setting.
Some module might not want to add the google_analytics JS code, like for example in the AMP pages.
There is already an existing issue and this is a reference issue for #2731279: Add google_analytics hook to skip default GA integration for AMP pages.
Proposed resolution
add \Drupal::moduleHandler()->alter('google_analytics_visibility_pages', $page_match); in _google_analytics_visibility_pages()
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | add_alter_hook_to-2733291-6.patch | 1.25 KB | tduong |
| #6 | interdiff-2733291-4-6.txt | 1.82 KB | tduong |
| #4 | interdiff-2733291-2-4.txt | 1.36 KB | tduong |
Comments
Comment #2
tduong commentedComment #4
tduong commentedComment #5
berdirto define if => to alter whether
This is a bit strange.
The part about multiple grants/modules can be removed, that's from the example you copied and doesn't apply here.
Not sure about GoogleAnalytics, is that the same elswewhere? I would expect "Google Analytics module"
This is a strange example, getPath() returns a string, not a boolean, and it always returns something. Do == '/my/special/page' instead or something like that.
Comment #6
tduong commentedDone.
Comment #7
hass commentedI'm wondering why we do not integrate Google Analytics for AMP into the module and add another condition for themes.
Comment #8
berdirWe could also do a theme condition, want us to create an issue for that?
Adding more hardcoded conditions is also not so great, though.
We'll close this anyway and update #1813014: Dynamically switch visibility setting as that one is older. I still think there are valid use cases for this.
Comment #9
hass commentedI'm wondering how you could know better than the ga code if a code should be shown. It is quite complex as there are so many conditions and with this alter you have no idea why ga module decided to disable it and there is no priority. Maybe the PHP code has disabled it and the alter enables it than. You only get a TRUE/FALSE and still have no context about the "why". Not sure if this is really important, but it could be.
I have seen there is an url param
amp. That may be easier than the theme to integrate into the path field.I still have no idea why this alter is required for any other things. Have you also tried using the library remove feature? I named all unique. That is like an alter hook, but does not require extra code in ga.
I'm open to discuss what is the best solution.
Comment #10
berdirKnowing/conflicts is a problem of the alter hook.
In case of AMP, for example, you *never* want to show it. AMP has its own google analytics integration (amp.module has basic integration for that. An google recommends to user a different property for it, so it actually makes sense to "duplicate" that).
Path matching doesn't support query arguments, you'd have to write your own implementation.
It's not about getting rid of the library but the inline JS, I don't think libray remove would help there? You could alter it out but then that's pointless processing that GA has to do. Better to prevent it from running.
Comment #11
tduong commentedClosing this issue as duplicated. Discussion moved to #1813014: Dynamically switch visibility setting.