feat:add password strength indicator to signup form#228
Conversation
Signed-off-by: Akshat Singhai <akshatsinghai6682@gmail.com>
|
@akshatsinghai6682-sketch is attempting to deploy a commit to the Kunal Verma's projects Team on Vercel. A member of the Team first needs to authorize it. |
🎉 Welcome to CodeLens — Thank You for Your Contribution!Hey @akshatsinghai6682-sketch! 👋 We are genuinely excited to have you here. Every single PR — big or small — makes CodeLens better, and yours is no exception. Take a moment to review the checklist below to help us merge your work quickly and smoothly. ✅ Before Requesting a Review
💬 Join Our Community Channel — This is MandatoryBeing part of our communication channel is compulsory for all contributors, not optional. Why join? This is where all important announcements, PR review updates, contribution discussions, and maintainer decisions happen in real time. Contributors who are not in the channel regularly miss critical context and updates, which often leads to duplicated or misaligned work. Staying connected here is what keeps the community strong and your contributions impactful. We are rooting for you! If you have any questions, drop them in the channel or comment right here on this PR. Let's build something great together. 🚀✨ |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesPassword Strength Indicator in Signup Form
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
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)
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: 1
🤖 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 20-24: The password strength validation object includes a
`longLength` criterion (password >= 12 characters) that affects the strength
score, but this criterion is not displayed in the user-facing checklist which
only shows four items (case, number, symbol, and lowercase/uppercase). To fix
this discrepancy, add a checklist item for the `longLength` criterion so users
can see all five criteria that contribute to their password strength score,
ensuring the displayed checks fully match the scoring logic.
🪄 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: bcf7616f-52ea-4522-8907-53dcc18a653f
📒 Files selected for processing (1)
frontend/src/pages/SignupPage.jsx
|
@kunalverma2512 just following up on the pr are there any changes required |
|
@akshatsinghai6682-sketch I will now tomorrow as today it was a bit busy time |
sure @kunalverma2512 |
|
@coderabbitai Please perform a deep, codebase-aware review of this PR. Focus on the following areas:
Please be extremely critical and detailed. Do not limit the review to the changed lines only—analyze the broader impact on the repository and existing functionality. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Repository =="
git remote -v | sed -n '1,4p'
echo
echo "== PR branch status =="
git status --short
echo
echo "== Changed files vs base =="
BASE_REF="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD origin/master 2>/dev/null || true)"
if [ -n "${BASE_REF}" ]; then
git diff --name-status "${BASE_REF}"...HEAD
else
git diff --name-status HEAD~1...HEAD || true
fi
echo
echo "== Diff stat =="
if [ -n "${BASE_REF}" ]; then
git diff --stat "${BASE_REF}"...HEAD
else
git diff --stat HEAD~1...HEAD || true
fi
echo
echo "== Locate SignupPage.jsx =="
fd -a 'SignupPage\.jsx$' .
echo
SIGNUP_FILE="$(fd 'SignupPage\.jsx$' . | head -n 1 || true)"
if [ -n "${SIGNUP_FILE}" ]; then
echo "== AST outline for ${SIGNUP_FILE} =="
ast-grep outline "${SIGNUP_FILE}" --view expanded || true
echo
echo "== Full numbered ${SIGNUP_FILE} =="
nl -ba "${SIGNUP_FILE}" | sed -n '1,280p'
echo
echo "== Remaining numbered ${SIGNUP_FILE} if any =="
nl -ba "${SIGNUP_FILE}" | sed -n '281,560p'
echo
fi
echo "== Diff for SignupPage.jsx =="
if [ -n "${BASE_REF}" ] && [ -n "${SIGNUP_FILE:-}" ]; then
git diff --find-renames -- "${SIGNUP_FILE}" | sed -n '1,260p'
fi
echo
echo "== Find auth/signup frontend call sites =="
rg -n -C3 --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
'\b(signup|signUp|register|create account|createAccount)\b|/auth/register|/auth/signup|/register' .
echo
echo "== Find backend password validation and auth routes =="
rg -n -C4 --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
'\bpassword\b|register|signup|signUp|User\.create|bcrypt|hash' .
echo
echo "== Locate package/config files =="
fd -a 'package\.json$|eslint|vite\.config|tailwind\.config|postcss\.config|README\.md$' .
echo
echo "== Relevant package.json files =="
while IFS= read -r f; do
echo "--- ${f} ---"
sed -n '1,220p' "${f}"
done < <(fd 'package\.json$' .)Length of output: 3245 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Repository =="
git remote -v | sed -n '1,4p'
echo
echo "== PR branch status =="
git status --short
echo
echo "== Changed files vs base =="
BASE_REF="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD origin/master 2>/dev/null || true)"
if [ -n "${BASE_REF}" ]; then
git diff --name-status "${BASE_REF}"...HEAD
else
git diff --name-status HEAD~1...HEAD || true
fi
echo
echo "== Diff stat =="
if [ -n "${BASE_REF}" ]; then
git diff --stat "${BASE_REF}"...HEAD
else
git diff --stat HEAD~1...HEAD || true
fi
echo
echo "== Locate SignupPage.jsx =="
fd -a 'SignupPage\.jsx$' .
echo
SIGNUP_FILE="$(fd 'SignupPage\.jsx$' . | head -n 1 || true)"
if [ -n "${SIGNUP_FILE}" ]; then
echo "== AST outline for ${SIGNUP_FILE} =="
ast-grep outline "${SIGNUP_FILE}" --view expanded || true
echo
echo "== Full numbered ${SIGNUP_FILE} =="
nl -ba "${SIGNUP_FILE}" | sed -n '1,280p'
echo
echo "== Remaining numbered ${SIGNUP_FILE} if any =="
nl -ba "${SIGNUP_FILE}" | sed -n '281,560p'
echo
fi
echo "== Diff for SignupPage.jsx =="
if [ -n "${BASE_REF}" ] && [ -n "${SIGNUP_FILE:-}" ]; then
git diff --find-renames -- "${SIGNUP_FILE}" | sed -n '1,260p'
fi
echo
echo "== Find auth/signup frontend call sites =="
rg -n -C3 --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
'\b(signup|signUp|register|create account|createAccount)\b|/auth/register|/auth/signup|/register' .
echo
echo "== Find backend password validation and auth routes =="
rg -n -C4 --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
'\bpassword\b|register|signup|signUp|User\.create|bcrypt|hash' .
echo
echo "== Locate package/config files =="
fd -a 'package\.json$|eslint|vite\.config|tailwind\.config|postcss\.config|README\.md$' .
echo
echo "== Relevant package.json files =="
while IFS= read -r f; do
echo "--- ${f} ---"
sed -n '1,220p' "${f}"
done < <(fd 'package\.json$' .)Length of output: 3245 🏁 Script executed: #!/bin/bash
set -u
num() { awk '{printf "%6d %s\n", NR, $0}' "$1"; }
BASE_REF="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD origin/master 2>/dev/null || true)"
SIGNUP_FILE="$(fd 'SignupPage\.jsx$' . | head -n 1 || true)"
echo "== Changed files vs base =="
if [ -n "${BASE_REF}" ]; then
git diff --name-status "${BASE_REF}"...HEAD
else
git diff --name-status HEAD~1...HEAD || true
fi
echo
echo "== Full numbered SignupPage.jsx =="
if [ -n "${SIGNUP_FILE}" ]; then
num "${SIGNUP_FILE}" | sed -n '1,420p'
fi
echo
echo "== Diff for SignupPage.jsx =="
if [ -n "${BASE_REF}" ] && [ -n "${SIGNUP_FILE}" ]; then
git diff --find-renames "${BASE_REF}"...HEAD -- "${SIGNUP_FILE}" | sed -n '1,320p'
fi
echo
echo "== Frontend pages/components outline =="
fd '\.(jsx|js|tsx|ts)$' frontend/src 2>/dev/null | sed -n '1,120p'
echo
echo "== Signup/auth frontend context =="
rg -n -C4 --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
'SignupPage|signup|signUp|register|auth/register|auth/signup|create account|Create Account|VITE_API_BASE_URL|localStorage|token' frontend src . 2>/dev/null | sed -n '1,420p'
echo
echo "== Backend auth/password context =="
rg -n -C5 --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
'router\.(post|get).*register|register|signup|signUp|password|bcrypt|jwt|User\.create|User\.findOne|hash' backend server api . 2>/dev/null | sed -n '1,520p'
echo
echo "== Relevant auth/backend files by name =="
fd -i 'auth|user|signup|register|route|controller|model|schema|server|app' . | sed -n '1,220p'
echo
echo "== Package/config files =="
fd 'package\.json$|eslint|vite\.config|tailwind\.config|postcss\.config|README\.md$' . | sed -n '1,120p'
echo
echo "== package.json contents =="
while IFS= read -r f; do
echo "--- ${f} ---"
sed -n '1,220p' "${f}"
done < <(fd 'package\.json$' .)Length of output: 94887 🏁 Script executed: #!/bin/bash
set -u
num() { awk '{printf "%6d %s\n", NR, $0}' "$1"; }
BASE_REF="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD origin/master 2>/dev/null || true)"
SIGNUP_FILE="$(fd 'SignupPage\.jsx$' . | head -n 1 || true)"
echo "== Changed files vs base =="
if [ -n "${BASE_REF}" ]; then
git diff --name-status "${BASE_REF}"...HEAD
else
git diff --name-status HEAD~1...HEAD || true
fi
echo
echo "== Full numbered SignupPage.jsx =="
if [ -n "${SIGNUP_FILE}" ]; then
num "${SIGNUP_FILE}" | sed -n '1,420p'
fi
echo
echo "== Diff for SignupPage.jsx =="
if [ -n "${BASE_REF}" ] && [ -n "${SIGNUP_FILE}" ]; then
git diff --find-renames "${BASE_REF}"...HEAD -- "${SIGNUP_FILE}" | sed -n '1,320p'
fi
echo
echo "== Frontend pages/components outline =="
fd '\.(jsx|js|tsx|ts)$' frontend/src 2>/dev/null | sed -n '1,120p'
echo
echo "== Signup/auth frontend context =="
rg -n -C4 --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
'SignupPage|signup|signUp|register|auth/register|auth/signup|create account|Create Account|VITE_API_BASE_URL|localStorage|token' frontend src . 2>/dev/null | sed -n '1,420p'
echo
echo "== Backend auth/password context =="
rg -n -C5 --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
'router\.(post|get).*register|register|signup|signUp|password|bcrypt|jwt|User\.create|User\.findOne|hash' backend server api . 2>/dev/null | sed -n '1,520p'
echo
echo "== Relevant auth/backend files by name =="
fd -i 'auth|user|signup|register|route|controller|model|schema|server|app' . | sed -n '1,220p'
echo
echo "== Package/config files =="
fd 'package\.json$|eslint|vite\.config|tailwind\.config|postcss\.config|README\.md$' . | sed -n '1,120p'
echo
echo "== package.json contents =="
while IFS= read -r f; do
echo "--- ${f} ---"
sed -n '1,220p' "${f}"
done < <(fd 'package\.json$' .)Length of output: 91721 🏁 Script executed: #!/bin/bash
set -u
num() { awk '{printf "%6d %s\n", NR, $0}' "$1"; }
for f in \
frontend/src/services/authService.js \
frontend/src/services/api.js \
frontend/src/context/AuthContext.jsx \
server/modules/auth/routes.js \
server/modules/auth/controller.js \
server/modules/auth/service.js \
server/modules/auth/validation.js \
server/models/User.js
do
if [ -f "$f" ]; then
echo "== ${f} =="
num "$f" | sed -n '1,260p'
echo
else
echo "== missing ${f} =="
fi
done
echo "== frontend eslint config =="
if [ -f frontend/eslint.config.js ]; then
num frontend/eslint.config.js | sed -n '1,220p'
fi
echo "== frontend package.json =="
if [ -f frontend/package.json ]; then
num frontend/package.json | sed -n '1,220p'
fiLength of output: 45643 🏁 Script executed: #!/bin/bash
set -u
num() { awk '{printf "%6d %s\n", NR, $0}' "$1"; }
for f in \
frontend/src/services/authService.js \
frontend/src/services/api.js \
frontend/src/context/AuthContext.jsx \
server/modules/auth/routes.js \
server/modules/auth/controller.js \
server/modules/auth/service.js \
server/modules/auth/validation.js \
server/models/User.js
do
if [ -f "$f" ]; then
echo "== ${f} =="
num "$f" | sed -n '1,260p'
echo
else
echo "== missing ${f} =="
fi
done
echo "== frontend eslint config =="
if [ -f frontend/eslint.config.js ]; then
num frontend/eslint.config.js | sed -n '1,220p'
fi
echo "== frontend package.json =="
if [ -f frontend/package.json ]; then
num frontend/package.json | sed -n '1,220p'
fiLength of output: 45643
Deep PR Review: Password Strength MeterI reviewed the changed file plus related auth/signup flow:
Final VerdictNot quite ready to merge as-is. The implementation is close and does not appear to break backend behavior, but I would fix one correctness/UX issue before merge: the current scoring can label a password Strong even when one of the visible core criteria is still unmet. Must Fix Before Merge1.
|
| Category | Rating | Notes |
|---|---|---|
| Correctness | 7/10 | Core feature works, but Strong can be inaccurate when a visible criterion is missing. |
| Code Quality | 7/10 | Simple implementation, but too much logic/component code is inline in the page. |
| Maintainability | 6/10 | Should extract component and scoring helper before this grows further. |
| Security | 8/10 | No backend/security regression; keeps 6-char policy. Meter is advisory only. |
| Performance | 10/10 | Regex checks on each keystroke are trivial. |
| UI/UX | 7/10 | Good visual integration, but accessibility and scoring consistency need polish. |
| Scope Adherence | 8/10 | Mostly scoped; minor unrelated formatting/hook-comment noise. |
| Overall Merge Readiness | 7/10 | Close, but I recommend fixing the misleading Strong classification first. |
Future Issue Discovery
Issue 1: Extract reusable PasswordStrengthMeter
Problem: Password strength UI is embedded in SignupPage.jsx.
Why it matters: Reset password and future change-password flows may need the same feedback.
Expected impact: Better reuse, smaller page component, easier testing.
Suggested implementation:
- Create
frontend/src/components/auth/PasswordStrengthMeter.jsx. - Move
PasswordStrengthMeterandStrengthCheckItem. - Import it in
SignupPage.jsx.
Issue 2: Add password strength utility tests
Problem: Password scoring has edge cases and currently no test coverage.
Why it matters: Classification logic can regress easily, especially around 8+ vs 12+ length and hidden bonus criteria.
Expected impact: Prevents misleading labels like Strong with missing visible criteria.
Suggested implementation:
- Extract
getPasswordStrength. - Add tests for representative weak/medium/strong passwords.
- Explicitly test that
Strongrequires the intended core criteria.
Issue 3: Align password guidance across signup, reset password, and account settings
Problem: Signup now gives strength feedback, but reset password likely still only enforces minimum length.
Why it matters: Users should receive consistent password guidance everywhere passwords are created.
Expected impact: Better security posture and consistent UX.
Suggested implementation:
- Reuse the extracted meter in forgot/reset password flow.
- Keep backend minimum at 6 unless a separate policy-change issue is approved.
Issue 4: Improve form accessibility across auth pages
Problem: Existing auth forms rely heavily on visual styling and could better connect labels, errors, and dynamic feedback.
Why it matters: Accessibility affects keyboard and screen-reader users.
Expected impact: Better WCAG alignment and more robust auth UX.
Suggested implementation:
- Add explicit
htmlFor/idpairs for labels and inputs. - Use
aria-describedbyfor errors and helper text. - Ensure dynamic status updates are meaningful and not noisy.
Issue 5: Clean up outdated auth documentation
Problem: Some README snippets mention localStorage tokens, but current code uses HttpOnly cookies via withCredentials.
Why it matters: Inconsistent docs can mislead contributors working on auth.
Expected impact: Reduces onboarding confusion and prevents accidental reintroduction of token-in-localStorage patterns.
Suggested implementation:
- Update auth docs to reflect cookie-based session management.
- Remove stale localStorage references where no longer accurate.
Recommended Merge Path
Before merge:
- Fix password strength classification so
Strongcannot appear while a visible core criterion is missing. - Add newline at EOF.
- Remove or resolve TODO comments.
Preferably before merge, but acceptable as follow-up if you want to keep this PR small:
- Extract
PasswordStrengthMetertofrontend/src/components/auth/PasswordStrengthMeter.jsx. - Extract
getPasswordStrengthto a utility. - Add scoring tests.
- Improve
aria-describedbyand checklist semantics.
|
@coderabbitai just tell for this issue i.e for scope of ths PR whats left to proceed to ready to merge |
|
Tip For best results, initiate chat on the files or code changes.
1. Fix the misleading
|
|
@akshatsinghai6682-sketch See this comment and resolve issue 1 , and remove those comments said in issue 2 and dont do that extract work and resolve issue 3 |
Signed-off-by: Akshat Singhai <akshatsinghai6682@gmail.com>
|
@kunalverma2512 I've addressed the requested changes:
The changes have been pushed. Please review when you get a chance. Thank you! |
|
@coderabbitai please review the latest commit and check with previous dicsussion that is this read to merge now ? also check if this got merged then will it created any chaos in production while sign up ? i.e any backend issue will come with this frontend work done in this PR |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -u
num() { awk '{printf "%6d %s\n", NR, $0}' "$1"; }
SIGNUP_FILE="$(fd 'SignupPage\.jsx$' . | head -n 1 || true)"
echo "== File: ${SIGNUP_FILE} =="
num "${SIGNUP_FILE}"
echo
echo "== EOF check (last 3 bytes hex) =="
tail -c 3 "${SIGNUP_FILE}" | xxd
echo
echo "== TODO check =="
rg -n 'TODO' "${SIGNUP_FILE}" || echo "No TODOs found"
echo
echo "== getPasswordStrength / label logic =="
rg -n 'Strong|Medium|Weak|visibleScore|longLength|label' "${SIGNUP_FILE}"
echo
echo "== Latest git log (last 3 commits) =="
git log --oneline -3
echo
echo "== Diff vs base for SignupPage.jsx =="
BASE_REF="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD origin/master 2>/dev/null || true)"
if [ -n "${BASE_REF}" ] && [ -n "${SIGNUP_FILE}" ]; then
git diff --find-renames "${BASE_REF}"...HEAD -- "${SIGNUP_FILE}"
fiLength of output: 26424
Status of the 3 Must-Fix Items
Fix
|
|
@akshatsinghai6682-sketch I think now its ready to merge but take care next time to give EOF in changed files |
|
ok @kunalverma2512 thanks for the suggestion and your valuable review |
📌 Pull Request Summary
🔗 Related Issue
Closes #227
📝 Description
Adds a real-time password strength indicator to the sign-up form, giving users live feedback on password quality before submitting.
Changes Made
PasswordStrengthMetercomponent scoring password strength (Weak / Medium / Strong) based on length, mixed case, numbers, and symbolsSignupPage.jsx, rendering below the password inputborder-4 border-black, bold uppercase type, no border-radius) — no new colors introducedMotivation
The sign-up form previously gave no feedback on password strength until after submission. This adds live, actionable guidance so users know how to improve a weak password before creating their account.
🚀 Type of Change
🧪 Testing
Verification
Test Details
Tested by entering passwords of varying strength (e.g.
abc,Abcdefgh1,Abc123!@#) and confirming the bar, label, and checklist update live and correctly. Confirmed existing form validation (6+ character minimum, submit button disabled state) is unaffected.📸 Screenshots / Demo (If Applicable)
✅ Checklist
📚 Additional Notes
None.
Summary by CodeRabbit