[f] 080 - Reset and Forget password page#24
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes in this pull request introduce new components and routing capabilities to the application, specifically for password management. The Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (5)
src/constants/routes.ts (2)
4-4: Consider grouping related route constants together.For better organization and maintainability, consider grouping the authentication-related routes (LOGIN_PATH, LOGOUT_PATH, FORGET_PASSWORD_PATH, RESET_PASSWORD_PATH) together in the file.
const ONBOARDING_PATH = '/onboarding' const LOGIN_PATH = '/login' const LOGOUT_PATH = '/logout' -const FORGET_PASSWORD_PATH = '/forget-password' const ROOT_PATH = '/' const BROADCAST_PATH = `${import.meta.env.VITE_BASE_URL}/broadcasts` -const RESET_PASSWORD_PATH = '/reset-password' +const FORGET_PASSWORD_PATH = '/forget-password' +const RESET_PASSWORD_PATH = '/reset-password'Also applies to: 7-7
4-4: Consider using 'forgot-password' instead of 'forget-password'.The term "forgot password" is more commonly used in user interfaces and follows established UX patterns. This change would align better with user expectations.
-const FORGET_PASSWORD_PATH = '/forget-password' +const FORGOT_PASSWORD_PATH = '/forgot-password'src/components/HeaderBar.tsx (1)
39-62: Enhance dropdown menu accessibilityWhile the implementation is solid, it could benefit from improved accessibility attributes.
Consider these accessibility enhancements:
<DropdownMenu> - <DropdownMenuTrigger asChild> + <DropdownMenuTrigger asChild aria-label="User menu"> <Button variant='ghost' size='icon' className='h-8 w-8 p-0 hover:bg-gray-50'> {user?.user_metadata.avatar_url ? ( <img src={user.user_metadata.avatar_url} alt='User Avatar' className='h-6 w-6 rounded-full' referrerPolicy='no-referrer' /> ) : ( <div className='flex h-6 w-6 items-center justify-center rounded-full bg-blue-600 text-xs text-white' + role="img" + aria-label={`User initials: ${getInitials(user?.email || '')}`} > {getInitials(user?.email || '')} </div> )} </Button> </DropdownMenuTrigger> - <DropdownMenuContent align='end' className='w-48'> + <DropdownMenuContent align='end' className='w-48' aria-label="User menu options">src/components/ForgetPassword.tsx (1)
56-64: Enhance form accessibility.The form could benefit from improved accessibility attributes and error handling.
<form onSubmit={handleSubmit(onSubmit)} className='space-y-4'> <Input id='email' type='email' + aria-label="Email address" + aria-describedby="email-error" placeholder='Your Email' className='w-full rounded-md border border-gray-300 px-4 py-2 focus:border-blue-500 focus:outline-none' {...register('email', { required: 'Email is required' })} /> - {errors.email && <p className='mt-1 text-xs text-red-500'>{errors.email.message}</p>} + {errors.email && <p id="email-error" role="alert" className='mt-1 text-xs text-red-500'>{errors.email.message}</p>}src/components/LoginPage.tsx (1)
101-103: LGTM! Consider enhancing accessibility.The "Forgot password?" navigation is implemented correctly and well-integrated with the existing login form.
Consider these accessibility improvements:
- <Button variant='link' className='h-auto p-0 text-blue-600' onClick={() => navigate('/forget-password')}> + <Button + variant='link' + className='h-auto p-0 text-blue-600' + onClick={() => navigate('/forget-password')} + aria-label="Navigate to forgot password page" + > Forgot password? </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
- src/App.tsx (2 hunks)
- src/components/ForgetPassword.tsx (1 hunks)
- src/components/HeaderBar.tsx (2 hunks)
- src/components/LoginPage.tsx (1 hunks)
- src/components/OnboardingPage.tsx (2 hunks)
- src/components/ResetPassword.tsx (1 hunks)
- src/components/SearchInterface.tsx (0 hunks)
- src/constants/routes.ts (1 hunks)
💤 Files with no reviewable changes (1)
- src/components/SearchInterface.tsx
🧰 Additional context used
🪛 Biome
src/components/ForgetPassword.tsx
[error] 100-100: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (10)
src/constants/routes.ts (1)
9-17: LGTM! Well-organized exports.The export statement is well-structured with:
- One constant per line for better git diff tracking
- Alphabetical ordering for easy scanning
src/App.tsx (2)
Line range hint
1-46: Overall structure maintains good separation of concerns.The changes integrate well with the existing architecture, maintaining a clear separation between public and authenticated routes. The file remains clean and organized.
28-29: Routes are correctly implemented and positioned.The new routes are appropriately placed with other public routes and follow the established pattern. However, since these are password recovery endpoints, we should ensure they're protected against abuse.
Let's verify if rate limiting is implemented for these endpoints:
src/components/HeaderBar.tsx (1)
3-3: LGTM: Import statements are properly structuredThe new imports for the LogOut icon and dropdown menu components are correctly organized and sourced from appropriate locations.
Also applies to: 10-10
src/components/ForgetPassword.tsx (2)
1-11: LGTM! Imports and type definitions are well-structured.The imports are appropriate and the FormData type is well-defined for the form handling requirements.
93-96: Good implementation of email masking for privacy.The email masking implementation helps protect user privacy while still providing enough information for recognition.
src/components/ResetPassword.tsx (2)
1-15: LGTM! Clean imports and type definitions.The imports are well-organized and the FormData type properly defines the expected form structure.
1-176: Verify route configuration and navigation.Let's verify that this component is properly integrated into the application's routing structure.
✅ Verification successful
Route configuration and navigation are properly implemented
The verification confirms that:
- The ResetPassword component is correctly registered in App.tsx with the proper route path
- The route constant RESET_PASSWORD_PATH is properly defined and used
- The component is accessible through a public route (not behind authentication)
- The reset password flow is properly integrated with ForgetPassword component, which includes the correct redirectTo URL
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify route configuration and navigation setup # Check route constant definition echo "Checking for route constant..." rg "RESET_PASSWORD_PATH.*=.*['\"].*['\"]" -g "*.{ts,tsx}" # Check route component registration echo "Checking route registration..." rg "Route.*path.*=.*(?i)reset.*password.*component.*=.*ResetPassword" -g "*.{ts,tsx}" # Check navigation to reset password echo "Checking navigation to reset password..." rg "navigate\(.*(?i)reset.*password.*\)" -g "*.{ts,tsx}"Length of output: 513
Script:
#!/bin/bash # Let's verify the route registration in the router configuration # First check for router setup files echo "Checking router configuration files..." fd "router|routes" -e ts -e tsx # Then check for ResetPassword component registration in router echo "Checking ResetPassword route registration..." rg -A 2 -B 2 "ResetPassword" -g "*router*.{ts,tsx}" # Check for imports of ResetPassword component echo "Checking ResetPassword imports..." rg "import.*ResetPassword" -g "*.{ts,tsx}" # Check for any navigation to /reset-password path echo "Checking navigation to reset-password path..." rg "('/reset-password'|/reset-password/)" -g "*.{ts,tsx}"Length of output: 874
Script:
#!/bin/bash # Let's check the App.tsx for route registration since we found the import there echo "Checking App.tsx route configuration..." rg -A 5 -B 5 "<Route.*ResetPassword" src/App.tsx # Also check for any auth-related route protection echo "Checking for auth protection patterns..." rg -A 3 -B 3 "ProtectedRoute|AuthRoute|RequireAuth" src/App.tsx # Check for any email-related navigation to reset password echo "Checking email-related reset password navigation..." rg "resetPassword|reset.*password" -g "*.{ts,tsx}" -iLength of output: 2485
src/components/OnboardingPage.tsx (2)
11-11: LGTM: Clean import additionThe PublicHeader import follows the existing import organization pattern.
153-153: Verify PublicHeader integration and layoutThe PublicHeader component is appropriately placed, but please verify:
- Visual consistency with other auth pages (login, forget password, reset password)
- Layout spacing and mobile responsiveness with the new header
Consider documenting any required props or configuration for PublicHeader in a comment, even if none are currently needed, to help future maintainers understand the component's interface.
✅ Verification successful
PublicHeader integration is consistent across auth pages
The verification shows that PublicHeader is consistently implemented across all authentication-related pages:
- ResetPassword.tsx
- OnboardingPage.tsx
- ForgetPassword.tsx
All pages follow similar patterns with centered content layouts and appropriate spacing through flex and padding classes. The implementation in OnboardingPage.tsx aligns with the established pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usages of PublicHeader to ensure consistent implementation ast-grep --pattern 'import PublicHeader from "./PublicHeader"' rg -A 2 '<PublicHeader'Length of output: 825
| import PrivateRoute from './components/PrivateRoute' | ||
| import OnboardingPage from './components/OnboardingPage' | ||
| import AuthenticatedLayout from './layouts/AuthenticatedLayout' | ||
| import ForgetPassword from './components/ForgetPassword' |
There was a problem hiding this comment.
Maintain consistent import style across components.
The import styles are inconsistent between ForgetPassword (default import) and ResetPassword (named import). This could indicate inconsistent export patterns in these components.
Consider using the same import style for both components. If they're similar components, they should follow the same pattern:
-import ForgetPassword from './components/ForgetPassword'
-import { ResetPassword } from './components/ResetPassword'
+import { ForgetPassword } from './components/ForgetPassword'
+import { ResetPassword } from './components/ResetPassword'Or:
-import ForgetPassword from './components/ForgetPassword'
-import { ResetPassword } from './components/ResetPassword'
+import ForgetPassword from './components/ForgetPassword'
+import ResetPassword from './components/ResetPassword'Also applies to: 16-16
| } | ||
|
|
||
| const HeaderBar: React.FC<HeaderBarProps> = () => { | ||
| const HeaderBar: React.FC = () => { |
There was a problem hiding this comment.
💡 Codebase verification
Type mismatch in HeaderBar component usage
The verification revealed an issue: AuthenticatedLayout is passing a user prop to HeaderBar (<HeaderBar user={user} />), but HeaderBar is defined without props (React.FC = ()). The component already accesses user data through the useAuth hook internally.
- Remove the
userprop from HeaderBar usage insrc/layouts/AuthenticatedLayout.tsx - The authentication integration itself is properly implemented using Supabase auth and the
useAuthhook
🔗 Analysis chain
Verify authentication integration
Since HeaderBarProps has been removed, we should verify its impact on the codebase.
Let me check the actual implementation and usage of HeaderBar to verify the authentication integration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to HeaderBarProps
rg "HeaderBarProps"
# Check for other components using the HeaderBar component
rg -l "HeaderBar"
Length of output: 102
Script:
#!/bin/bash
# Check HeaderBar implementation
cat src/components/HeaderBar.tsx
# Check how HeaderBar is used in AuthenticatedLayout
cat src/layouts/AuthenticatedLayout.tsx
Length of output: 5994
| const handleLogout = async () => { | ||
| await supabase.auth.signOut() | ||
| } |
There was a problem hiding this comment.
Add error handling to logout function
The current implementation lacks error handling and loading state management. This could lead to a poor user experience if the signOut operation fails.
Consider implementing error handling and loading state:
+ const [isLoggingOut, setIsLoggingOut] = useState(false)
const handleLogout = async () => {
+ try {
+ setIsLoggingOut(true)
await supabase.auth.signOut()
+ } catch (error) {
+ console.error('Error signing out:', error)
+ // Consider adding a toast notification here
+ } finally {
+ setIsLoggingOut(false)
+ }
}Then update the dropdown menu item:
- <DropdownMenuItem className='cursor-pointer' onClick={handleLogout}>
+ <DropdownMenuItem
+ className='cursor-pointer'
+ onClick={handleLogout}
+ disabled={isLoggingOut}
+ >Committable suggestion was skipped due to low confidence.
| export default function ForgetPassword() { | ||
| const [isLoading, setIsLoading] = useState(false) | ||
| const [isSuccess, setIsSuccess] = useState(false) | ||
| const [email, setEmail] = useState('') | ||
|
|
||
| const { | ||
| register, | ||
| handleSubmit, | ||
| formState: { errors } | ||
| } = useForm<FormData>() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider optimizing state management.
The email state variable might be redundant since the email value is already managed by react-hook-form. You could access it from the form data when needed.
export default function ForgetPassword() {
const [isLoading, setIsLoading] = useState(false)
const [isSuccess, setIsSuccess] = useState(false)
- const [email, setEmail] = useState('')
const {
register,
handleSubmit,
+ watch,
formState: { errors }
} = useForm<FormData>()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default function ForgetPassword() { | |
| const [isLoading, setIsLoading] = useState(false) | |
| const [isSuccess, setIsSuccess] = useState(false) | |
| const [email, setEmail] = useState('') | |
| const { | |
| register, | |
| handleSubmit, | |
| formState: { errors } | |
| } = useForm<FormData>() | |
| export default function ForgetPassword() { | |
| const [isLoading, setIsLoading] = useState(false) | |
| const [isSuccess, setIsSuccess] = useState(false) | |
| const { | |
| register, | |
| handleSubmit, | |
| watch, | |
| formState: { errors } | |
| } = useForm<FormData>() |
| <Button | ||
| onClick={() => (window.location.href = '/login')} | ||
| className='w-full rounded-md bg-blue-600 py-2 text-white hover:bg-blue-700 focus:outline-none'> | ||
| Return to Login | ||
| </Button> |
There was a problem hiding this comment.
Improve navigation handling.
Using direct window.location manipulation is not ideal in a React application. Consider using React Router's navigation hooks.
+ import { useNavigate } from 'react-router-dom';
export default function ForgetPassword() {
+ const navigate = useNavigate();
// ... other code ...
<Button
- onClick={() => (window.location.href = '/login')}
+ onClick={() => navigate('/login')}
className='w-full rounded-md bg-blue-600 py-2 text-white hover:bg-blue-700 focus:outline-none'>
Return to Login
</Button>Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 Biome
[error] 100-100: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
| const onSubmit: SubmitHandler<FormData> = async data => { | ||
| setIsLoading(true) | ||
| setEmail(data.email) | ||
|
|
||
| try { | ||
| const { error } = await supabase.auth.resetPasswordForEmail(data.email, { | ||
| redirectTo: `${window.location.origin}/reset-password` | ||
| }) | ||
|
|
||
| if (error) throw error | ||
|
|
||
| setIsSuccess(true) | ||
| } catch (error) { | ||
| // Handle error case | ||
| } finally { | ||
| setIsLoading(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
Enhance error handling and user feedback.
The error handling is incomplete and doesn't provide feedback to users when something goes wrong.
try {
const { error } = await supabase.auth.resetPasswordForEmail(data.email, {
redirectTo: `${window.location.origin}/reset-password`
})
if (error) throw error
setIsSuccess(true)
} catch (error) {
- // Handle error case
+ console.error('Password reset error:', error)
+ toast({
+ title: "Error",
+ description: "Failed to send reset email. Please try again.",
+ variant: "destructive"
+ })
} finally {
setIsLoading(false)
}Committable suggestion was skipped due to low confidence.
| const onSubmit: SubmitHandler<FormData> = async data => { | ||
| setError('') | ||
| setIsLoading(true) | ||
|
|
||
| try { | ||
| const { error } = await supabase.auth.updateUser({ | ||
| password: data.password | ||
| }) | ||
|
|
||
| if (error) { | ||
| throw error | ||
| } | ||
| // Show success message or redirect | ||
| navigate('/login', { | ||
| state: { | ||
| message: 'Password successfully reset. Please login with your new password.' | ||
| } | ||
| }) | ||
| } catch (err) { | ||
| console.error('Error:', err) | ||
| setError(err instanceof Error ? err.message : 'An error occurred while resetting the password.') | ||
| } finally { | ||
| setIsLoading(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
Enhance password validation and error handling.
The form submission logic needs improvements in error handling and password security:
- Missing password strength validation
- Production console.error
- Hardcoded success message
Consider these improvements:
+ const PASSWORD_REQUIREMENTS = {
+ minLength: 8,
+ requireNumbers: true,
+ requireSpecialChars: true,
+ requireUppercase: true
+ }
+ const SUCCESS_MESSAGE = 'Password successfully reset. Please login with your new password.'
const onSubmit: SubmitHandler<FormData> = async data => {
setError('')
setIsLoading(true)
try {
+ // Validate password strength
+ if (!/\d/.test(data.password)) {
+ throw new Error('Password must contain at least one number')
+ }
+ if (!/[!@#$%^&*]/.test(data.password)) {
+ throw new Error('Password must contain at least one special character')
+ }
+ if (!/[A-Z]/.test(data.password)) {
+ throw new Error('Password must contain at least one uppercase letter')
+ }
const { error } = await supabase.auth.updateUser({
password: data.password
})
if (error) {
throw error
}
navigate('/login', {
state: {
- message: 'Password successfully reset. Please login with your new password.'
+ message: SUCCESS_MESSAGE
}
})
} catch (err) {
- console.error('Error:', err)
setError(err instanceof Error ? err.message : 'An error occurred while resetting the password.')
} finally {
setIsLoading(false)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const onSubmit: SubmitHandler<FormData> = async data => { | |
| setError('') | |
| setIsLoading(true) | |
| try { | |
| const { error } = await supabase.auth.updateUser({ | |
| password: data.password | |
| }) | |
| if (error) { | |
| throw error | |
| } | |
| // Show success message or redirect | |
| navigate('/login', { | |
| state: { | |
| message: 'Password successfully reset. Please login with your new password.' | |
| } | |
| }) | |
| } catch (err) { | |
| console.error('Error:', err) | |
| setError(err instanceof Error ? err.message : 'An error occurred while resetting the password.') | |
| } finally { | |
| setIsLoading(false) | |
| } | |
| } | |
| const PASSWORD_REQUIREMENTS = { | |
| minLength: 8, | |
| requireNumbers: true, | |
| requireSpecialChars: true, | |
| requireUppercase: true | |
| } | |
| const SUCCESS_MESSAGE = 'Password successfully reset. Please login with your new password.' | |
| const onSubmit: SubmitHandler<FormData> = async data => { | |
| setError('') | |
| setIsLoading(true) | |
| try { | |
| // Validate password strength | |
| if (!/\d/.test(data.password)) { | |
| throw new Error('Password must contain at least one number') | |
| } | |
| if (!/[!@#$%^&*]/.test(data.password)) { | |
| throw new Error('Password must contain at least one special character') | |
| } | |
| if (!/[A-Z]/.test(data.password)) { | |
| throw new Error('Password must contain at least one uppercase letter') | |
| } | |
| const { error } = await supabase.auth.updateUser({ | |
| password: data.password | |
| }) | |
| if (error) { | |
| throw error | |
| } | |
| navigate('/login', { | |
| state: { | |
| message: SUCCESS_MESSAGE | |
| } | |
| }) | |
| } catch (err) { | |
| setError(err instanceof Error ? err.message : 'An error occurred while resetting the password.') | |
| } finally { | |
| setIsLoading(false) | |
| } | |
| } |
| return ( | ||
| <div className='min-h-screen'> | ||
| <PublicHeader /> | ||
| <div className='mx-auto px-4 py-16'> | ||
| <Card className='mx-auto max-w-lg rounded-xl p-8'> | ||
| <CardHeader className='space-y-3 text-center'> | ||
| <CardTitle className='text-3xl font-bold tracking-tight text-gray-900'>Reset your Password</CardTitle> | ||
| <p className='text-gray-500'>Almost done. Enter your new password below.</p> | ||
| </CardHeader> | ||
|
|
||
| <CardContent className='mt-8'> | ||
| <form onSubmit={handleSubmit(onSubmit)} className='space-y-6'> | ||
| <div className='space-y-6'> | ||
| <div className='relative'> | ||
| <Label className='block text-sm font-medium text-gray-700' htmlFor='password'> | ||
| New Password | ||
| </Label> | ||
| <div className='relative mt-2'> | ||
| <Input | ||
| id='password' | ||
| type={showPassword ? 'text' : 'password'} | ||
| {...register('password', { | ||
| required: 'Password is required', | ||
| minLength: { | ||
| value: 6, | ||
| message: 'Password must be at least 6 characters long' | ||
| } | ||
| })} | ||
| className='block w-full rounded-lg border-gray-300 px-4 py-3 pr-10 focus:border-blue-500 focus:ring-blue-500' | ||
| placeholder='Enter new password' | ||
| /> | ||
| <button | ||
| type='button' | ||
| className='absolute right-3 top-1/2 -translate-y-1/2 text-gray-400 hover:text-gray-600' | ||
| onClick={() => setShowPassword(!showPassword)}> | ||
| {showPassword ? <EyeOff className='h-5 w-5' /> : <Eye className='h-5 w-5' />} | ||
| </button> | ||
| </div> | ||
| {errors.password && <p className='mt-2 text-sm text-red-600'>{errors.password.message}</p>} | ||
| </div> | ||
|
|
||
| <div className='relative'> | ||
| <Label className='block text-sm font-medium text-gray-700' htmlFor='confirmPassword'> | ||
| Confirm Password | ||
| </Label> | ||
| <div className='relative mt-2'> | ||
| <Input | ||
| id='confirmPassword' | ||
| type={showConfirmPassword ? 'text' : 'password'} | ||
| {...register('confirmPassword', { | ||
| required: 'Please confirm your password', | ||
| validate: value => value === password || 'The passwords do not match' | ||
| })} | ||
| className='block w-full rounded-lg border-gray-300 px-4 py-3 pr-10 focus:border-blue-500 focus:ring-blue-500' | ||
| placeholder='Confirm new password' | ||
| /> | ||
| <button | ||
| type='button' | ||
| className='absolute right-3 top-1/2 -translate-y-1/2 text-gray-400 hover:text-gray-600' | ||
| onClick={() => setShowConfirmPassword(!showConfirmPassword)}> | ||
| {showConfirmPassword ? <EyeOff className='h-5 w-5' /> : <Eye className='h-5 w-5' />} | ||
| </button> | ||
| </div> | ||
| {errors.confirmPassword && ( | ||
| <p className='mt-2 text-sm text-red-600'>{errors.confirmPassword.message}</p> | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| {error && ( | ||
| <div className='rounded-md bg-red-50 p-4'> | ||
| <p className='text-sm text-red-600'>{error}</p> | ||
| </div> | ||
| )} | ||
|
|
||
| <Button | ||
| type='submit' | ||
| className='w-full rounded-lg bg-blue-600 px-4 py-3 text-white hover:bg-blue-700 focus:outline-none focus:ring-4 focus:ring-blue-300' | ||
| disabled={isLoading}> | ||
| {isLoading ? ( | ||
| <div className='flex items-center justify-center'> | ||
| <Loader2 className='mr-2 h-5 w-5 animate-spin' /> | ||
| <span>Please wait...</span> | ||
| </div> | ||
| ) : ( | ||
| 'Reset Password' | ||
| )} | ||
| </Button> | ||
| </form> | ||
| </CardContent> | ||
| </Card> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility and user experience.
The UI implementation needs accessibility improvements and could benefit from additional user feedback:
- Missing aria-labels for password visibility toggles
- No password strength indicator
- Missing aria-live region for error messages
Consider these improvements:
<button
type='button'
className='absolute right-3 top-1/2 -translate-y-1/2 text-gray-400 hover:text-gray-600'
+ aria-label={showPassword ? 'Hide password' : 'Show password'}
onClick={() => setShowPassword(!showPassword)}>
{showPassword ? <EyeOff className='h-5 w-5' /> : <Eye className='h-5 w-5' />}
</button>
+ {/* Add password strength indicator */}
+ <div className="mt-2">
+ <div className="h-1 w-full bg-gray-200 rounded">
+ <div
+ className={`h-1 rounded transition-all ${
+ password.length > 8 ? 'bg-green-500 w-full' : 'bg-red-500 w-1/3'
+ }`}
+ />
+ </div>
+ </div>
{error && (
- <div className='rounded-md bg-red-50 p-4'>
+ <div className='rounded-md bg-red-50 p-4' role="alert" aria-live="polite">
<p className='text-sm text-red-600'>{error}</p>
</div>
)}Committable suggestion was skipped due to low confidence.
| useEffect(() => { | ||
| const updateSession = async () => { | ||
| try { | ||
| if (params.access_token && params.refresh_token) { | ||
| const response = await supabase.auth.setSession({ | ||
| access_token: params.access_token, | ||
| refresh_token: params.refresh_token | ||
| }) | ||
| } | ||
| } catch (error) { | ||
| console.log(error) | ||
| setError('Invalid or expired reset link') | ||
| setTimeout(() => navigate('/login'), 3000) | ||
| } | ||
| } | ||
|
|
||
| updateSession() | ||
| }, [params.access_token, params.refresh_token, navigate]) |
There was a problem hiding this comment.
Security and error handling improvements needed.
Several issues need attention in the session handling logic:
- URL fragments containing tokens could be exposed in browser history
- Missing cleanup for the timeout
- Untyped error handling
- Production console.log
Consider these improvements:
useEffect(() => {
const updateSession = async () => {
try {
if (params.access_token && params.refresh_token) {
const response = await supabase.auth.setSession({
access_token: params.access_token,
refresh_token: params.refresh_token
})
}
} catch (error) {
- console.log(error)
+ const errorMessage = error instanceof Error ? error.message : 'Invalid or expired reset link'
setError('Invalid or expired reset link')
- setTimeout(() => navigate('/login'), 3000)
+ const timeout = setTimeout(() => navigate('/login'), 3000)
+ return () => clearTimeout(timeout)
}
}
updateSession()
+ // Clear URL fragment after reading tokens
+ window.history.replaceState(null, '', window.location.pathname)
}, [params.access_token, params.refresh_token, navigate])Also consider implementing a more secure token transfer mechanism, such as using state parameters or secure cookie-based approaches.
Committable suggestion was skipped due to low confidence.
| if (inboxNotifications.length === 0) { | ||
| return <div className='p-4 text-center'>No notifications</div> | ||
| } | ||
| // ... NotificationsList component remains the same ... |
2 participants
Story
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
These changes enhance user experience by providing streamlined access to password recovery options and improving navigation throughout the application.