-
Notifications
You must be signed in to change notification settings - Fork 4
Added support for using new setUser method #9
Conversation
This helps to future proof for companies using the latest version of the embeddable
…ytics.js-integration-elevio * 'master' of https://github.com/segment-integrations/analytics.js-integration-elevio: Bump a.js-int-tester version to ^3.1.0 (segment-integrations#7) Pin karma, karma-mocha dev dependencies (segment-integrations#6) Release 2.1.0 # Conflicts: # HISTORY.md # package.json
Had double quotes instead of single. Sorry.
Codecov Report
@@ Coverage Diff @@
## master #9 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 1 1
Lines 32 34 +2
=====================================
+ Hits 32 34 +2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thanks for the PR! I'm taking a look over it now, and it looks good! I left a single small comment, and I'll be happy to merge this in as soon as I can (probably Monday so that I can confirm our build with the new code looks solid).
lib/index.js
Outdated
if (plan) user.groups = [plan]; | ||
if (objectKeys(traits).length > 0) user.traits = traits; | ||
window._elev.user = user; | ||
if (typeof window._elev.setUser !== 'undefined') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we change this to === to 'function' instead? Just to avoid any uncaughts in the browser if, for whatever reason, this property gets overwritten by some defined, non-function value. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have made the update, thanks @Peripheral1994 :)
@duellsy thanks for the PR! Just deployed this. Should be updated for all in the next ~10min |
…ytics.js-integration-elevio * 'master' of https://github.com/segment-integrations/analytics.js-integration-elevio: Added support for using new setUser method (segment-integrations#9) # Conflicts: # HISTORY.md # lib/index.js # package.json
Thanks @hankim813 Actually, a quick hot fix needs to be released, which you can see here: #10 The previous release had a logic error, which is causing issues on multiple sites. Would appreciate if this could get pushed out quickly. |
This helps to future proof for companies using the latest version of the embeddable