Enhance Auth page with inline form validation #234
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
WalkthroughAdds client-side inline validation to the Auth page with per-field error state, on-change error clearing, and ARIA-linked error messages; updates documentation and todo metadata to record the completed validation task. Changes
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)web/pages/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧬 Code graph analysis (1)web/pages/Auth.tsx (1)
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for split-but-wiser ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.Jules/changelog.md:
- Line 10: The changelog claim about "accessibility support" is inaccurate
because web/pages/Auth.tsx and the Input component only pass errors via the
error prop and render messages in a plain div; either update .Jules/changelog.md
to remove or tone down the accessibility claim, or implement proper ARIA
support: modify the Input component to apply aria-invalid when an error exists,
render error elements with stable IDs and expose them via
aria-describedby/aria-errormessage on the input, and ensure the Auth page uses
an aria-live="assertive" or role="alert" live region for announcing validation
errors so screen readers receive immediate feedback.
In @.Jules/knowledge.md:
- Around line 152-169: The Form Validation pattern is incomplete: extend the
validate function (validate) to include concrete rules for email, password, and
conditional name checks and ensure it sets per-field errors in fieldErrors; add
a clearFieldError(field) helper and call it from each input's onChange so errors
clear as the user types; wrap inputs in a <form> with noValidate and wire
handleSubmit to call validate before submission; pass per-field error strings to
Input via the error prop and add accessibility props (aria-invalid,
aria-describedby) to the Input and render an associated error element with
role="alert" whose id matches aria-describedby.
In @web/pages/Auth.tsx:
- Around line 34-61: Extract the duplicated error shape into a shared type alias
(e.g., declare type FormErrors = { email?: string; password?: string; name?:
string } and use it both where state is declared and inside validateForm),
remove the redundant isValid boolean inside validateForm, build newErrors as
before, setFieldErrors(newErrors), and return Object.keys(newErrors).length ===
0 as the validation result (or !Object.values(newErrors).some(Boolean)) so the
function only computes and returns validity based on newErrors.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.Jules/changelog.md.Jules/knowledge.md.Jules/todo.mdweb/pages/Auth.tsx
🧰 Additional context used
📓 Path-based instructions (1)
web/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Web pages should be created as components in
web/pages/and added to routing configuration
Files:
web/pages/Auth.tsx
🧬 Code graph analysis (1)
web/pages/Auth.tsx (1)
web/components/ui/Input.tsx (1)
Input(11-60)
🔇 Additional comments (8)
.Jules/todo.md (1)
149-152: Align the completion note with changelog claims.The changelog (
.Jules/changelog.mdline 10) claims "accessibility support" was added, but this completion note doesn't mention any accessibility improvements.For consistency across documentation, either:
- Update this impact statement to include accessibility improvements if they were actually implemented, or
- Update the changelog to remove the "accessibility support" claim if it wasn't fully addressed
Based on learnings, documentation consistency is important for future reference.
web/pages/Auth.tsx (7)
96-102: LGTM!Validation is correctly invoked before proceeding with the API call, and the early return prevents unnecessary loading states and network requests.
134-138: LGTM!The conditional check before updating state is a good optimization to avoid unnecessary re-renders when the field has no existing error.
254-254: LGTM!Adding
noValidateis the correct approach to suppress native browser validation in favor of custom inline validation.
262-272: LGTM!The error clearing on input change provides good UX by immediately dismissing errors as the user corrects their input.
277-288: LGTM!Consistent error handling pattern applied to the email input.
289-300: LGTM!Consistent error handling pattern applied to the password input.
322-334: LGTM!Clearing both field-specific and general errors when toggling between login and signup is good UX, preventing stale validation messages from the other form mode.
|
@jules In @.Jules/changelog.md: - Line 10: The changelog claim about "accessibility support" is inaccurate because web/pages/Auth.tsx and the Input component only pass errors via the error prop and render messages in a plain div; either update .Jules/changelog.md to remove or tone down the accessibility claim, or implement proper ARIA support: modify the Input component to apply aria-invalid when an error exists, render error elements with stable IDs and expose them via aria-describedby/aria-errormessage on the input, and ensure the Auth page uses an aria-live="assertive" or role="alert" live region for announcing validation errors so screen readers receive immediate feedback. In @.Jules/knowledge.md: - Around line 152-169: The Form Validation pattern is incomplete: extend the validate function (validate) to include concrete rules for email, password, and conditional name checks and ensure it sets per-field errors in fieldErrors; add a clearFieldError(field) helper and call it from each input's onChange so errors clear as the user types; wrap inputs in a with noValidate and wire handleSubmit to call validate before submission; pass per-field error strings to Input via the error prop and add accessibility props (aria-invalid, aria-describedby) to the Input and render an associated error element with role="alert" whose id matches aria-describedby. In @web/pages/Auth.tsx: - Around line 34-61: Extract the duplicated error shape into a shared type alias (e.g., declare type FormErrors = { email?: string; password?: string; name?: string } and use it both where state is declared and inside validateForm), remove the redundant isValid boolean inside validateForm, build newErrors as before, setFieldErrors(newErrors), and return Object.keys(newErrors).length === 0 as the validation result (or !Object.values(newErrors).some(Boolean)) so the function only computes and returns validity based on newErrors. |
Thank you for the detailed feedback. I have implemented the requested changes:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/pages/Auth.tsx (1)
302-310: Addrole="alert"to the server-side error container for accessibility.Field-level errors are properly announced via
Input.tsx, but server-side errors in this container won't be announced to screen reader users.♿ Proposed fix
<motion.div initial={{ opacity: 0, y: -10 }} animate={{ opacity: 1, y: 0 }} - className={`p-3 text-red-600 text-sm font-medium border border-red-100 ${isNeo ? 'bg-red-100 border-2 border-black rounded-none' : 'bg-red-50 rounded-lg'}`} + role="alert" + className={`p-3 text-red-600 text-sm font-medium border border-red-100 ${isNeo ? 'bg-red-100 border-2 border-black rounded-none' : 'bg-red-50 rounded-lg'}`} >
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.Jules/changelog.md.Jules/knowledge.mdweb/components/ui/Input.tsxweb/pages/Auth.tsx
🧰 Additional context used
📓 Path-based instructions (1)
web/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Web pages should be created as components in
web/pages/and added to routing configuration
Files:
web/pages/Auth.tsx
🧬 Code graph analysis (1)
web/pages/Auth.tsx (1)
web/components/ui/Input.tsx (1)
Input(11-63)
🔇 Additional comments (8)
.Jules/changelog.md (1)
10-10: Changelog entry accurately reflects the implementation.The ARIA accessibility features (
aria-invalid,aria-describedby,role="alert") mentioned here are correctly implemented inweb/components/ui/Input.tsx..Jules/knowledge.md (1)
152-191: Good documentation of the validation pattern.The documented pattern accurately reflects the implementation in
Auth.tsxandInput.tsx. This will help maintain consistency across future forms.web/components/ui/Input.tsx (1)
16-16: Well-implemented accessibility pattern.The ARIA linkage is correctly implemented:
errorIdis stable (derived frominputId)aria-describedbyis conditionally set only when error existsrole="alert"ensures screen readers announce errors immediatelyAlso applies to: 41-42, 60-60
web/pages/Auth.tsx (5)
19-23: Type definition is appropriate for the current use case.The
FormErrorstype with optional string fields aligns well with the validation logic andInputcomponent's error prop.
40-61: Clean validation logic.The implementation is straightforward and correctly derives validity from the absence of error keys. The email regex is basic but acceptable for client-side UX—server-side validation should handle edge cases.
96-102: Good integration of validation into submission flow.The early return on validation failure and the typed
clearFieldErrorhelper are well-implemented. CheckingfieldErrors[field]before updating state avoids unnecessary re-renders.Also applies to: 134-138
254-254: Good UX pattern withnoValidateand inline error clearing.Using
noValidateto suppress browser validation bubbles combined with clearing errors on input change provides a polished user experience.Also applies to: 262-272
324-328: Good UX: clearing errors on form mode toggle.Resetting both
fieldErrorsanderrorwhen switching between login and signup prevents stale error messages from persisting.
|
@jules web/pages/Auth.tsx (1) Field-level errors are properly announced via Input.tsx, but server-side errors in this container won't be announced to scree |
Acknowledged. I have added |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Implemented client-side form validation for the Auth page.
fieldErrorsstate to track and display individual field errors.validateFormto check for required fields, email format, and password length.Inputcomponents to receive and display error messages inline.noValidateto the form to suppress default browser validation bubbles..Jules/todo.md,.Jules/changelog.md,.Jules/knowledge.md).PR created automatically by Jules for task 9851474974045613348 started by @Devasy23
Summary by CodeRabbit
New Features
Accessibility
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.