ankane
Add custom click properties
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:
- make the
data-prefix configurable and default it as above: seemed a bit overkill - put the extra properties at the top, not nested under properties: I was mimicking Rails Ahoy here and didn't want to complicate the behavior with the possibility of name collisions
@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.
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 data-ahoy-properties (or data-ahoy-props) for the JSON format and data-ahoy-prop-product-id for individual props so they don't overlap?
It might also be good to have an option to automatically typecast values for individual properties so they're not always strings (true and false converted to boolean, 123 converted to integer). Wanted to bring it up now in case it makes sense to have as the default. Let me know what you think.
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
data-ahoy-properties(ordata-ahoy-props) for the JSON format anddata-ahoy-prop-product-idfor individual props so they don't overlap?
That makes sense. I'm neutral on the names. Do you prefer data-ahoy-properties or data-ahoy-props ?
It might also be good to have an option to automatically typecast values for individual properties so they're not always strings (
trueandfalseconverted to boolean,123converted to integer). Wanted to bring it up now in case it makes sense to have as the default. Let me know what you think.
I think of the JSON data-ahoy-props as how you would specify a structure with datatypes. I'm somewhat opposed to automatic type conversion for the scalars. Doing that raises questions of what types to try in what order and how one could disable the conversion if for example they wanted "42" as a string. I originally though of doing something like data-ahoy-prop-foo="42" and data-ahoy-prop-foo-type="string", but that led me to JSON.
@mgk @ankane Hey both, is there anything I can do to help get this PR merged?
@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.