(SP:2) [Frontend] refactor: modularize auth pages and unify layout, fields, and banners#159
Conversation
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughRefactors locale auth pages to delegate UI and logic to new client-side form components; adds reusable auth UI/field components and a getSafeRedirect utility; pages now render respective form components and remove inline submission/state logic; reset-password export dynamic flag removed. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User\n(browser)"
participant LoginForm as "LoginForm\n(component)"
participant API as "Auth API\n(/api/auth/login)"
participant Router as "Router\n(next/navigation)"
User->>LoginForm: Enter email & password, submit
LoginForm->>LoginForm: validate inputs, set loading
LoginForm->>API: POST /api/auth/login {email,password}
API-->>LoginForm: 200 OK / 4xx error (e.g., EMAIL_NOT_VERIFIED)
alt success
LoginForm->>Router: navigate to returnTo or /[locale]/dashboard
else email not verified
LoginForm-->>User: show AuthErrorBanner with resend option
User->>LoginForm: click resendVerification
LoginForm->>API: POST /api/auth/resend-verification {email}
API-->>LoginForm: 200 OK
LoginForm-->>User: show AuthSuccessBanner
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@frontend/components/auth/LoginForm.tsx`:
- Around line 83-95: resendVerification currently ignores failures; wrap the
fetch in a try/catch, check response.ok and parse any error JSON, and update
state accordingly: on success call setVerificationSent(true) and clear errors
(setErrorCode(null), setErrorMessage(null)), on non-ok response set setErrorCode
and setErrorMessage from the parsed body (or a fallback message), and on
network/exception set a generic error code/message; ensure you return or await
the parsed body as needed and don't swallow exceptions so callers and users see
proper error state.
In `@frontend/lib/auth/safe-redirect.ts`:
- Around line 1-11: The getSafeRedirect function allows backslashes which
browsers normalize to slashes and can turn inputs like "/\evil.com" into a
protocol-relative "//evil.com"; update getSafeRedirect to reject any input
containing backslashes by returning an empty string when raw includes "\" (e.g.,
add a check in getSafeRedirect that returns "" if raw.includes("\\") or
otherwise contains a backslash) in addition to the existing checks so
backslash-based open-redirects are blocked.
In `@frontend/project-structure.txt`:
- Around line 75-76: This PR mixes unrelated shop page restructuring into the
auth refactor—revert the moves/added files for shop pages in this branch
(specifically the relocation of error.tsx and page.tsx under shop/orders and the
added page.tsx under shop/products) so the auth-only changes remain; then create
a new branch/PR containing only the shop-related changes (shop/orders/error.tsx,
shop/orders/page.tsx, shop/products/page.tsx) for separate review and possible
rollback.
- Line 352: This PR mixes an unrelated rate-limiting feature into the auth UI
refactor; remove the rate-limiting artifacts (0004_add_api_rate_limits.sql,
0004_snapshot.json, rate-limit.ts) from this branch/commit and put them into a
dedicated feature branch/PR instead: revert or cherry-pick out those three files
from the current branch so the auth refactor only contains UI/auth changes,
create a new branch containing the rate-limit files, add the necessary DB
migration tests and unit/integration tests for rate-limit.ts in that new PR, and
ensure the migration/name (0004) and Drizzle snapshot are correct and validated
in CI before opening the separate PR for review.
🧹 Nitpick comments (9)
frontend/components/auth/fields/PasswordField.tsx (1)
20-27: AddautoCompleteattribute for better browser autofill and security.Password inputs should specify
autoCompleteto help password managers and improve UX. Consider addingautoComplete="current-password"for login or making it configurable via props.Suggested change
type PasswordFieldProps = { name?: string; placeholder?: string; minLength?: number; + autoComplete?: "current-password" | "new-password"; }; export function PasswordField({ name = "password", placeholder = "Password", minLength, + autoComplete = "current-password", }: PasswordFieldProps) { const [visible, setVisible] = useState(false); return ( <div className="relative"> <input name={name} type={visible ? "text" : "password"} placeholder={placeholder} required minLength={minLength} + autoComplete={autoComplete} className="w-full rounded border px-3 py-2 pr-10" />frontend/components/auth/fields/EmailField.tsx (1)
8-21: AddautoComplete="email"for improved autofill support.Similar to the password field, adding
autoCompleteimproves browser autofill behavior for users.Suggested change
return ( <input name="email" type="email" placeholder="Email" required + autoComplete="email" className="w-full rounded border px-3 py-2" onChange={ onChange ? e => onChange(e.target.value) : undefined } /> );frontend/components/auth/AuthErrorBanner.tsx (2)
1-1: Remove commented-out import.Dead code should be removed to keep the codebase clean.
Suggested change
-// import type { ReactNode } from "react"; - type AuthErrorBannerProps = {
14-27: Consider using red/error color scheme instead of yellow/warning.The component is named
AuthErrorBannerbut uses yellow styling typically associated with warnings. For errors, a red color scheme (border-red-400 bg-red-50 text-red-800) would be more semantically appropriate and align with user expectations.If this is intentional (e.g., for softer error presentation), consider renaming to
AuthWarningBanneror adding avariantprop.frontend/components/auth/ForgotPasswordForm.tsx (1)
23-25: Consider simplifying email tracking.The email value is tracked both via
useState(updated byEmailField onChange) and extracted fromFormDataon submit. SincesetEmail(emailValue)on Line 25 re-sets the same value that's already in state (from theonChangehandler), the FormData extraction is redundant for state purposes.You could simplify by using just the
- const formData = new FormData(e.currentTarget); - const emailValue = String(formData.get("email") || ""); - setEmail(emailValue); ... - body: JSON.stringify({ email: emailValue }), + body: JSON.stringify({ email }),However, the current approach does ensure the submitted value matches form state even if they diverge, so this is a minor consideration.
frontend/components/auth/ResetPasswordForm.tsx (3)
14-16: Consider validating the token before rendering the form.The component accepts an empty string token (as passed from the page when
searchParams.get("token")is null). Rendering a password form when no token exists will result in a guaranteed API failure.Consider showing an error state upfront if the token is empty:
💡 Suggested enhancement
export function ResetPasswordForm({ token, }: ResetPasswordFormProps) { + if (!token) { + return ( + <AuthShell title="Set new password"> + <AuthErrorBanner message="Invalid or missing reset link. Please request a new password reset." /> + </AuthShell> + ); + } + const [loading, setLoading] = useState(false);
64-67: Consider adding a login link after successful password reset.After successfully updating their password, users will need to navigate to login. The
AuthSuccessBannersupports afooterprop that could provide this navigation:💡 Suggested enhancement
+import Link from "next/link"; ... <AuthSuccessBanner message="Your password has been updated successfully." + footer={ + <Link href="/login" className="underline"> + Go to login + </Link> + } />
38-41: Type safety:formData.get("password")may benull.The password field value is passed directly to
JSON.stringifywithout null handling. While the<PasswordField>hasrequired, the FormData API can still returnnull.💡 Suggested fix
body: JSON.stringify({ token, - password: formData.get("password"), + password: formData.get("password") ?? "", }),frontend/components/auth/fields/NameField.tsx (1)
6-18: Consider adding accessible labeling for screen readers.The input relies solely on
placeholderfor context, but placeholders disappear on focus and are not announced reliably by all screen readers. For better accessibility, consider adding a visually-hidden label or usingaria-label.♿ Suggested accessibility improvement
export function NameField({ name = "name", placeholder = "Name", }: NameFieldProps) { return ( <input name={name} type="text" placeholder={placeholder} + aria-label={placeholder} required className="w-full rounded border px-3 py-2" /> ); }This pattern would also benefit
EmailFieldandPasswordFieldfor consistent accessibility across all auth field components.
| 📄 error.tsx | ||
| 📄 page.tsx |
There was a problem hiding this comment.
Unrelated shop page restructuring mixed into auth refactoring PR.
These shop-related structural changes appear unrelated to the stated PR objective of refactoring authentication pages:
- Moving
error.tsxandpage.tsxundershop/orders(lines 75-76) - Adding
page.tsxundershop/products(line 81)
Consider separating these shop page changes into a dedicated PR for clearer review and easier rollback if needed.
Also applies to: 81-81
🤖 Prompt for AI Agents
In `@frontend/project-structure.txt` around lines 75 - 76, This PR mixes unrelated
shop page restructuring into the auth refactor—revert the moves/added files for
shop pages in this branch (specifically the relocation of error.tsx and page.tsx
under shop/orders and the added page.tsx under shop/products) so the auth-only
changes remain; then create a new branch/PR containing only the shop-related
changes (shop/orders/error.tsx, shop/orders/page.tsx, shop/products/page.tsx)
for separate review and possible rollback.
There was a problem hiding this comment.
project-structure.txt can be ignored in PRs
There was a problem hiding this comment.
@kryvosheyin, thank you for the clarification! I've noted that project-structure.txt should be ignored during PR reviews.
(\\_/)
(•‿•)
/>📝 Noted!
| 📄 0001_add_payment_attempts.sql | ||
| 📄 0002_clean_martin_li.sql | ||
| 📄 0003_add_stripe_events_claim_lock.sql | ||
| 📄 0004_add_api_rate_limits.sql |
There was a problem hiding this comment.
Unrelated rate limiting feature mixed into auth refactoring PR.
The addition of rate limiting functionality appears unrelated to the auth UI refactoring:
0004_add_api_rate_limits.sql- database migration (line 352)0004_snapshot.json- Drizzle metadata snapshot (line 358)rate-limit.ts- rate limiting implementation (line 415)
Rate limiting is a significant infrastructure/security feature that deserves its own PR with dedicated testing and review. Mixing it with UI refactoring makes both changes harder to review and increases the risk of issues.
Also applies to: 358-358, 415-415
🤖 Prompt for AI Agents
In `@frontend/project-structure.txt` at line 352, This PR mixes an unrelated
rate-limiting feature into the auth UI refactor; remove the rate-limiting
artifacts (0004_add_api_rate_limits.sql, 0004_snapshot.json, rate-limit.ts) from
this branch/commit and put them into a dedicated feature branch/PR instead:
revert or cherry-pick out those three files from the current branch so the auth
refactor only contains UI/auth changes, create a new branch containing the
rate-limit files, add the necessary DB migration tests and unit/integration
tests for rate-limit.ts in that new PR, and ensure the migration/name (0004) and
Drizzle snapshot are correct and validated in CI before opening the separate PR
for review.
There was a problem hiding this comment.
project-structure.txt can be ignored in PRs
There was a problem hiding this comment.
@kryvosheyin, thank you for clarifying! I understand that project-structure.txt is an auto-generated file and shouldn't be part of the review process. I'll make sure to skip it in future reviews.
✏️ Learnings added
Learnt from: kryvosheyin
Repo: DevLoversTeam/devlovers.net PR: 159
File: frontend/project-structure.txt:352-352
Timestamp: 2026-01-19T00:12:01.180Z
Learning: The file `frontend/project-structure.txt` is auto-generated and should be ignored during PR reviews. Changes to this file don't represent actual code changes that need review attention.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
kryvosheyin
force-pushed
the
chore/auth-ui-refactor
branch
from
2a9d63d to
72177c8
Compare
2 participants
This pull request refactors the authentication-related pages (
login,signup,forgot-password, andreset-password) in the frontend to use new, reusable form components and supporting UI blocks. The changes improve code organization, promote reuse, and simplify the page components by moving logic and UI into dedicated components.Componentization and UI improvements:
AuthShell: Provides a consistent layout and title for auth pages.AuthProvidersBlock: Renders OAuth buttons and a separator.AuthErrorBannerandAuthSuccessBanner: Standardized banners for displaying error and success messages. [1] [2]Page refactoring for maintainability:
LoginFormforlogin/page.tsx(frontend/app/[locale]/login/page.tsxL4-L217)SignupFormforsignup/page.tsx(frontend/app/[locale]/signup/page.tsxL4-L195)ForgotPasswordFormforforgot-password/page.tsx(frontend/app/[locale]/forgot-password/page.tsxL2-R6)ResetPasswordFormforreset-password/page.tsx(frontend/app/[locale]/reset-password/page.tsxL3-R10)Authentication logic centralization:
getSafeRedirectutility, ensuring consistent and secure handling of redirect URLs across auth pages. (frontend/app/[locale]/login/page.tsxL4-L217, frontend/app/[locale]/signup/page.tsxL4-L195)These changes make the authentication flow more modular, easier to maintain, and visually consistent.…d banners
How Has This Been Tested?
Screenshots (if applicable)
Checklist
Before submitting
Reviewers
Summary by CodeRabbit
Refactor
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.