-
-
Notifications
You must be signed in to change notification settings - Fork 728
[MIG] website_google_tag_manager: Migration to 18.0 #1081
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
[MIG] website_google_tag_manager: Migration to 18.0 #1081
Conversation
acbeb4f to
842d5fe
Compare
|
/ocabot rebase |
|
@StefanRijnhart The rebase process failed, because command |
|
@SodexisTeam can you rebase to fix tests? |
|
@StefanRijnhart |
|
@amkarthik Did something not go as planned? I'm surprised to see that commits (and actual changes) that were merged to the 18.0 branch recently are now showing up here in this branch, as if the 18.0 branch was rebased onto this PR's branch rather than the other way around. |
580e1de to
5ebcf9a
Compare
|
@StefanRijnhart |
StefanRijnhart
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, all good now after the latest push!
|
/ocabot migration website_google_tag_manager |
|
Hi @StefanRijnhart @pedrobaeza this is ready :) |
|
This PR has the |
| License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). --> | ||
| <odoo> | ||
| <template id="layout" inherit_id="website.layout"> | ||
| <xpath expr="//div[@id='wrapwrap']" position="after"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part should be added right after the opening tag, as mentioned in this documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SodexisTeam what do you think? Is this something that can be improved?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works if the code snippet is inside the <body>. However, the later it is added, the later it is loaded.
Which is not ideal, because you might miss some visitors. If this can be improved, that would be great
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that no response from @SodexisTeam has come in three months, @JrAdhoc I see you implemented this in your own migration PR of this module with the failing CI (#1064). Would you like to pick it up there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We made the correction. Please check @StefanRijnhart @TomAlbrechtQuatra @JrAdhoc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Is this alright with you @JrAdhoc?
5ebcf9a to
9bd08e3
Compare
|
/ocabot migration website_google_tag_manager |
pilarvargas-tecnativa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reduce administrative commits
9bd08e3 to
0cb1635
Compare
* [ADD] New module 'website_google_tag_manager' * [FIX] website_google_tag_manager - Added OCA as author * [FIX] website_google_tag_manager - Fix RST syntax error (duplicate implicit target name 'google tag manager') * [IMP] website_google_tag_manager - Replace 'openerp' tags by 'odoo' ones + Remove 'data' tags
Updated by Update PO files to match POT (msgmerge) hook in Weblate.
Currently translated at 100.0% (8 of 8 strings) Translation: website-14.0/website-14.0-website_google_tag_manager Translate-URL: https://translation.odoo-community.org/projects/website-14-0/website-14-0-website_google_tag_manager/es_AR/
Currently translated at 72.7% (8 of 11 strings) Translation: website-14.0/website-14.0-website_google_tag_manager Translate-URL: https://translation.odoo-community.org/projects/website-14-0/website-14-0-website_google_tag_manager/fr_FR/
Currently translated at 12.5% (1 of 8 strings) Translation: website-16.0/website-16.0-website_google_tag_manager Translate-URL: https://translation.odoo-community.org/projects/website-16-0/website-16-0-website_google_tag_manager/it/
Currently translated at 100.0% (8 of 8 strings) Translation: website-16.0/website-16.0-website_google_tag_manager Translate-URL: https://translation.odoo-community.org/projects/website-16-0/website-16-0-website_google_tag_manager/es/
Currently translated at 100.0% (8 of 8 strings) Translation: website-16.0/website-16.0-website_google_tag_manager Translate-URL: https://translation.odoo-community.org/projects/website-16-0/website-16-0-website_google_tag_manager/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: website-17.0/website-17.0-website_google_tag_manager Translate-URL: https://translation.odoo-community.org/projects/website-17-0/website-17-0-website_google_tag_manager/
…anager Adding script before the wrapwrap element affects style [1] and hide the nav bar behind of the main menu, so it is required adding it after the wrapwrap element, the same way as [2] does it. Reference: - [1] https://github.com/odoo/odoo/blob/32a07bf4/addons/website/static/src/scss/website.scss#L200 - [2] https://github.com/odoo/odoo/blob/32a07bf4/addons/website/views/website_templates.xml#L151
0cb1635 to
02d87ed
Compare
Reduced commits as per https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate |
|
Looks like all comments have been addressed! /ocabot merge nobump |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at 09c283d. Thanks a lot for contributing to OCA. ❤️ |

No description provided.