Add support for Google data layer #2613
Conversation
|
Kudos, SonarCloud Quality Gate passed!
|
| * {@code true} if the data layer client library should be included. | ||
| * @since om.adobe.cq.wcm.core.components.models 12.24.0 | ||
| */ | ||
| default String getDataLayerName() { return DATALAYER_OBJECT_NAME_ADOBE; } |
There was a problem hiding this comment.
This will require a package minor version bump
| * {@code true} if ACDL library should be used. | ||
| * @since om.adobe.cq.wcm.core.components.models 12.24.0 | ||
| */ | ||
| default boolean isDataLayerSkipAcdlInclude() { return false; }; |
There was a problem hiding this comment.
We already have isDataLayerClientlibIncluded that serves the same purpose (to control if the ACDL scripts should be included).
There was a problem hiding this comment.
unfortunately isDataLayerClienlibIncluded removes the datalayer.js file not just the ACDL. This may be a bug: when the flag is set to true there is an odd state - some component datalayer interaction happens as before (accordian, text etc), but the components that are caught in a loop in datalayer.js are no longer pushed to the datalayer. In short we get a semi-enabled state.
This is the reason (clienlib.config.js):
assets: {
js: [
"src/scripts/datalayer/v1/polyfill.js",
"node_modules/@adobe/adobe-client-data-layer/dist/adobe-client-data-layer.min.js",
"src/scripts/datalayer/v1/datalayer.js"
]
}
For the GDL I need to keep datalayer.js. This has led to the ugly approach of "remove DL clientlib and include part of it again"
Perhaps it would be better to change the existing functionality so that only the ACDL code is removed by isDataLayerClientlibIncluded? That would allow me to make the whole thing cleaner, and seems more intuitive.
| * @private | ||
| * @param {HTMLElement} item The item to annotate as expanded | ||
| */ | ||
| * Annotates the item and its internals with |
There was a problem hiding this comment.
I see no reason to reformat these
| @@ -26,5 +26,12 @@ | |||
| <sly data-sly-test="${clientLibCategoriesJsHead}" data-sly-call="${clientlib.js @ categories=clientLibCategoriesJsHead, async=page.isClientlibsAsync}"></sly> | |||
| <sly data-sly-test="${clientLibCategories}" data-sly-call="${clientlib.css @ categories=clientLibCategories}"></sly> | |||
| <link data-sly-test="${staticDesignPath}" href="${staticDesignPath}" rel="stylesheet" type="text/css" /> | |||
| <sly data-sly-test="${page.data && page.dataLayerClientlibIncluded}" data-sly-call="${clientlib.js @ categories='core.wcm.components.commons.datalayer.v1', async=true}"></sly> | |||
| <sly data-sly-test="${!page.dataLayerSkipAcdlInclude}"> | |||
There was a problem hiding this comment.
We already have isDataLayerClientlibIncluded that serves the same purpose (to control if the ACDL scripts should be included).
There was a problem hiding this comment.
see Page.java explanation above
| * @return the type of push function (push() or gtag()) of the datalayer | ||
| * Defaults to push. | ||
| */ | ||
| @Property(label = "Use gtag function to push data") |
There was a problem hiding this comment.
Wouldn't it be enough to know that the name of the datalayer is gtag?
There was a problem hiding this comment.
no, the datalayer can be called anything or normally dataLayer, regardless of whether GA (gtag) or GTM is used.
| * Defaults to false. | ||
| */ | ||
| @Property(label = "ACDL library not included") | ||
| boolean skipAcdlInclude() default false; |
There was a problem hiding this comment.
We already have skipClientlibInclude that serves the same purpose (to control if the ACDL scripts should be included).
There was a problem hiding this comment.
see reply to Paga.java above
This is rooted in the same issue as described in Page.java there are four states now, assuming data layer is enabled: ACDL + datalayer.js included - skipClientlibInclude = false what would be good and would remove a boolean option, if the second state is actually a bug is: enabled: datalayer.js included ACDL included - skipClientlibInclude = false |
|
blocked by #2620 |
|
will create a new PR after #2620 is fixed |
The PR adds code to:
Make the name of the data layer configurable
Option to exclude ACDL but keep the other logic
Option to push to a datalayer using the gtag() function instead of push()
Modified unit tests
Please see the updated documentation in DATA_LAYER_INTEGRATION.md