-
Notifications
You must be signed in to change notification settings - Fork 99
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
Refactor form validation #2704
Refactor form validation #2704
Conversation
This hook might replace all StateDialog state handling in future. It stores an object, has a function to update the state with value and name and only re-renders if the state is changed.
Remove the synchronization of state. This is always a problem and should be avoided. The whole state must be handled outside of useFormValidation. Validation functions now get the new value and the state (all other values) as a second parameter. The function must either return a string as an error message to display a errornous validation or undefined to indicate that the validation was successful. Alternatively an exception may be thrown to indicate an error. Only return on object that has three properties: 1. hasError to indicate that an error during the validation 2. errors an object of error messages 3. validate a function to check validate the form as a whole (sets hasError)
Update the functions to the new expected return values. Also always use arrow functions.
Adjust current forms that are using the useFormValidation hook to use the new contract.
Codecov Report
@@ Coverage Diff @@
## gsa-21.04 #2704 +/- ##
=============================================
+ Coverage 53.71% 53.84% +0.12%
=============================================
Files 1072 1074 +2
Lines 26223 26206 -17
Branches 7474 7471 -3
=============================================
+ Hits 14086 14110 +24
+ Misses 11019 10980 -39
+ Partials 1118 1116 -2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks awesome!
What:
Refactor the form validation.
Why:
After looking at the rows per page user setting for a bugfix I had a quick look at the useFormValidation hook. The original version had a major downside by tying to synchronize states. This should always be avoided. State should be handled at one place only.
How:
Added new tests and run all other unit tests.
Checklist: