-
-
Notifications
You must be signed in to change notification settings - Fork 434
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
fix: validation errors in signup form #1174
fix: validation errors in signup form #1174
Conversation
WalkthroughThe changes involve updating the form validation and error display logic in a signup form. Instead of using toast messages to show validation errors, the errors are now directly associated with each form field and displayed beneath them. This approach enhances user experience by providing immediate, contextual feedback on input fields. Changes
Assessment against linked issues
TipsChat with CodeRabbit Bot (
|
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.
Hello @Endless8, thank you for raising a pull request.
Currently, the pull request is marked as https://github.com/MilanCommunity/Milan/labels/status%3A%20todo%20%E2%8F%B3 so please wait until the maintainers/owner review it and provide you with feedback/suggestions to proceed further.
Feel free to reach out to Tamal on Twitter, or drop a mail at [email protected] if you think that this pull request is of critical priority.
Give us a ⭐ to show some support
Happy OpenSource 🚀
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.
Review Status
Actionable comments generated: 6
Configuration used: CodeRabbit UI
Files selected for processing (4)
- src/hooks/useFormLogic.js (1 hunks)
- src/hooks/useValidation.js (3 hunks)
- src/pages/Auth/AuthPage.css (1 hunks)
- src/pages/Auth/AuthSignup.jsx (16 hunks)
Files skipped from review due to trivial changes (1)
- src/pages/Auth/AuthPage.css
Additional comments: 3
src/hooks/useValidation.js (2)
144-147: The error message for the club description length is clear, but the condition for checking the length is missing in the provided hunk. Ensure that the condition for checking the length of
credentials.description
is present and correct to avoid pushing an error when it's not necessary.242-254: The pincode validation checks for a length of exactly 5 or 6 characters, which is a common requirement for postal codes in many countries. However, ensure that this requirement matches the expected format for pincodes in the target user demographic. If pincodes can have different lengths or formats, this validation might need to be adjusted.
src/pages/Auth/AuthSignup.jsx (1)
- 428-431: The
renderErrorMessage
function is called after each input field to display the error message related to that field. This is a good practice for user experience as it provides immediate feedback. However, ensure that therenderErrorMessage
function is not causing unnecessary re-renders. If performance issues are observed, consider memoizing the error messages or the function itself.
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 pull request has been deployed to Vercel.
|
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- src/hooks/useValidation.js (3 hunks)
- src/pages/Auth/AuthSignup.jsx (16 hunks)
Additional comments: 1
src/pages/Auth/AuthSignup.jsx (1)
- 53-76: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [53-432]
The implementation of the
renderErrorMessage
function and its usage across various form fields is consistent and aligns with the pull request's aim to enhance user experience by providing clearer, field-specific error messages.
closes #1172
👷🏻 Changes made
I removed the toast notifications, assigned the validation errors to formState and output them below the corresponding field adding a field property to all the errors for filtering.
📸 Screenshots
Summary by CodeRabbit
Refactor
New Features
Style
Bug Fixes