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

Added queueName option for custom dataLayer #758

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ccook10-chwy
Copy link

What does this PR do?
Google Tag Manager supports custom queue (dataLayer) name but the existing integration does not allow for this. This change adds a queueName option to the GTM integration to support this.

Are there breaking changes in this PR?
No. This change adds an optional option which defaults to dataLayer so that it will continue to function as prior to the change.

Testing
Testing completed successfully. Unit tests were updated to test the new queueName option.

Any background context you want to provide?
This change is needed to aid migration from on-site GTM to Segment destination for large or complex websites which cannot implement the migration all at once. Without this change, a GTM destination cannot be added to a Segment source without breaking an existing GTM implementation.

Is there parity with the server-side/android/iOS integration components (if applicable)?
No

Does this require a new integration setting? If so, please explain how the new setting works
Yes.

  • DataLayer
  • You can specify a custom name for your dataLayer variable if you need to (e.g., coexist with an existing GTM implementation). The name must contain only letters, numbers, underscore, or dollar sign to meet JavaScript identifier rules.

Links to helpful docs and other external resources

@ccook10-chwy
Copy link
Author

@CarlosMecha please review, or advise what the process is for requesting a review. This change is partly inspired by this issue.

#282

@ccook10-chwy
Copy link
Author

Happy birthday to this PR. It addresses the need in #282 so why hasn't it been reviewed?

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