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

Integrate form library #28

Merged
merged 5 commits into from
Feb 6, 2021
Merged

Integrate form library #28

merged 5 commits into from
Feb 6, 2021

Conversation

uzuntonev
Copy link
Contributor

Motivation and context

Screenshots:

Login Register
Screenshot_2 Screenshot_1
Forgotten Password Change Password
Screenshot_4 Screenshot_3

New dependencies:

  • formik
  • yup

New dev dependencies:

  • @types/yup

@uzuntonev uzuntonev requested a review from kachar February 5, 2021 19:38
Copy link
Member

@kachar kachar left a comment

Choose a reason for hiding this comment

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

@uzuntonev Awesome work, thanks for the integration! 💯

We had some hesitations on which form library should be chosen.
I guess Formik is good choice at this point and will help us move faster in the future.

In the next updates I'll be happy to see integration of types/interfaces with the form library as this will ensure our form payloads are strongly typed.

const ChangePasswordSchema = yup.object().shape({
password: yup
.string()
.min(6, t('auth:validation.password-min', { count: 6 }))
Copy link
Member

Choose a reason for hiding this comment

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

It's gonna be better to handle the translation at the last step right before displaying the error.

Otherwise we risk to receive mixed translations as the messages are memoized.

We can handle similar validations in a central location as it's shown at https://github.com/jquense/yup#using-a-custom-locale-dictionary

This will also help us decompose the validation schema definition in a separate file, away from the actual form component implementation.

Not super important at this stage but in future can be confusing.

Peek.2021-02-06.12-05.mp4

@kachar kachar merged commit 0555af7 into master Feb 6, 2021
@kachar kachar deleted the integrate-form-library branch February 6, 2021 10:37
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