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

Improve form valid state #95

Merged
merged 3 commits into from
May 28, 2019
Merged

Improve form valid state #95

merged 3 commits into from
May 28, 2019

Conversation

rubencosta
Copy link
Contributor

This PR has the following changes all related with form isValid state:

  • updateInputsWithError now takes a second argument that when it evaluates to true causes the form to be invalidated
  • passing non empty validationErrors causes the form to be invalidated unless preventExternalInvalidation is set
  • updated API.md for updateInputsWithError

I have made this changes because I consider the form to be invalid when the fields have validation errors. I also depend on the form valid state for disabling the submit button while the form is invalid. This is inconsistent due to the submit button being disabled by client side validation but not by server side validation.

Thanks for this amazing library and all the hard work :)

@MilosRasic
Copy link
Contributor

Looks good. Would you mind adding tests to cover this behavior?

@rubencosta
Copy link
Contributor Author

@MilosRasic I have added some tests, please review.

@rkuykendall
Copy link
Member

This looks really solid. Am I reading it right that this would invalidate the form in situations where it currently remains valid though?

@rubencosta
Copy link
Contributor Author

@rkuykendall yes, passing a non empty validationErrors prop invalidates the form.

@rkuykendall
Copy link
Member

@rubencosta Thank you for this PR as well! I'd like to merge it immediately to master.

Same as I said in the other PR, unfortunately, it requires the branch to be cleaned up some. I have a near-term goal of getting these build files out of PRs so they are easier to rebase and merge.

I'll be watching the Gitter chat. I would appreciate your expertise on this project if you're interested.

@rubencosta
Copy link
Contributor Author

@rkuykendall I rebased the branch on the current master. I'm looking forward to seeing version 2 released 👍
I'll try and drop by the chat :)

@rkuykendall rkuykendall merged commit 9518834 into formsy:master May 28, 2019
hashwin pushed a commit to hashwin/formsy-react that referenced this pull request Jun 12, 2019
* feat(form):  Allow invalidating the form with `updateInputsWithError`

* fix(form): Setting `validationErrors` prop invalidates the form

* test(form): Add form valid state tests
This pull request was closed.
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.

3 participants