Skip to content

Update copy in Google Tag Manager article #3331

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

Merged
merged 2 commits into from
Aug 25, 2022
Merged

Update copy in Google Tag Manager article #3331

merged 2 commits into from
Aug 25, 2022

Conversation

vanand17
Copy link
Contributor

@vanand17 vanand17 commented Aug 4, 2022

Proposed changes

Changing a letter from upper case to lower case based on how the event shows up in the destination.

Merge timing

@vanand17 vanand17 requested a review from kdaswani as a code owner August 4, 2022 01:55
@kdaswani
Copy link
Contributor

kdaswani commented Aug 4, 2022

@vanand17 Have we confirmed this with a live test? I actually don't see anywhere in the code where we send "Loaded a Page" so want to make sure this is actually all true. https://github.com/segmentio/analytics.js-integrations/blob/master/integrations/google-tag-manager/lib/index.js

Probably happens somewhere else but would love 100% confirmation from a true test if possible. We should also probably update the setting description to lowercase "a" in partner portal as well if so: https://app.segment.com/partner-portal/integration/google-tag-manager/settings

@markzegarelli markzegarelli changed the title Update index.md Update copy in Google Tag Manager article Aug 4, 2022
@markzegarelli markzegarelli added the content-update updates to content that are not new features, includes grammar fixes, added notes label Aug 4, 2022
@kdaswani
Copy link
Contributor

@vanand17 following up on my last comment before we merge this in.

Have we confirmed this with a live test? I actually don't see anywhere in the code where we send "Loaded a Page" so want to make sure this is actually all true. https://github.com/segmentio/analytics.js-integrations/blob/master/integrations/google-tag-manager/lib/index.js

Probably happens somewhere else but would love 100% confirmation from a true test if possible. We should also probably update the setting description to lowercase "a" in partner portal as well if so: https://app.segment.com/partner-portal/integration/google-tag-manager/settings

@vanand17
Copy link
Contributor Author

vanand17 commented Aug 24, 2022 via email

@kdaswani
Copy link
Contributor

Hi @vanand17 - thanks! I ran a test myself to be 100% sure that is what we send. I see that upon a page call, if tracking all pages, we see "Loaded a Page" in the GTM data layer. Approving this PR given my test. I will update the setting description in partner portal to also lowercase the "a" to match
Screen Shot 2022-08-24 at 10 24 55 AM
.

@vanand17
Copy link
Contributor Author

vanand17 commented Aug 24, 2022 via email

@pwseg pwseg merged commit 41a0ab7 into develop Aug 25, 2022
@pwseg pwseg deleted the vanand17-patch-1 branch August 25, 2022 16:28
@github-actions
Copy link
Contributor

Thank you for your contribution! Your pull request is merged, but may take a day or two to appear on the site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content-update updates to content that are not new features, includes grammar fixes, added notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants