Allow to set recovery phone when creating account #29
Conversation
vgrichina
changed the title
| @@ -41,6 +43,17 @@ export function handleRefreshAccount(wallet, history) { | |||
| }) | |||
| .catch(e => { | |||
| console.log(e) | |||
|
|
|||
| if (e.message && e.message.indexOf('is not valid') !== -1) { | |||
There was a problem hiding this comment.
pull 'is not valid' into a constant somewhere?
There was a problem hiding this comment.
or better yet this function might be good as a util function
There was a problem hiding this comment.
proper way to fix this would be to haver error codes returned by nearcore. This works as a temporary kludge as is.
| })) | ||
| } | ||
|
|
||
| isLegitField(name, value) { |
There was a problem hiding this comment.
let's rename legit to something else, maybe valid?
There was a problem hiding this comment.
this seems subjective, Legit is used in all other code in wallet.
| </Header> | ||
| <Header as='h3' className='color-blue'> | ||
| You can skip this if you plan to backup account keys manually. | ||
| We won't be able to help you with account recovery otherwise. |
There was a problem hiding this comment.
Is this text from the official UX? I think the sentence should read as "You can skip this blah blah... If you do skip this, we won't be able to help with your account recovery" or something similar. Right now it reads as having the opposite meaning of what we want to say.
There was a problem hiding this comment.
There is no official UX for now, going to update text after comments from Jake and Erik
| <Grid.Row className='disclaimer border-top-bold'> | ||
| <Grid.Column computer={16} tablet={16} mobile={16} className=''> | ||
| <span className='disclaimer-info'>DISCLAIMER: </span> | ||
| This is a developers' preview Wallet. It should be used for |
There was a problem hiding this comment.
Is this still true about the preview?
| import styled from 'styled-components' | ||
|
|
||
| const RecoveryInfoForm = styled(Form)` | ||
| &&& input { |
There was a problem hiding this comment.
is it normal in react to have css inline? I definitely prefer separate css files
There was a problem hiding this comment.
is it normal in react to have css inline? I definitely prefer separate css files
This is somewhat common in react for component-level styles that aren't going to be reused elsewhere to avoid css anti-pattern of overriding, then overriding again with !important. I'm not personally a fan, but this is pretty widely accepted.
There was a problem hiding this comment.
That being said, this is a big enough block of style that I would argue it should be taken out of here.
There was a problem hiding this comment.
that's normal pattern, the way to refactor it is to cut components into smaller pieces
the good part about this is that it's scoped by component, so very easy to manage vs global styles which can get applied to whatever
| return false | ||
| } | ||
|
|
||
| const { dispatch } = this.props; |
There was a problem hiding this comment.
Let's start adding tests for the wallet code. This looks like it doesn't have a lot of UI logic, so should be possible to write unit tests for this.
There was a problem hiding this comment.
I plan to add integration tests for contract helper and end-to-end tests for wallet.
Unit tests are going to be easy to write for Redux-based app, but I don't think they are going to be very useful. Most of our real world problems are with broken integrations.
No description provided.