Skip to content
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

#3494 sync businessPhone validation with member api #3507

Merged
merged 3 commits into from
Jan 25, 2020

Conversation

yoution
Copy link
Collaborator

@yoution yoution commented Jan 22, 2020

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

@yoution the fix itself works good for me. Though implementation doesn't use formsy which is used to implement PhoneInput field.

To check valid/not valid you've added one more custom argument isValid. Though formsy already supports adding validators https://github.com/christianalfoni/formsy-react. So the code can be reimplemented the next way:

Also, please, check the comments below in the code.

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

All works good.

As has been discussed in comments, it looks like it's not possible to include validation rule inside the component, other than creating checkPhone method.

@yoution
Copy link
Collaborator Author

yoution commented Jan 23, 2020

@maxceem no, it not work good, I lost the country code validate in this repo, so I push again

@maxceem
Copy link
Collaborator

maxceem commented Jan 24, 2020

Thanks, @yoution.

On registration page works perfect now: validaties both code and phone with regexp.

On the profile settings page, it does validate code, but doesn't validate phone with regexp: if enter phone with valid country code, but after I add - the number is still valid, though - is not allowed in regexp:

image

@yoution
Copy link
Collaborator Author

yoution commented Jan 24, 2020

@maxceem have pushed, please review . #3507

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Works good.

@maxceem maxceem merged commit 39eb1de into appirio-tech:cf20 Jan 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants