Remove Umami event attributes from site links, add the ones bibi wanted #617
Conversation
✅ Deploy Preview for webmonetization-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Anca2022
left a comment
Anca2022
left a comment
There was a problem hiding this comment.
Just the event properties, the rest looks good! 😃
| <a | ||
| href={card.cta.href} | ||
| class='btn flexible secondary' | ||
| data-umami-event={card.cta.event} |
There was a problem hiding this comment.
we should also remove the event property from plugins objects in /src/data/developers.ts, since it’s no longer being used.
There was a problem hiding this comment.
And, in general, we should remove this property everywhere else it’s no longer being used.
|
Looks good, but let's run this past Alex before merging. |
There was a problem hiding this comment.
I would have thought we would be interest in wallet list clicks, especially for the Interledger app wallet.
| <a | ||
| href={content.primaryCta.href} | ||
| class='btn primary' | ||
| data-umami-event={content.primaryCta.event}>{content.primaryCta.text}</a |
There was a problem hiding this comment.
I'm surprised that we'd remove event tracking for the primary audience CTAs.
| <a | ||
| class='btn primary flexible' | ||
| href='/publishers' | ||
| data-umami-event='Home page link - Publishers' |
There was a problem hiding this comment.
These are also primary CTAs, why would we not care about them being clicked?
|
Reviewing this I realise that in my original PR for the redesign it seems I failed to include tracking for the extenion links in here |
|
Reviewers need to check this is in line with what was laid out in WEB-94 |
Description
Testing