fix: add inline email validation feedback in signup flow#50
Conversation
🚀 PR Received SuccessfullyHello @Ushnika09, Thank you for taking the initiative to contribute to this project. Please ensure that your PR follows all project guidelines properly before requesting review.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughSignupPage implements custom client-side email validation: a new isEmailValid predicate validates input, the form disables browser validation via noValidate, an inline error appears for invalid-but-nonempty emails, and the submit button is disabled until the email is valid. ChangesEmail Validation in Signup Flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/pages/SignupPage.jsx`:
- Around line 102-104: The form uses noValidate so client-side HTML validation
is disabled; update the handleRegister function to perform the same checks the
UI enforces before calling authService.register: validate the email format
(e.g., using the existing email regex or validation util), ensure required
fields (password, confirmPassword) meet criteria and match, and return early (or
set errors) if validation fails; this mirrors the disabled-button guard and
prevents authService.register(...) from being called with invalid input.
- Line 19: The inline heuristic in SignupPage.jsx (const isEmailValid) is too
permissive and should be replaced with a single regex-based predicate that trims
input and tests the full address; create or replace with a validator like
validateEmail(email) and set isEmailValid = validateEmail(email.trim()), using a
standard email regex (RFC-like simple pattern) to gate signup UI/actions instead
of the split/includes logic in the current isEmailValid declaration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c849c008-bae1-48aa-ab97-c6c18276bb12
📒 Files selected for processing (1)
frontend/src/pages/SignupPage.jsx
|
Hey @Ushnika09 👋 Good work on improving the signup UX and adding inline validation feedback. The custom inline error experience is much cleaner than the default browser popup UI. ✨ However, before merging, there are a few important improvements I’d like you to iterate on so the implementation becomes more production-ready and scalable. Required Improvements1. Avoid Manual String-Based Email ValidationCurrently the validation logic is based on manual string checks similar to: const isEmailValid =
email.includes("@") &&
email.split("@")[1]?.includes(".") &&
email.split(".").pop().length > 1;This approach is fragile because malformed emails can still pass validation. Examples: abc @gmail.com
test@.com
hello@gmailSince the input already uses: type="email"the browser already provides robust email validation internally. So instead of manually checking strings, please leverage the browser validity API from the existing Recommended approach: <input
type="email"
value={email}
onChange={(e) => {
setEmail(e.target.value);
setIsEmailValid(e.target.validity.valid);
}}
/>This gives:
2. Keep
|
|
Also, please prepare and verify multiple test cases for the email input field covering as many edge cases as possible before the next commit. Examples:
This will help ensure the validation logic behaves consistently across different scenarios and improves overall robustness of the implementation. |
|
🎥 Loom Walkthrough: https://www.loom.com/share/8255b4a607434d6487e63d75a6e380a7 Covering multiple edge cases for robust validation handling. |
|
@coderabbitai please re-review the latest changes. Addressed the email validation robustness, submit handler safeguards, accessibility improvements, and tested multiple edge cases. |
🧩 Analysis chain🏁 Script executed: Length of output: 4048
✅ What's Been Well Addressed
🔴 Remaining Issue —
|
|
@coderabbitai please re-review the latest changes. |
🧩 Analysis chain🏁 Script executed: Length of output: 3879
✅ Fully Addressed
🔴 One Remaining Bug —
|
|
@Ushnika09 update? |
…o fix/signup-email-inline-validation
|
@coderabbitai please re-review the latest changes. Resolved merge conflicts, preserved the GitHub signup flow alongside email registration, and verified the accessibility + validation improvements are still functioning correctly. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 10708
✅ All Issues Resolved
The (˳◕ᴗ◕)˳ ♡ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
|
@coderabbitai while testing I noticed emails like Updating the validation logic from: return value.trim().length > 0 && validity.valid;to: return (
value.trim().length > 0 &&
validity.valid &&
value.split("@")[1]?.includes(".")
);fixes the issue properly while still using the browser validity API. Please review it. |
|
Tip For best results, initiate chat on the files or code changes.
Why
|
|
Good. This iteration is properly refined now and the validation handling looks much more production-oriented after the edge case analysis and fixes. Maintain this level of debugging depth, implementation quality, and iteration discipline in future contributions as well. |
📌 Fixes #35
✨ Changes Made
aria-invalidsupport for improved accessibilitynoValidateon the form to use custom validation UI🛠️ Implementation Details
isEmailValidvalidation logic🎥 Demo
Loom walkthrough: [Video Demo](https://www.loom.com/share/b37e7c7b243d4cf2abf0bf2d068d52fb?utm_source=chatgpt.com)
✅ Result
Users now receive instant visual feedback for invalid email input, improving accessibility and overall signup experience.
Summary by CodeRabbit