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

Add Google Tag Manager in admin header #22

Merged
merged 5 commits into from
Jun 2, 2023
Merged

Conversation

laurajaime
Copy link
Collaborator

@laurajaime laurajaime commented May 22, 2023

Deface has a strange fail due to nokogiri gem and the overrides with Deface don't work when applied inside the HEAD tag. See:

spree/deface#84
spree/spree#2633

@laurajaime laurajaime added the enhancement New feature or request label May 22, 2023
@laurajaime laurajaime requested a review from tramuntanal May 22, 2023 11:13
@laurajaime laurajaime self-assigned this May 22, 2023
Copy link
Collaborator

@tramuntanal tramuntanal left a comment

Choose a reason for hiding this comment

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

LGTM!

@laurajaime
Copy link
Collaborator Author

@tramuntanal can you review it again, please?

Copy link
Collaborator

@tramuntanal tramuntanal left a comment

Choose a reason for hiding this comment

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

Ok, let's use these overrides but also add a test to check it in further upgrades

README.md Outdated
### Add Google Tag Manager to admin backoffice

Deface has a strange fail with nokogiri gem and the overrides with Deface not works with this changes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you write the required nokogiri version in order to use deface in the HEAD tag please?

Copy link
Collaborator

@tramuntanal tramuntanal left a comment

Choose a reason for hiding this comment

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

LGTM!

@laurajaime laurajaime merged commit ced8e63 into main Jun 2, 2023
@laurajaime laurajaime deleted the add/gtm_in_admin branch June 2, 2023 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants