Auth mobile #783
Auth mobile #783
Conversation
|
@valentinewallace I created an issue to fix the breaking build. It's out of scope for this PR though: #784 |
There was a problem hiding this comment.
Excellente. I'm pretty pleasantly surprised by how fast mobile is coming along and the amount of code reuse we're getting.
Very nice separation of the auto-mobile actions from the wallet. I added a minor suggestion about an error message but I'm good to merge this!
| * or a fingerprint reader on Android. | ||
| * @return {Promise<undefined>} | ||
| */ | ||
| async tryFingerprint() { |
There was a problem hiding this comment.
is there a plan to add copy so the user knows that they can use their fingerprint/faceID?
There was a problem hiding this comment.
If the device supports it the native OS UI will already display copy...
There was a problem hiding this comment.
Ah, not on Android. I just tried it and it worked
There was a problem hiding this comment.
Ok cool. I try to write the code so that it at least runs on Android, but I think we can ignore Android specific issues and focus on iOS for now.
mobile/main.js
Outdated
| @@ -1,5 +1,5 @@ | |||
| /** | |||
| * @fileOverview The main module that wires up all depdencies for mobile. | |||
| * @fileOverview the main module that wires up all depedencies for mobile. | |||
There was a problem hiding this comment.
lol, nit: s/depedencies/dependencies
| @@ -21,6 +27,7 @@ const WaitView = () => ( | |||
| color={color.lightPurple} | |||
| style={styles.spinner} | |||
| /> | |||
| <Text style={styles.copy}>Loading network...</Text> | |||
src/action/auth-mobile.js
Outdated
| async checkNewPin() { | ||
| const { newPin, pinVerify } = this._store.auth; | ||
| if (newPin.length !== PIN_LENGTH || newPin !== pinVerify) { | ||
| this._alert('Incorrect PIN', () => this.initSetPin()); |
There was a problem hiding this comment.
I liked in the previous commit in this PR where it told the user when the PINs did not match. Maybe we could switch to that error message -- is it possible for them to enter a pin of less than the correct length, since it won't let them go to the next screen otherwise?
There was a problem hiding this comment.
I liked in the previous commit in this PR where it told the user when the PINs did not match. Maybe we could switch to that error message
Ok. I changed it back.
is it possible for them to enter a pin of less than the correct length, since it won't let them go to the next screen otherwise?
Nope. The check only happens when all six bubbles are filled.
| return; | ||
| } | ||
| const msg = 'Unlock your Wallet'; | ||
| const { success } = await this._Fingerprint.authenticateAsync(msg); |
There was a problem hiding this comment.
Can confirm that fingerprint auth works on Android!!
There was a problem hiding this comment.
Nice. Did you get that iOS device yet?
SGTM! |
Closes #778, closes #607
N.B. before we use this in production we need to fix #777