fix(list): make clicking on list items trackable by analytics apps #3339
Conversation
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3339/ |
| data-text={item.text} | ||
| data-secondary-text={item.secondaryText} |
There was a problem hiding this comment.
@magnusfagerlund this change reflects that text and secondaryText props of the list items on the <li /> element, as data-text and data-secondary-text like below:
Important
These could be different based on the language. Is it OK for you and your Heap? Or do you need some static thing like id which is the same for each language.
I should just say that if we go that way, you don't get any information from Heap, unless we update all lists everywhere, and add some id to their items manually.
That we could still do probably in future. But I think this should be enough for now. Right?
There was a problem hiding this comment.
It would be great if we could use id. It will be too much admin work in Heap to keep track of the different languages otherwise.
There was a problem hiding this comment.
@jgroth also raised a concern about leaking personal info, if we use the data itself. While that could be the case for anything clickable. I still think it's a valid concern, and an important thing to mitigate if we can.
There was a problem hiding this comment.
@adrianschmidt has some idea using some keys, but don't do it during the cycle if that takes too much time Adrian
There was a problem hiding this comment.
If there is a risk of leaking personal information using text and secondaryText, then we shouldn't do it. Then we should only use íd.
There was a problem hiding this comment.
Perfect! I've updated the commit 👌
There was a problem hiding this comment.
But Lime Go are applying
data-intercom-targetin some places of the app. In this case to make it easier to user Intercom when creating Product Tours and such, but could we apply something similar?
That's basically what we are doing here, but the problem is knowing what value to put in there. Go has probably hard-coded a few values on their main menu items specifically. We could also do that for the select few places where we know exactly what the menu option is going to be, because it's not configurable. But for any other place, where configuration, user role, or anything else, can affect what options are available, we need some automatic way.
Going with only the main text, and only on menu items, not on list items, seems like a very good solution for now 👍
There was a problem hiding this comment.
Ah… now I think I understood the comments about using id. We don't have a property called id on menuItem, but we do have a property called value, which I think is the one that should be used if you need to identify the menu option in a language independent way. So perhaps we should simply use that one instead?
There was a problem hiding this comment.
value can be anything so it's hard to use that. If an object is used it will just say [Object object] everywhere. You could of course implement toString() to get something else, but it also makes it a bit unclear that it will be used for this, and if you do it on let's say a LimeObject we are back at sending personal information since we have no control over what value is.
Using something like id is probably a lot safer since it should be used to identify things 😄
There was a problem hiding this comment.
Ah, crap. Good point!
Then I think using only text and only for menus is the way to go for now.
adrianschmidt
force-pushed
the
list-item-tracking
branch
from
3bf9008 to
d8f49c1
Compare
…oftware like Heap -CHANGELOG- This change adds the value of the menu item's `text` property under the attribute `data-text`, to make it possible to distinguish which menu option was selected, in analytics apps like Heap. Note that if you are putting any kind of sensitive data in the `text` property of menu items, and you are using a service for analysing user behaviour, such data could be sent to your analytics service, which may be a violation of GDPR and other regulations. If including such data in the menu item is absolutely necessary, we recommend including it in the menu item's `data-secondary-text` property instead.
adrianschmidt
force-pushed
the
list-item-tracking
branch
from
d8f49c1 to
7df17a9
Compare
adrianschmidt
dismissed
their stale review
We might want to make some changes…
|
🎉 This PR is included in version 37.75.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fix #3338
Review:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: