Bugfix: onboarding analytics #976
Conversation
|
fixes #978 |
| this.continue(); | ||
| }; | ||
|
|
||
| /** | ||
| * Callback on press confirm | ||
| */ | ||
| onConfirm = async () => { | ||
| await AsyncStorage.setItem('@MetaMask:metricsOptIn', 'agreed'); | ||
| // await AsyncStorage.setItem('@MetaMask:metricsOptIn', 'agreed'); |
There was a problem hiding this comment.
my bad, debug code don't modified good call
| @@ -324,24 +353,17 @@ class ImportWallet extends PureComponent { | |||
| ); | |||
| return false; | |||
| } | |||
|
|
|||
| if (this.props.navigation.getParam('existingUser', false)) { | |||
There was a problem hiding this comment.
because we were doing the same thing here https://github.com/MetaMask/metamask-mobile/pull/976/files#diff-e78d44598405f73efa09c3b995881f68R340 before calling this onPressSync method
There was a problem hiding this comment.
in fact this line is using a param that we don't provide when doing a navigation.push so this will always be false
app/reducers/onboarding/index.js
Outdated
| }; | ||
|
|
||
| /** | ||
| * Reducer to keep track of user oboarding actions to send it to analytics if thw user |
app/reducers/onboarding/index.js
Outdated
| case 'SAVE_EVENT': | ||
| return { | ||
| ...state, | ||
| event: action.event |
There was a problem hiding this comment.
I'd prefer if this was an array instead and you push events to the stack, so you can track the scenario of the user going to import wallet and then coming back to the first screen and then create a new wallet.
There was a problem hiding this comment.
totally agree, added a comment on this on analytics sheet. As we didn't have this array analytics behavior there yet I chose to wait for MetaMask/Design#119
brunobar79
added
the
needs-qa
| @@ -192,7 +214,11 @@ class OptinMetrics extends PureComponent { | |||
| onConfirm = async () => { | |||
| await AsyncStorage.setItem('@MetaMask:metricsOptIn', 'agreed'); | |||
| Analytics.enable(); | |||
| Analytics.trackEvent(ANALYTICS_EVENT_OPTS.ONBOARDING_METRICS_OPT_IN); | |||
| InteractionManager.runAfterInteractions(() => { | |||
| this.props.event && Analytics.trackEvent(this.props.event); | |||
There was a problem hiding this comment.
Assuming you have an array of events instead:
if(this.props.events && this.props.events.length){
this.props.events.forEach(e => Analytics.trackEvent(e));
}
|
The events that are saved and triggered later are not being shown in mixpanel As seen here (in this example I did import via seed phrase, but also happens with start exploring and sync)
This example is from an android device, but same thing happens on iOS as well Example with iOS doing "start exploring"/create new wallet |
|
Looks good 👍 |
* handle state * handle onboarding flow * tests * fix timing to save erase event * dah * typo * events * events
Description
This PR adds the ability to save analytics events for first time onboarding flow. If the user wants to Start Exploring or Scan QR Code or Import Seedphrase, that event will be saved in state awaiting for the user to Optin or Optout in MetaMetrics. If the user Optin will send the event, if not, it will erase that information.
Related #930 (comment)
Checklist
Issue
Resolves #???