-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat!: added file download tracking (#260) #263
feat!: added file download tracking (#260) #263
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks Jonah! I've added a couple of comments. Can you also double-check the GTM parameter value limit? I was under the impression that parameter names are limited to 40 but parameter values can be 100.
src/components/Index/components/AzulFileDownload/azulFileDownload.tsx
Outdated
Show resolved
Hide resolved
@MillenniumFalconMechanic my bad, I read the wrong limit, it is 100 characters. Some file names come very close to that, but I couldn't find any that have more than 100 characters, so this approach is probably fine |
Breaking Changes @frano-m
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you @jpaten 🚀
d102cc5
to
ac850fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks Jonah!
Changes
file_downloaded
event and relevant models as specified in Add tracking of file download #260AzulFileDownload
, adding new optional props as needed