Closed lucascumsille closed 2 months ago
@zarino thanks for all the help when creating the issue. I'm not sure how to test it though, I added some console logs before to check if we were gathering correctly the params, event name and callbacks, and it seems to be working fine.
I split the fileType and imgRatio, but we could have merged those two together, I think because they are two separate things it makes sense to keep them separate, but I'm not to fuzzy about either option.
Thanks for the feedback @zarino 🙌
Regarding the following comments:
Although—crazy thought—I wonder whether we even need to be recording section, fileType and imgRatio separately, when we could just track the URL of the image the user is downloading? 🤔 Wonder whether that might be simpler.
I thought we were making the split between the different properties because it had an impact for the person checking the clicks, but if it doesn't have any practical benefit then I would go for the simple solution. Let me know if you have any preference regarding this.
I’m slightly worried this is a bit fragile if we happened to change the URL pattern, and some missing data in GA would go easily unnoticed. Since we’re pulling lots of other data out of custom attributes on the elements, maybe we should do the same here?
Good point! I tried finding how we pull the section codes on a section page, I couldn't find anything there, except by the canonical_url
bit. I'm sure we can add something to sectionView
if we wanted to add it as an attribute. In the meantime I have replace the section by the current page url, which gives up the section code.
Let me know if you have any preference regarding this.
My instinct is simpler is better. There won’t be a huge number of events here, and if somebody really wanted to split out the filetypes and aspect ratios, they could do it from the image URLs.
But maybe check that just recording the URLs of the images, and how often they’ve been downloaded, is good enough for CEUK? Since they’re the ones that asked for the tracking in the first place :-)
@zarino at the end we will be using only the url as parameter, considering that it also displays:
Fixes: https://github.com/mysociety/caps/issues/688
We are tracking: