feat: add data attributes to script #466
Conversation
dine
force-pushed
the
support-data-attributes
branch
from
e918213 to
e149aa9
Compare
dine
force-pushed
the
support-data-attributes
branch
from
e149aa9 to
21a4129
Compare
| @@ -1,4 +1,5 @@ | |||
| import type { GtmIdContainer, GtmQueryParams } from './gtm-container'; | |||
| import { type DataAttributes } from './utils'; | |||
There was a problem hiding this comment.
suggestion:
| import { type DataAttributes } from './utils'; | |
| import type { DataAttributes } from './utils'; |
| /** | ||
| * Will add data attributes to script tag. | ||
| */ | ||
| dataAttributes?: DataAttributes[] |
There was a problem hiding this comment.
suggestion:
| dataAttributes?: DataAttributes[] | |
| dataAttributes?: DataAttributes[]; |
| config.dataAttributes.forEach(({ name, value }) => { | ||
| script.setAttribute(`data-${name}`, value); | ||
| }); |
There was a problem hiding this comment.
suggestion:
| config.dataAttributes.forEach(({ name, value }) => { | |
| script.setAttribute(`data-${name}`, value); | |
| }); | |
| for ({ name, value } of config.dataAttributes) { | |
| script.setAttribute(`data-${name}`, value); | |
| } |
| @@ -160,6 +161,40 @@ describe('utils', () => { | |||
| }, | |||
| ); | |||
|
|
|||
| // Test dataAttributes | |||
There was a problem hiding this comment.
nitpick: this comment is not needed
| // Test dataAttributes |
| @@ -16,6 +16,11 @@ export interface OnReadyOptions { | |||
| script: HTMLScriptElement; | |||
| } | |||
|
|
|||
| export interface DataAttributes { | |||
There was a problem hiding this comment.
suggestion:
interfaces should use singular
| export interface DataAttributes { | |
| export interface DataAttribute { |
| async: true, | ||
| defer: false, | ||
| nonce: '', | ||
| dataAttributes, |
There was a problem hiding this comment.
issue (blocking): you are passing, but it is not used right now at all
please change the expectScriptToBeCorrect to test internally for data attribute correctness
|
Hi @Shinigami92 |
Some tools require a data attribute for consent management, for example:
https://support.didomi.io/enable-google-consent-mode-with-didomi-new-flow#gcm_didomi_console
Maybe related to issue: gtm-support/vue-gtm#434