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

Analytics: Change load event to fire from the window instead of created script element #8015

Closed
wants to merge 2 commits into from

Conversation

anselmbradford
Copy link
Member

We have this analytics code that fires a gtmloaded event when a script tag has loaded. The cfpb-analytics package looks for this event (see https://github.com/cfpb/cfpb-analytics/blob/main/packages/cfpb-analytics/src/cfpb-analytics.js#L109).

If the gtm script loads before other scripts that use cfpb-analytics, it'll fire the gtmloaded event before other scripts. This PR changes this so that the load event fires on the window, so that it'll fire after all page scripts have loaded.

Changes

  • Change load event listening from gtm script tag to window object.

How to test this PR

  1. This would need to be merged into production and then pages like http://localhost:8000/about-us/blog/?debug-gtm=true should show debug messages in the console when scrolling the page.

@anselmbradford
Copy link
Member Author

Turns out this seems to be an issue in Chrome, as the logging is working fine in Safari. Closing in favor of #8018. We might need to revisit this if in fact Chrome isn't logging at all.

auto-merge was automatically disabled November 9, 2023 19:27

Pull request was closed

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.

1 participant