Skip to content
This repository was archived by the owner on Sep 2, 2023. It is now read-only.

Conversation

DanSallau
Copy link
Contributor

Hi,

Attached contained little validations that i think should enhance our SettingsAddress page as its done in binary.com for your perusal.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 27.911% when pulling 7e5e189 on nuruddeensalihu:nuru/fix_setting_address into 491c94d on binary-com:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 27.911% when pulling 7e5e189 on nuruddeensalihu:nuru/fix_setting_address into 491c94d on binary-com:master.

@borisyankov
Copy link
Contributor

Examine how the validation and server handing is done in virtual account creation.
Many issues with this approach - non translated errors etc.

@DanSallau
Copy link
Contributor Author

DanSallau commented Jul 14, 2016

@boris have thought as well also. But the problem is the server errors with regard to the address and the phone number are not specific. SO i think we have to use the front end . Example for address it returns something like. Input validation failed: address_line_1. In the case of the phone number length , the validation actually takes place in the front end. There is no such validation in the backend. But considering the binary.com applies the same validation which i think is a good idea btw, i think that's why they are asking about it. We can implement both in the jsx syntax though without the phoneError .

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 28.536% when pulling 2920fbd on nuruddeensalihu:nuru/fix_setting_address into 4a2def4 on binary-com:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 28.513% when pulling 155e4e9 on nuruddeensalihu:nuru/fix_setting_address into 4a2def4 on binary-com:master.

@borisyankov
Copy link
Contributor

I wasn't that much against the validation itself, than the way we store it.
Examine in detail the virtual account creation and most of the new Settings pages.
We do not store the errors in state but calculate them each time, we have a validatedOnce prop that is set to true on first try, we have onSubmit for the form etc.
Just follow the same approach.

@DanSallau
Copy link
Contributor Author

@borisyankov have moved the error to the jsx/return method as suggested . One more thing to remember. Unlike the create account, here we do not have a server validation for phone number length. This feature is solely a front end validation on the binary.com itself.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 28.513% when pulling e451d6c on nuruddeensalihu:nuru/fix_setting_address into 4a2def4 on binary-com:master.

if (phone.length < 6) {
this.setState({ phoneError: 'length' });
} else if (phone.match(/[a-z]/i)) {
this.setState({ phoneError: 'allowed' });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why set error to allowed ? or you actually want to remove error?

@borisyankov borisyankov merged commit d4f2761 into regentmarkets-repo-archive:master Aug 2, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants