Feature: analytics v2 #1453
Feature: analytics v2 #1453
Conversation
estebanmino
added
the
needs-qa
|
Issue 1: Tapping on Instapay shows as Navigation Drawer > Logout (row 34 in spreadsheet) Issue 2: Metrics Opt Out doesn’t seem to be triggered (row 7 in spreadsheet) Issue 3: Taps on account icon and Taps on network isn't working (rows 97 and 98 in spreadsheet) Issue 4: Send Flow "confirm send" (“User taps ‘Send’ on review screen”) doesn’t seem to get triggered Issue 5: Approve Request View tx details User taps ‘View details’ isn't working (row 34 in spreadsheet) |
|
@ibrahimtaveras00 everything was fixed but |
…obile into feature/analytics-v2
There was a problem hiding this comment.
Everything looks good. I made some minor suggestions, and ask a couple of questions on things I was uncertain about.
Another thought, that does not need to be addressed in this PR, we can probably create a utility method for calls of type:
InteractionManager.runAfterInteractions(() => {
Analytics.trackEvent(ANALYTICS_EVENT_OPTS.ONBOARDING_SELECTED_NO_THANKS);
});
That's an improvement to be made in another issue though.
| @@ -69,6 +71,15 @@ const styles = StyleSheet.create({ | |||
| } | |||
| }); | |||
|
|
|||
| const stepDescription = { | |||
There was a problem hiding this comment.
Perhaps these should be included in metamask-mobile/app/util/analytics.js
| closing && | ||
| InteractionManager.runAfterInteractions(() => { | ||
| Analytics.trackEventWithParameters(ANALYTICS_EVENT_OPTS.ONBOARDING_SELECTED_SKIP_TUTORIAL, { | ||
| View: stepDescription[step] |
There was a problem hiding this comment.
There seems to be a convention of using a this.getTrackingParams method with trackEventWithParameters. Should we follow that same convention here?
There was a problem hiding this comment.
replied the same in another comment, if we use it just one time I don't think is a need for cleaner code, WDYT?
| @@ -216,14 +217,13 @@ class OptinMetrics extends PureComponent { | |||
| * Callback on press confirm | |||
| */ | |||
| onConfirm = async () => { | |||
| await AsyncStorage.setItem('@MetaMask:metricsOptIn', 'agreed'); | |||
| Analytics.enable(); | |||
There was a problem hiding this comment.
Why is Analytics.enable(); no longer needed?
There was a problem hiding this comment.
because I added it on componentDidMount
| @@ -575,6 +588,9 @@ class Confirm extends PureComponent { | |||
| assetType | |||
| }); | |||
| this.checkRemoveCollectible(); | |||
| Analytics.trackEventWithParameters(ANALYTICS_EVENT_OPTS.SEND_FLOW_CONFIRM_SEND, { | |||
| network: providerType | |||
There was a problem hiding this comment.
Should we follow the this.getTrackingParameters convention here? (I asked a similar question above)
There was a problem hiding this comment.
I only added getTrackingParameters on places where I used it more than one time, if we do it here it seems like boilerplate code to me
|
Also, have we anywhere documented all the params/properties we are sending to mixpanel? |
|
@danjm left comments on yours and added an object to the utils file
|
* onboarding step 1 * finish onboarding * finish onboarding * instapay * navigation * wallet view * not done yet * now it's done * receive * added new events * instapay * send flow * approve * update send flow with data * snaps * fixes * opt out * snapps * after await * disbale internally * Revert "disbale internally" This reverts commit 426ed06. * disbale internally * utils
Description
Several updates to the events we're tracking based on https://docs.google.com/spreadsheets/d/1XYj78KAUytJQCKk1fD8pERxXsZRZ6aQhcMP5GJS5X8k/edit#gid=1878986372
Checklist
Issue
Resolves #1350