Send target element along with track call #50
Conversation
support to pass through clicked element on track call
Passing element which was clicked on track so we can add some auto tracking based on the element type.
Codecov Report
@@ Coverage Diff @@
## master #50 +/- ##
==========================================
+ Coverage 99.3% 99.31% +<.01%
==========================================
Files 15 15
Lines 433 435 +2
==========================================
+ Hits 430 432 +2
Misses 3 3
Continue to review full report at Codecov.
|
| @@ -62,7 +63,8 @@ export class TrackBindingPlugin { | |||
| return; | |||
| } | |||
|
|
|||
| let elem = event.target || event.srcElement; | |||
| var originalElem = event.target || event.srcElement; | |||
There was a problem hiding this comment.
Do you need this new variable?
Just pass elem Into callback
Also, prefer let to var
There was a problem hiding this comment.
I believe we do need that variable as elem becomes reassigned and I want to be able to track data on the original element that was clicked. Happy to change to let
There was a problem hiding this comment.
Ah ok, you're right. Change to let and I'll merge. Thanks for the PR
| @@ -371,11 +371,12 @@ export class Metrics extends EventEmitter { | |||
| * @returns {Promise} | |||
| * @private | |||
| */ | |||
| _addEventNameToPromise(eventName, promise, shouldMerge) { | |||
| _addEventNameToPromise(eventName, promise, shouldMerge, element) { | |||
There was a problem hiding this comment.
Non blocking question... would it make sense to change the _addEventNameToPromise signature to use a single object argument instead of 4 arguments for legibility?
_addEventNameToPromise({eventName, promise, shouldMerge, element}) {
|
@mmairs9 I'm down to merge this if you can just make the couple small changes noted in the review. |
We want to be able to default some track attributes based on the element type. If we have context of the clicked element we can begin auto tracking alot of data.