-
Notifications
You must be signed in to change notification settings - Fork 2
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
[feat] implented confirm password #31
Changes from all commits
b98864f
a7e0ce6
cd31373
13e047c
69deefd
3f9a82c
0a45e41
0be3008
c2f53ae
84dcea1
da31f06
f78988e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,73 @@ | ||
'use client'; | ||
|
||
import { useState } from 'react'; | ||
import { useRouter } from 'next/navigation'; | ||
import { useAuth } from '../../../utils/AuthProvider'; | ||
import PasswordComplexity from '@/components/PasswordComplexity'; | ||
import PasswordInput from '@/components/PasswordInput'; | ||
import { useAuth } from '@/utils/AuthProvider'; | ||
|
||
export default function SignUp() { | ||
const { signUp } = useAuth(); | ||
const [email, setEmail] = useState(''); | ||
const [password, setPassword] = useState(''); | ||
const router = useRouter(); | ||
const [password, setPassword] = useState<string>(''); | ||
const [confirmPassword, setConfirmPassword] = useState<string>(''); | ||
const [passwordError, setPasswordError] = useState(''); | ||
const [passwordComplexityError, setPasswordComplexityError] = useState< | ||
string | null | ||
>(null); | ||
const [passwordComplexity, setPasswordComplexity] = useState<boolean>(false); | ||
const [showPassword, setShowPassword] = useState(false); | ||
const [showConfirmPassword, setShowConfirmPassword] = useState(false); | ||
|
||
// Handles input to password | ||
const handlePasswordChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you could refactor this to
Then, on line 93, you can do |
||
const newPassword = e.target.value; | ||
setPassword(newPassword); | ||
validatePasswords(newPassword, confirmPassword); | ||
validatePasswordComplexity(newPassword); | ||
}; | ||
|
||
// Handles input to confirm password | ||
const handleConfirmPasswordChange = ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see comment about refactoring/simplfying handlePasswordChange - we can do smth similar here ! |
||
e: React.ChangeEvent<HTMLInputElement>, | ||
) => { | ||
const newConfirmPassword = e.target.value; | ||
setConfirmPassword(newConfirmPassword); | ||
validatePasswords(password, newConfirmPassword); | ||
}; | ||
|
||
// Checks if passwords match and sets error | ||
const validatePasswords = ( | ||
password: string | null, | ||
confirmPassword: string | null, | ||
) => { | ||
if (password !== confirmPassword) { | ||
setPasswordError('Passwords do not match.'); | ||
} else { | ||
setPasswordError(''); // Clear error when passwords match | ||
} | ||
}; | ||
|
||
// Set password complexity error if requirements are not met | ||
const validatePasswordComplexity = (password: string | null) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when would password be null? if nothing is typed, we the password can be an empty string |
||
const hasLowerCase = /[a-z]/.test(password || ''); | ||
const hasNumber = /\d/.test(password || ''); | ||
const longEnough = (password || '').length >= 8; | ||
|
||
if (password && hasLowerCase && hasNumber && longEnough) { | ||
setPasswordComplexity(true); | ||
setPasswordComplexityError(null); // Clear error if all conditions are met | ||
} else if (password) { | ||
setPasswordComplexity(false); | ||
setPasswordComplexityError('Password must meet complexity requirements'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just flagging that it makes more sense for this error to only show up if user tries to submit without meeting requirements. it's fine for now but we may want to adjust the logic in the future |
||
} else { | ||
setPasswordComplexity(false); | ||
setPasswordComplexityError(null); // Clear error if password is empty | ||
} | ||
}; | ||
|
||
const handleSignUp = async () => { | ||
// Define handleSignUp | ||
try { | ||
if (password) { | ||
await signUp(email, password); | ||
router.push('/'); | ||
} catch (error) { | ||
if (error instanceof Error) { | ||
console.error(error.message); | ||
} | ||
} | ||
}; | ||
|
||
|
@@ -30,24 +79,45 @@ export default function SignUp() { | |
value={email} | ||
placeholder="Email" | ||
/> | ||
<input | ||
type="password" | ||
name="password" | ||
onChange={e => setPassword(e.target.value)} | ||
{/* Email input*/} | ||
<PasswordInput | ||
value={password} | ||
onChange={handlePasswordChange} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see comment about refactoring handlePasswordChange and handleConfirmPasswordChange |
||
placeholder="Password" | ||
isVisible={showPassword} | ||
toggleVisibility={() => setShowPassword(!showPassword)} | ||
name="password" | ||
/> | ||
<input | ||
type="password" | ||
name="Confirm Password" | ||
onChange={e => setPassword(e.target.value)} | ||
value={password} | ||
{/* Password input with toggle visibility */} | ||
<PasswordInput | ||
value={confirmPassword} | ||
onChange={handleConfirmPasswordChange} | ||
placeholder="Confirm Password" | ||
isVisible={showConfirmPassword} | ||
toggleVisibility={() => setShowConfirmPassword(!showConfirmPassword)} | ||
name="confirmPassword" | ||
/> | ||
<button type="button" onClick={handleSignUp}> | ||
{/* Confirm password input with toggle visibility */} | ||
<button | ||
type="button" | ||
onClick={handleSignUp} | ||
disabled={!email || !!passwordError || !!passwordComplexityError} | ||
> | ||
Sign up | ||
</button>{' '} | ||
{/* Sign up button */} | ||
{confirmPassword && passwordError && ( | ||
<p style={{ color: 'red' }}>{passwordError}</p> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. eventually we want to avoid inline styling, and we can use the global text styles once that's merged -just flagging this for now |
||
)} | ||
{/* Conditional password validation error message */} | ||
<PasswordComplexity | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. while the "Passwords Do Not Match" error is in the correct place rn, to match designs, |
||
password={password || ''} // Set default value if password is null | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if password is not able to be of type null, we don't need this additional |
||
/> | ||
{/* Password complexity requirements */} | ||
{password && !passwordComplexity && passwordComplexityError && ( | ||
<p style={{ color: 'red' }}>{passwordComplexityError}</p> | ||
)} | ||
{/* Password complexity error message */} | ||
</> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
export default function PasswordComplexity({ password }: { password: string }) { | ||
// Display requirements if there is input | ||
if (password.length > 0) { | ||
return ( | ||
<div> | ||
<Requirement | ||
met={/[a-z]/.test(password)} | ||
text="At least 1 lowercase character" | ||
/> | ||
<Requirement met={/\d/.test(password)} text="At least 1 number" /> | ||
<Requirement met={password.length >= 8} text="At least 8 characters" /> | ||
</div> | ||
); | ||
} | ||
|
||
return null; | ||
} | ||
|
||
// Helper component to display each requirement with conditional styling | ||
function Requirement({ met, text }: { met: boolean; text: string }) { | ||
return ( | ||
<p style={{ color: met ? 'green' : 'red' }}> | ||
{met ? '✓' : '✗'} {text} | ||
</p> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
import React from 'react'; | ||
|
||
interface PasswordInputProps { | ||
value: string | null; | ||
onChange: (e: React.ChangeEvent<HTMLInputElement>) => void; | ||
placeholder: string; | ||
isVisible: boolean; | ||
toggleVisibility: () => void; | ||
name: string; | ||
} | ||
|
||
const PasswordInput: React.FC<PasswordInputProps> = ({ | ||
value, | ||
onChange, | ||
placeholder, | ||
isVisible, | ||
toggleVisibility, | ||
name, | ||
}) => { | ||
return ( | ||
<div style={{ position: 'relative' }}> | ||
<input | ||
type={isVisible ? 'text' : 'password'} | ||
name={name} | ||
onChange={onChange} | ||
value={value || ''} | ||
placeholder={placeholder} | ||
/> | ||
<button type="button" onClick={toggleVisibility}> | ||
{isVisible ? 'Hide' : 'Show'} | ||
</button> | ||
</div> | ||
); | ||
}; | ||
|
||
export default PasswordInput; |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
would it be easier to change these variables to more descriptive names? lowkey got a bit confused haha, maybe
isPasswordComplexityMet
or something similar would help to differentiate it frompasswordComplexityError
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.
good point! agreed about renaming