-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bugfix: onboarding analytics #976
Changes from 6 commits
3ca51c1
086a2e6
a876ab4
7ba8856
d7af0df
c9464ba
331798c
52dc8cd
5f171f0
9a878a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
/** | ||
* Saves an onboarding analytics event in state | ||
* | ||
* @param {object} event - Event object | ||
*/ | ||
export function saveOnboardingEvent(event) { | ||
return { | ||
type: 'SAVE_EVENT', | ||
event | ||
}; | ||
} | ||
|
||
/** | ||
* Erases any event stored in state | ||
*/ | ||
export function eraseOnboardingEvent() { | ||
return { | ||
type: 'SAVE_EVENT', | ||
event: undefined | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,16 @@ | ||
import React, { PureComponent } from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import { Platform, Alert, ActivityIndicator, Image, Text, View, ScrollView, StyleSheet } from 'react-native'; | ||
import { | ||
Platform, | ||
Alert, | ||
ActivityIndicator, | ||
Image, | ||
Text, | ||
View, | ||
ScrollView, | ||
StyleSheet, | ||
InteractionManager | ||
} from 'react-native'; | ||
import AsyncStorage from '@react-native-community/async-storage'; | ||
import { connect } from 'react-redux'; | ||
import { passwordSet, seedphraseBackedUp } from '../../../actions/user'; | ||
|
@@ -16,6 +26,9 @@ import SecureKeychain from '../../../core/SecureKeychain'; | |
import AppConstants from '../../../core/AppConstants'; | ||
import PubNubWrapper from '../../../util/syncWithExtension'; | ||
import AnimatedFox from 'react-native-animated-fox'; | ||
import Analytics from '../../../core/Analytics'; | ||
import ANALYTICS_EVENT_OPTS from '../../../util/analytics'; | ||
import { saveOnboardingEvent } from '../../../actions/onboarding'; | ||
|
||
const styles = StyleSheet.create({ | ||
scroll: { | ||
|
@@ -120,7 +133,11 @@ class ImportWallet extends PureComponent { | |
/** | ||
* Selected address | ||
*/ | ||
selectedAddress: PropTypes.string | ||
selectedAddress: PropTypes.string, | ||
/** | ||
* Save onboarding event to state | ||
*/ | ||
saveOnboardingEvent: PropTypes.func | ||
}; | ||
|
||
seedwords = null; | ||
|
@@ -297,7 +314,19 @@ class ImportWallet extends PureComponent { | |
|
||
onPressImport = () => { | ||
const { existingUser } = this.state; | ||
const action = () => this.props.navigation.push('ImportFromSeed'); | ||
const action = () => { | ||
this.props.navigation.push('ImportFromSeed'); | ||
InteractionManager.runAfterInteractions(async () => { | ||
if (Analytics.getEnabled()) { | ||
Analytics.trackEvent(ANALYTICS_EVENT_OPTS.ONBOARDING_SELECTED_IMPORT_WITH_SEEDPHRASE); | ||
return; | ||
} | ||
const metricsOptIn = await AsyncStorage.getItem('@MetaMask:metricsOptIn'); | ||
if (!metricsOptIn) { | ||
this.props.saveOnboardingEvent(ANALYTICS_EVENT_OPTS.ONBOARDING_SELECTED_IMPORT_WITH_SEEDPHRASE); | ||
} | ||
}); | ||
}; | ||
if (existingUser) { | ||
this.alertExistingUser(action); | ||
} else { | ||
|
@@ -324,24 +353,17 @@ class ImportWallet extends PureComponent { | |
); | ||
return false; | ||
} | ||
|
||
if (this.props.navigation.getParam('existingUser', false)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this deleted? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because we were doing the same thing here https://github.com/MetaMask/metamask-mobile/pull/976/files#diff-e78d44598405f73efa09c3b995881f68R340 before calling this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in fact this line is using a param that we don't provide when doing a navigation.push so this will always be false |
||
Alert.alert( | ||
strings('sync_with_extension.warning_title'), | ||
strings('sync_with_extension.warning_message'), | ||
[ | ||
{ | ||
text: strings('sync_with_extension.warning_cancel_button'), | ||
onPress: () => false, | ||
style: 'cancel' | ||
}, | ||
{ text: strings('sync_with_extension.warning_ok_button'), onPress: () => this.showQrCode() } | ||
], | ||
{ cancelable: false } | ||
); | ||
} else { | ||
this.showQrCode(); | ||
} | ||
InteractionManager.runAfterInteractions(async () => { | ||
if (Analytics.getEnabled()) { | ||
Analytics.trackEvent(ANALYTICS_EVENT_OPTS.ONBOARDING_SELECTED_SYNC_WITH_EXTENSION); | ||
return; | ||
} | ||
const metricsOptIn = await AsyncStorage.getItem('@MetaMask:metricsOptIn'); | ||
if (!metricsOptIn) { | ||
this.props.saveOnboardingEvent(ANALYTICS_EVENT_OPTS.ONBOARDING_SELECTED_SYNC_WITH_EXTENSION); | ||
} | ||
}); | ||
this.showQrCode(); | ||
}; | ||
|
||
renderLoader() { | ||
|
@@ -439,7 +461,8 @@ const mapStateToProps = state => ({ | |
const mapDispatchToProps = dispatch => ({ | ||
passwordHasBeenSet: () => dispatch(passwordSet()), | ||
setLockTime: time => dispatch(setLockTime(time)), | ||
seedphraseBackedUp: () => dispatch(seedphraseBackedUp()) | ||
seedphraseBackedUp: () => dispatch(seedphraseBackedUp()), | ||
saveOnboardingEvent: event => dispatch(saveOnboardingEvent(event)) | ||
}); | ||
|
||
export default connect( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import { REHYDRATE } from 'redux-persist'; | ||
|
||
const initialState = { | ||
event: undefined | ||
}; | ||
|
||
/** | ||
* Reducer to keep track of user oboarding actions to send it to analytics if the user | ||
* decides to optin after finishing onboarding flow | ||
*/ | ||
const onboardingReducer = (state = initialState, action) => { | ||
switch (action.type) { | ||
case REHYDRATE: | ||
if (action.payload && action.payload.onboarding) { | ||
return { ...state, ...action.payload.onboarding }; | ||
} | ||
return state; | ||
case 'SAVE_EVENT': | ||
return { | ||
...state, | ||
event: action.event | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
}; | ||
default: | ||
return state; | ||
} | ||
}; | ||
|
||
export default onboardingReducer; |
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.
Assuming you have an array of events instead: