Add custom click properties #41
Conversation
|
Hiya @ankane, any thoughts on this one? |
|
@mgk Thanks for the PR, but haven't had a chance to review yet. Will take a look next time I'm working on Ahoy.js. |
|
Can this be included in the upcoming |
- DOM data attributes generally use dashes instead of underscores and rails ERB templates replace undescores with dashes - update example in README to reflect this convention - convert dashes to underscores in generated event properties JSON to be consistent with consistent with Ahoy JSON naming convention
|
Hey @mgk, thanks for the PR, and sorry for the long delay. The idea and implementation look great. For naming, what do you think of It might also be good to have an option to automatically typecast values for individual properties so they're not always strings ( |
| let jsonAttribute = prefix + 'json'; | ||
|
|
||
| if (element.hasAttribute(jsonAttribute)) { | ||
| return JSON.parse(element.getAttribute(jsonAttribute)); |
There was a problem hiding this comment.
Would be good to combine if both are set.
| for (let i = 0; i < attributeNames.length; i++) { | ||
| let attributeName = attributeNames[i]; | ||
| if (attributeName.startsWith(prefix)) { | ||
| let propertyName = attributeName.substring(prefix.length).replace("-", "_"); |
That makes sense. I'm neutral on the names. Do you prefer
I think of the JSON |
|
@juliancheal As it stands now, my group has been using my PR version for some time and it is working well for us. Still happy to update this PR once questions above are answered. If the project owner feels the typecasting he suggested is important maybe you could add that. |
Overview
This adds custom click properties if
data-ahoy-click-prefixed attributes are present.Notes
Considered but did not implement the following features, keeping things simple, but happy to add them if you think they make sense:
data-prefix configurable and default it as above: seemed a bit overkill