Phone verification #946
Phone verification #946
Conversation
| render() { | ||
| const { cancelButton, emailErrorMessage, phoneErrorMessage, isPending, fieldType } = this.props; | ||
|
|
||
| return fieldType === 'phone' ? ( |
There was a problem hiding this comment.
Is there a particular reason we use a single component to do two (seemingly) distinct duties, as opposed to UserEmailNew and UserPhoneNew or equivalent?
There was a problem hiding this comment.
this was in the spirit of DRY a la our coding maxims.
There is less of a case for userFieldNew to be used for two field types, but userFieldVerify is basically the same for both types. I made userFieldNew to be consistent with that.
There was a problem hiding this comment.
I don't see very much savings here aside from maybe some boilerplate in index.js, and it adds complexity to declaration and handling changes. I'd split these.
| )} | ||
| </p> | ||
| <Form onSubmit={this.handleSubmit.bind(this)}> | ||
| <FormRow |
There was a problem hiding this comment.
Does this have to be collected separately as opposed to letting them type a full number? If it is separate field, could it at least be an inline select next to the full number entry?
There was a problem hiding this comment.
I suppose it could be a select. Twilio wants a separate country code. I tried to make this as fool proof and intuitive as possible. I think the way it is makes a lot of sense (it was based on a discussion in tech-app), but I am completely open to changing it.
There was a problem hiding this comment.
@liamcardenas yes, please make the country code a select displayed before the phone number entry.
| dispatch(doCloseModal()); | ||
| dispatch(doNavigate('/rewards')); |
There was a problem hiding this comment.
Is it correct to always bump the user to rewards from here?
There was a problem hiding this comment.
I believe so, since the only place you can access this is from the verify identity page.
|
@liamcardenas Also, I seem to no longer be prompted for my email immediately after the welcome popup. |
|
@kauffj oh, for some reason, I thought we were taking that out. Should I add it back in? |
not quite ready for review yet, still ironing out some things