ENB-8870 Analytics tracking / [MWPW-185828] [premiere]collection should be allowed to load in aem.page and aem.live url#863
Conversation
|
suhjainadobe
changed the title
suhjainadobe
changed the title
suhjainadobe
requested review from
drashti1712,
hkhatana26,
hkuraware and
nkthakur48
|
@suhjainadobe Could you sync the branch please. Also please feel free to duplicate the ENB ticket into MWPW and reference it in PR title |
|
Skipped merging 863: ENB-8870 Analytics tracking / [MWPW-185828] [premiere]collection should be allowed to load in aem.page and aem.live url due to missing verified label. kindly make sure that the PR has been verified |
Need confirmation on tab flow
|
@suhjainadobe Could you please have a look at @spadmasa's query? |
|
Validation is done on the pr on accessibility and analytics fix provided , able to see the collection loading in aem.page and aem.live |
nkthakur48
left a comment
nkthakur48
left a comment
There was a problem hiding this comment.
@suhjainadobe Please handle my review comments as part of next PR
|
|
||
| const trackEvent = (eventName) => { | ||
| // eslint-disable-next-line no-underscore-dangle | ||
| if (window._satellite) { |
There was a problem hiding this comment.
This check looks redundant since window._satellite?.track below this will handle it correctly. Please take it as part of next PR
| const createEditButton = (buttonText) => { | ||
| const button = createTag('a', { class: CLASSES.BUTTON, tabindex: '0' }); | ||
| const button = createTag('a', { | ||
| class: CLASSES.BUTTON, |
There was a problem hiding this comment.
Could you check whether button HTML element is more correct semantically here?
| observer.disconnect(); | ||
| } | ||
| }); | ||
| }, { threshold: 0.1 }); |
There was a problem hiding this comment.
Keep the magic numbers at the top
5 participants
Resolves: ENB-8870
Resolves: [MWPW-185828]
Test URLs: