add the auth security password strength meter and validation to signup form#96
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces ChangesPassword Strength Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/Firebase/SignUp.tsx (2)
30-35: 💤 Low valueConsider using
PasswordScoretype for thescoreprop.For consistency with the
passwordStrengthmodule and to enforce the valid 0–5 range at compile time, use the exportedPasswordScoretype instead ofnumber.♻️ Proposed change
+import { + analysePassword, + isPasswordAcceptable, + STRENGTH_COLORS, + METER_BAR_COLORS, + type PasswordStrength, + type PasswordScore, +} from './passwordStrength'; + interface StrengthMeterProps { password: string; strength: PasswordStrength; - score: number; + score: PasswordScore; visible: boolean; }🤖 Prompt for 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. In `@src/Firebase/SignUp.tsx` around lines 30 - 35, The StrengthMeterProps interface defines the score prop with type number, which lacks type safety for the valid password strength range. Replace the number type with the PasswordScore type for the score prop in the StrengthMeterProps interface to enforce the 0-5 range at compile time and maintain consistency with the passwordStrength module.
101-107: ⚡ Quick winAvoid redundant
analysePasswordcall by passing requirements as a prop.The parent
SignUpcomponent already callsanalysePassword(password)at line 226. Calling it again here duplicates computation on every keystroke. Pass therequirementsarray as a prop instead.♻️ Proposed change
Update the props interface:
interface RequirementsChecklistProps { password: string; visible: boolean; + requirements: PasswordRequirement[]; } -function RequirementsChecklist({ password, visible }: RequirementsChecklistProps) { - const { requirements } = analysePassword(password); +function RequirementsChecklist({ password, visible, requirements }: RequirementsChecklistProps) {Then in SignUp, pass the already-computed requirements:
<RequirementsChecklist password={password} visible={showStrengthUI} requirements={requirements} />🤖 Prompt for 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. In `@src/Firebase/SignUp.tsx` around lines 101 - 107, The RequirementsChecklist component duplicates password analysis computation by calling analysePassword on every render, when the parent SignUp component already computes this at line 226. Add a requirements property to the RequirementsChecklistProps interface, remove the analysePassword call from inside the RequirementsChecklist function body, and instead use the requirements value passed as a prop. Finally, update the SignUp component where RequirementsChecklist is instantiated to pass the already-computed requirements as a prop along with password and visible.
🤖 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 `@src/Firebase/passwordStrength.test.ts`:
- Around line 175-177: The test case description "returns 1 when only min-length
criterion is met" is inaccurate because the assertion expects a score of 2,
which corresponds to both minimum length and lowercase criteria being satisfied
for the input 'abcdefgh'. Update the test description in the it() function to
accurately reflect that both minimum length and lowercase criteria are being
validated, not just the minimum length criterion alone.
In `@src/Firebase/passwordStrength.ts`:
- Around line 56-58: The regex pattern in the hasSpecialChar function contains
unnecessary escape characters that trigger ESLint warnings. Remove the backslash
before the square bracket in \[ and before the forward slash in \/ since these
characters do not require escaping when used inside a character class. Update
the regex pattern to remove these unnecessary escape sequences while maintaining
the same validation logic for special characters.
---
Nitpick comments:
In `@src/Firebase/SignUp.tsx`:
- Around line 30-35: The StrengthMeterProps interface defines the score prop
with type number, which lacks type safety for the valid password strength range.
Replace the number type with the PasswordScore type for the score prop in the
StrengthMeterProps interface to enforce the 0-5 range at compile time and
maintain consistency with the passwordStrength module.
- Around line 101-107: The RequirementsChecklist component duplicates password
analysis computation by calling analysePassword on every render, when the parent
SignUp component already computes this at line 226. Add a requirements property
to the RequirementsChecklistProps interface, remove the analysePassword call
from inside the RequirementsChecklist function body, and instead use the
requirements value passed as a prop. Finally, update the SignUp component where
RequirementsChecklist is instantiated to pass the already-computed requirements
as a prop along with password and visible.
🪄 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 Plus
Run ID: f3b3383c-5610-426f-8cb7-79fc45fc465d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.jsonsrc/Firebase/SignUp.tsxsrc/Firebase/passwordStrength.test.tssrc/Firebase/passwordStrength.tsvite.config.ts
| it('returns 1 when only min-length criterion is met', () => { | ||
| expect(calculatePasswordScore('abcdefgh')).toBe(2); // length + lowercase | ||
| }); |
There was a problem hiding this comment.
Test case description contradicts the asserted behavior.
At Line 175, the description says “only min-length criterion is met,” but Line 176 correctly expects 2 for 'abcdefgh' (minLength + lowercase). Please rename the test to match the behavior.
🤖 Prompt for 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.
In `@src/Firebase/passwordStrength.test.ts` around lines 175 - 177, The test case
description "returns 1 when only min-length criterion is met" is inaccurate
because the assertion expects a score of 2, which corresponds to both minimum
length and lowercase criteria being satisfied for the input 'abcdefgh'. Update
the test description in the it() function to accurately reflect that both
minimum length and lowercase criteria are being validated, not just the minimum
length criterion alone.
description
add the auth security password strength meter and validation to signup form
add the password strengh.ts
User types a password >>analysePassword("Passw0rd!")
↓
┌──────────────────────────────┐
│ hasMinLength → ✓ (1pt) │
│ hasUppercase → ✓ (1pt) │
│ hasLowercase → ✓ (1pt) │
│ hasDigit → ✓ (1pt) │
│ hasSpecialChar → ✓ (1pt) │
└──────────────────────────────┘
↓
score = 5 → classifyStrength() → "Strong"
Scoring table:
enhance the signup.tsx , The original form had just a plain password . Here's what was added on top of it, without breaking anything:
-- Segmented Strength Bar
-- Live Requirements Checklist
-- Dynamic Border Colour
-- Weak Password Alert (on Submit)
-- Submit Button Feedback
-- Accessibility
closes #92
Summary by CodeRabbit