UI overhaul#20
Merged
Merged
Conversation
- New amber design system via CSS variables (--accent, --bg, --surface, etc.) - Dark mode via class on <html> with FOUC prevention script - ThemeToggle (sun/moon) with localStorage persistence - Sticky header with ThemeToggle and clean layout - BookmarkCard: hover-reveal delete, amber tags, fade-up animation - SaveUrlForm, SearchBar: use design tokens throughout - Auth page: branded with amber logo, card layout - Loading skeleton: matches new layout - Staggered card animation on main page Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…size Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Full landing page at / with hero, features, how it works, resurfacing callout, CTA, footer - Hero: animated blobs, staggered headline, mockup card with floating animation - Scrolling marquee social proof strip - Feature cards with hover lift + glow - /app route for authenticated bookmarks (moved from /) - Auth redirects to /app after login - robots.ts + sitemap.ts for SEO - CSS variables scoped to .landing-page to avoid conflicts with app theme Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Landing page with hero, features, FAQ, and smart / routing (redirects authed users to /app) - Full CSS variable design token system with light/dark mode support - New logo, favicon, and amber accent brand identity - /app route with Header, BookmarkCard, SearchBar, SaveUrlForm, skeleton loading - BookmarkCardSkeleton component for Suspense fallback and route-level loading.tsx - Custom 404 page (not-found.tsx) with amber glow and themed buttons - Auth page show/hide password toggle using lucide-react Eye/EyeOff icons - Empty state in bookmark list with Lucide Bookmark icon - Header background fixed to use var(--bg)/80 instead of hardcoded dark color - Supabase SSR session refresh via proxy.ts (Next.js 16.2 pattern) - Document titles use | separator consistently across all pages - lucide-react added as dependency Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Docs & deps CLAUDE.md, client/package.json |
Updated milestone/status and pre-M8 checklist in CLAUDE.md; added lucide-react dependency in client/package.json. |
Root landing & app split client/src/app/page.tsx, client/src/app/app/page.tsx |
Converted root page to marketing LandingPage; added protected server-rendered /app page with metadata, Supabase SSR auth check, and Prisma bookmark fetch/render (redirects unauthenticated users to /auth). |
Auth flows & callback client/src/app/auth/page.tsx, client/src/app/auth/callback/route.ts |
Auth page: added OAuth flow (handleOAuth, oauthLoading state), password visibility toggle, and redirects to /app; added OAuth GET callback route to exchange code for Supabase session and redirect safely. |
SSR proxy & matcher client/src/proxy.ts |
Expanded config.matcher to `"/((?!_next/static |
SEO endpoints client/src/app/robots.ts, client/src/app/sitemap.ts |
Added robots() and sitemap() route handlers returning robots rules and sitemap entries. |
Layout, fonts & globals client/src/app/layout.tsx, client/src/app/globals.css |
Reworked font imports/metadata and injected themeScript to initialize .dark; replaced previous theme with CSS token-based theming, added animations, reduced-motion handling, landing-page token overrides, and new font utility classes. |
Loading, not-found & skeletons client/src/app/loading.tsx, client/src/app/app/loading.tsx, client/src/app/not-found.tsx |
Reworked global and app-specific loading skeletons; added NotFound 404 page with decorative blobs and navigation links. |
Components & UX client/src/components/*, client/src/components/BookmarkCard.tsx, client/src/components/BookmarkCardSkeleton.tsx, client/src/components/SaveUrlForm.tsx, client/src/components/SearchBar.tsx, client/src/components/Header.tsx, client/src/components/ThemeToggle.tsx, client/src/components/AnimateOnScroll.tsx |
Added BookmarkCardSkeleton, ThemeToggle, AnimateOnScroll; BookmarkCard gained delete handler and hostname parsing; SearchBar uses hostname helper and removed router; SaveUrlForm and Header restyled; header now sticky with theme toggle and logout redirects to /. |
Misc pages & metadata client/src/app/app/page.tsx, client/src/app/page.tsx, client/src/app/robots.ts, client/src/app/sitemap.ts |
Added page metadata, landing content, and sitemap/robots routes; adjusted routes and imports to reflect landing/app split. |
Sequence Diagram(s)
sequenceDiagram
participant Client
participant NextProxy as "Next.js proxy"
participant Supabase as Supabase
participant AppServer as "Server components / Prisma"
Client->>NextProxy: HTTP request (path + cookies)
NextProxy->>Supabase: createServerClient(cookies) / getUser()
Supabase-->>NextProxy: session / user?
alt path starts with /app and no user
NextProxy->>Client: Redirect -> /auth
else allowed or user present
NextProxy->>AppServer: render requested server page
AppServer->>Prisma: fetch bookmarks for userId
Prisma-->>AppServer: bookmarks
AppServer-->>Client: HTML with bookmarks or landing content
end
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
- M7: Supabase Auth #11 — Overlaps proxy/middleware and SSR auth gating changes.
- M4: bookmark list UI renders saved items #2 — Related edits to bookmark listing, server rendering, and Prisma usage.
- feat: add SaveUrlForm to page for in-browser #4 — Related changes to SaveUrlForm and its integration on the app page.
🚥 Pre-merge checks | ❌ 3
❌ Failed checks (2 warnings, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | The PR description is largely incomplete; the 'What does this PR do?' section is empty despite being a required template section. | Add a brief description explaining what the UI overhaul includes, such as the new theme system, landing page redesign, dark mode support, and authentication flow improvements. | |
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Title check | ❓ Inconclusive | The title 'UI overhaul' is vague and generic, lacking specific information about the main changes. | Provide a more specific title that highlights a key change, such as 'Redesign landing page with new theme system and auth improvements' or 'Add dark mode toggle and refactor app landing page UI'. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
ui-overhaul
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (3)
client/src/app/robots.ts (1)
1-6: Consider using an environment variable for the base URL.The sitemap URL is hardcoded, which could cause issues across different environments (development, staging, production). Using an environment variable would make this more maintainable.
♻️ Suggested improvement
+import type { MetadataRoute } from "next"; + +const BASE_URL = process.env.NEXT_PUBLIC_BASE_URL || "https://recallsave.vercel.app"; + -export default function robots() { +export default function robots(): MetadataRoute.Robots { return { rules: { userAgent: "*", allow: "/" }, - sitemap: "https://recallsave.vercel.app/sitemap.xml", + sitemap: `${BASE_URL}/sitemap.xml`, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/robots.ts` around lines 1 - 6, The robots() function currently returns a hardcoded sitemap URL; change it to build the sitemap from an environment variable (e.g., NEXT_PUBLIC_BASE_URL or VERCEL_URL with https:// prefix) so it works across environments: read the env var inside robots(), ensure it falls back to a sensible default (localhost or empty) and concatenate "/sitemap.xml" to produce the sitemap value, and update the returned object accordingly (reference function name robots).client/src/app/sitemap.ts (1)
1-16: Same hardcoded URL concern; also add explicit return type for consistency.Similar to
robots.ts, the base URL is hardcoded. For consistency and maintainability across environments, consider extracting this to a shared constant or environment variable.♻️ Suggested improvement
+import type { MetadataRoute } from "next"; + +const BASE_URL = process.env.NEXT_PUBLIC_BASE_URL || "https://recallsave.vercel.app"; + -export default function sitemap() { +export default function sitemap(): MetadataRoute.Sitemap { return [ { - url: "https://recallsave.vercel.app", + url: BASE_URL, lastModified: new Date(), changeFrequency: "monthly" as const, priority: 1, }, { - url: "https://recallsave.vercel.app/auth", + url: `${BASE_URL}/auth`, lastModified: new Date(), changeFrequency: "yearly" as const, priority: 0.5, }, ]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/sitemap.ts` around lines 1 - 16, The sitemap() function currently hardcodes the base URL and lacks an explicit return type; extract the base URL into a shared constant or environment variable (e.g., NEXT_PUBLIC_BASE_URL or a shared getBaseUrl/getSiteConfig) and use it to construct the two URLs inside sitemap(), add an explicit return type for sitemap (e.g., MetadataRoute.Sitemap or a typed array of {url:string,lastModified:Date,changeFrequency:string,priority:number}) for consistency, and update robots.ts to reference the same shared constant so both files use a single source of truth.client/src/components/BookmarkCard.tsx (1)
29-35: Extract the hostname formatter into a shared helper.This parser now exists here and in
client/src/components/SearchBar.tsx, and the two call sites are already diverging on fallback behavior. Moving it into a shared utility will keep cards and search results consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/BookmarkCard.tsx` around lines 29 - 35, Extract the inline hostname parser used in BookmarkCard (the IIFE that assigns hostname) into a shared utility function (e.g., formatHostname or getHostname) exported from a common helper module and replace the IIFE with a call to that helper; ensure the helper implements the same logic (new URL(bookmark.url).hostname.replace(/^www\./, "") inside try/catch) and define a single, consistent fallback return (the original raw URL), then update the other call site(s) that currently duplicate this logic to call the same helper so both BookmarkCard and SearchBar use identical parsing and fallback behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/app/app/page.tsx`:
- Around line 15-19: The initial query using prisma.bookmark.findMany that
populates bookmarks (variable bookmarks) currently fetches all user rows
including tags; limit the result set by adding pagination/cursoring (e.g.,
take/skip or take + cursor) or at minimum a first-page limit (take: N) and
return a nextCursor/token so the UI components (SaveUrlForm, BookmarkCard) can
request subsequent pages; update the server handler that renders the /app page
to accept page/cursor params and adjust any client-side refreshes to use paged
refetches rather than reloading the entire bookmarks list.
In `@client/src/app/globals.css`:
- Around line 144-164: Rename the keyframes to kebab-case and remove the unused
one: change `@keyframes` fadeUp to `@keyframes` fade-up-lg, change `@keyframes`
blobDrift to `@keyframes` blob-drift, and delete the `@keyframes` shimmerBorder
entirely; then update all references in client/src/app/page.tsx replacing
"fadeUp" with "fade-up-lg" (6 occurrences) and "blobDrift" with "blob-drift" (3
occurrences) so the animation names match, and ensure no other code references
shimmerBorder before committing.
In `@client/src/app/loading.tsx`:
- Around line 26-37: The staggered delay is applied to the parent div (the
container with key={i}) but the actual CSS animation class is on the child
elements (animate-skeleton), so animationDelay has no effect; fix by either
moving the animationDelay style onto each animated child (add style={{
animationDelay: `${i * 60}ms` }} to the three child divs that have className
"animate-skeleton") or give the parent container the "animate-skeleton" class so
its animation responds to its style; update the container or the child elements
accordingly to ensure the intended staggered effect.
In `@client/src/app/page.tsx`:
- Around line 431-443: Update the stale footer text and remove placeholder
broken links: change the hardcoded "© 2025" inside the footer span to the
current year (or derive it dynamically), and replace or hide any link entries in
the mapped array that use the placeholder href "#" (the array mapped in the JSX
and the <a> render logic) — either point them to real routes like "/privacy" and
"/terms" or filter them out/conditionally render them until those pages exist;
keep the external GitHub link behavior (target/_blank + rel) as-is.
- Around line 452-492: The injected stylesheet applies continuous animations
(blobDrift, float) and repeated entrance transitions but lacks a
prefers-reduced-motion escape hatch; update the style block in page.tsx to add a
`@media` (prefers-reduced-motion: reduce) rule that targets the animated selectors
(e.g., elements using blobDrift, float, and classes like .feature-card,
.nav-cta, or any selector that uses animation/transition) and force-disable
motion by setting animation: none !important; transition: none !important;
transform: none !important; and reduce any keyframe-driven motion, ensuring
users with prefers-reduced-motion get a static experience.
In `@client/src/components/AnimateOnScroll.tsx`:
- Around line 6-24: The effect in AnimateOnScroll currently assumes
IntersectionObserver and leaves [data-animate] elements hidden if JS/observer is
unavailable; update the useEffect in the AnimateOnScroll component to (1) add a
JS-enabled marker by calling document.documentElement.classList.add('js') on
mount (so CSS can scope hidden styles to .js [data-animate]), and (2) if
window.IntersectionObserver is not present, immediately mark all queried
elements as visible (e.g., add the "visible" class or remove the data-animate
attribute) instead of relying on the observer; preserve the existing observer
logic when available and still disconnect it on unmount.
In `@client/src/components/BookmarkCard.tsx`:
- Around line 124-130: The delete button is hidden with "opacity-0
group-hover:opacity-100", making it inaccessible on touch and keyboard; update
the class on the button (the element using onClick={deleteBookmark}) to make it
visible for focus and non-hover interactions—replace "opacity-0
group-hover:opacity-100" with something like "opacity-100
group-hover:opacity-100 focus:opacity-100 group-focus:opacity-100
focus-visible:opacity-100" (or remove the initial opacity-0 entirely) so the
delete affordance is discoverable for keyboard and touch users while preserving
the hover behavior.
In `@client/src/components/SearchBar.tsx`:
- Around line 87-90: The subtitle currently always renders hostname(b.url),
which loses disambiguating path info; update the subtitle rendering in
SearchBar.tsx (the span that uses hostname(b.url)) to instead display a
normalized, truncated URL path or full URL when the path is non-root: derive let
u = new URL(b.url) and show u.pathname + u.search (and u.hash optionally)
truncated, falling back to hostname(b.url) when pathname === '/' or empty; keep
the existing CSS classes and truncation behavior so the UI stays clean while
preserving more URL detail for bookmarks with identical hostnames.
In `@client/src/components/ThemeToggle.tsx`:
- Around line 6-25: Initialize the ThemeToggle's dark state from the same
preloaded/hydrated theme source used by the app layout instead of defaulting to
false on SSR: change the useState initialization for dark to derive its initial
value synchronously (e.g., via a lazy initializer that reads the client/hydrated
theme value or localStorage with a window guard) so the correct theme is
rendered on first paint, keep the existing useEffect only for syncing if needed,
and ensure the toggle function (toggle) still updates
document.documentElement.classList and localStorage; also expose the control
state to assistive tech by adding aria-pressed={dark} to the button so screen
readers see the actual pressed/state value.
In `@client/src/proxy.ts`:
- Around line 15-29: The redirect is creating a fresh NextResponse and losing
cookies that Supabase set via the custom setAll implementation; update setAll to
write to both res.cookies and req.cookies (or request.cookies) and when blocking
unauthenticated access return the mutated res object instead of calling
NextResponse.redirect(...) directly (i.e. use the existing res and set its
status/location or use NextResponse.redirect but start from the mutated res),
ensuring supabase.auth.getUser()'s cookie mutations on res are preserved; locate
the setAll function, the res.cookies usage, the supabase.auth.getUser() call,
and the redirect branch to make this change.
- Around line 27-36: The middleware path check using
req.nextUrl.pathname.startsWith("/app") should be made segment-aware: replace it
with a check like path === "/app" || path.startsWith("/app/") (i.e., update the
condition where user is checked and NextResponse.redirect is returned) to avoid
matching routes such as /application; also update export const config.matcher to
broaden exclusions so middleware does not run for metadata/static files (exclude
exact files like /robots.txt and /sitemap.xml and common static extensions such
as .txt, .xml, .png, .jpg, .jpeg, .svg, .ico, .webmanifest, etc.) so auth work
is skipped for those requests.
---
Nitpick comments:
In `@client/src/app/robots.ts`:
- Around line 1-6: The robots() function currently returns a hardcoded sitemap
URL; change it to build the sitemap from an environment variable (e.g.,
NEXT_PUBLIC_BASE_URL or VERCEL_URL with https:// prefix) so it works across
environments: read the env var inside robots(), ensure it falls back to a
sensible default (localhost or empty) and concatenate "/sitemap.xml" to produce
the sitemap value, and update the returned object accordingly (reference
function name robots).
In `@client/src/app/sitemap.ts`:
- Around line 1-16: The sitemap() function currently hardcodes the base URL and
lacks an explicit return type; extract the base URL into a shared constant or
environment variable (e.g., NEXT_PUBLIC_BASE_URL or a shared
getBaseUrl/getSiteConfig) and use it to construct the two URLs inside sitemap(),
add an explicit return type for sitemap (e.g., MetadataRoute.Sitemap or a typed
array of {url:string,lastModified:Date,changeFrequency:string,priority:number})
for consistency, and update robots.ts to reference the same shared constant so
both files use a single source of truth.
In `@client/src/components/BookmarkCard.tsx`:
- Around line 29-35: Extract the inline hostname parser used in BookmarkCard
(the IIFE that assigns hostname) into a shared utility function (e.g.,
formatHostname or getHostname) exported from a common helper module and replace
the IIFE with a call to that helper; ensure the helper implements the same logic
(new URL(bookmark.url).hostname.replace(/^www\./, "") inside try/catch) and
define a single, consistent fallback return (the original raw URL), then update
the other call site(s) that currently duplicate this logic to call the same
helper so both BookmarkCard and SearchBar use identical parsing and fallback
behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d1d87dd5-9e3c-410e-893d-376ebaa491b9
⛔ Files ignored due to path filters (3)
client/package-lock.jsonis excluded by!**/package-lock.jsonclient/public/favicon.svgis excluded by!**/*.svgclient/public/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (20)
CLAUDE.mdclient/package.jsonclient/src/app/app/loading.tsxclient/src/app/app/page.tsxclient/src/app/auth/page.tsxclient/src/app/globals.cssclient/src/app/layout.tsxclient/src/app/loading.tsxclient/src/app/not-found.tsxclient/src/app/page.tsxclient/src/app/robots.tsclient/src/app/sitemap.tsclient/src/components/AnimateOnScroll.tsxclient/src/components/BookmarkCard.tsxclient/src/components/BookmarkCardSkeleton.tsxclient/src/components/Header.tsxclient/src/components/SaveUrlForm.tsxclient/src/components/SearchBar.tsxclient/src/components/ThemeToggle.tsxclient/src/proxy.ts
Comment on lines
+15
to
+19
| const bookmarks = await prisma.bookmark.findMany({ | ||
| where: { userId }, | ||
| orderBy: { createdAt: "desc" }, | ||
| include: { tags: true }, | ||
| }); |
There was a problem hiding this comment.
Cap the initial bookmark query.
This loads every bookmark plus every tag for the user, and the route is refreshed again after mutations from SaveUrlForm and BookmarkCard. Once an account grows, /app becomes an unbounded DB query and HTML payload on every save/delete. Add pagination, cursoring, or at least a first-page limit here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/app/page.tsx` around lines 15 - 19, The initial query using
prisma.bookmark.findMany that populates bookmarks (variable bookmarks) currently
fetches all user rows including tags; limit the result set by adding
pagination/cursoring (e.g., take/skip or take + cursor) or at minimum a
first-page limit (take: N) and return a nextCursor/token so the UI components
(SaveUrlForm, BookmarkCard) can request subsequent pages; update the server
handler that renders the /app page to accept page/cursor params and adjust any
client-side refreshes to use paged refetches rather than reloading the entire
bookmarks list.
Comment on lines
+144
to
+164
| @keyframes fadeUp { | ||
| from { opacity: 0; transform: translateY(20px); } | ||
| to { opacity: 1; transform: translateY(0); } | ||
| } | ||
| @keyframes float { | ||
| 0%, 100% { transform: translateY(0px); } | ||
| 50% { transform: translateY(-12px); } | ||
| } | ||
| @keyframes blobDrift { | ||
| 0%, 100% { transform: translate(0,0) scale(1); } | ||
| 33% { transform: translate(40px,-30px) scale(1.08); } | ||
| 66% { transform: translate(-30px,20px) scale(0.95); } | ||
| } | ||
| @keyframes marquee { | ||
| from { transform: translateX(0); } | ||
| to { transform: translateX(-50%); } | ||
| } | ||
| @keyframes shimmerBorder { | ||
| 0% { background-position: 0% 50%; } | ||
| 100% { background-position: 200% 50%; } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for usages of the renamed keyframes
rg -n "fadeUp|blobDrift|shimmerBorder" --type css --type tsx --type tsRepository: MinitJain/recall
Length of output: 87
🏁 Script executed:
# First, let's verify the file exists and read the relevant lines
head -n 180 client/src/app/globals.css | tail -n 40Repository: MinitJain/recall
Length of output: 1136
🏁 Script executed:
# Search for all usages of the keyframe names (fixing the rg command - use extensions instead of file types)
rg -n "fadeUp|blobDrift|shimmerBorder|fade-up" --glob "*.css" --glob "*.tsx" --glob "*.ts" --glob "*.jsx" --glob "*.js"Repository: MinitJain/recall
Length of output: 1967
Keyframe names should use kebab-case for consistency, and unused keyframes should be removed.
Stylelint reports that fadeUp, blobDrift, and shimmerBorder violate the keyframes-name-pattern rule. Additionally, these keyframes have inline references in client/src/app/page.tsx that must be updated:
fadeUp(6 references at lines 72, 89, 104, 113, 138, 144)blobDrift(3 references at lines 52, 59, 66)shimmerBorderis defined but never used and should be removed
♻️ Rename to kebab-case and remove unused keyframes
-@keyframes fadeUp {
+@keyframes fade-up-lg {
from { opacity: 0; transform: translateY(20px); }
to { opacity: 1; transform: translateY(0); }
}
`@keyframes` float {
0%, 100% { transform: translateY(0px); }
50% { transform: translateY(-12px); }
}
-@keyframes blobDrift {
+@keyframes blob-drift {
0%, 100% { transform: translate(0,0) scale(1); }
33% { transform: translate(40px,-30px) scale(1.08); }
66% { transform: translate(-30px,20px) scale(0.95); }
}
`@keyframes` marquee {
from { transform: translateX(0); }
to { transform: translateX(-50%); }
}
-@keyframes shimmerBorder {
- 0% { background-position: 0% 50%; }
- 100% { background-position: 200% 50%; }
-}Then update references in client/src/app/page.tsx: replace "fadeUp" with "fade-up-lg" and "blobDrift" with "blob-drift".
🧰 Tools
🪛 Stylelint (17.5.0)
[error] 144-144: Expected keyframe name "fadeUp" to be kebab-case (keyframes-name-pattern)
(keyframes-name-pattern)
[error] 152-152: Expected keyframe name "blobDrift" to be kebab-case (keyframes-name-pattern)
(keyframes-name-pattern)
[error] 161-161: Expected keyframe name "shimmerBorder" to be kebab-case (keyframes-name-pattern)
(keyframes-name-pattern)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/globals.css` around lines 144 - 164, Rename the keyframes to
kebab-case and remove the unused one: change `@keyframes` fadeUp to `@keyframes`
fade-up-lg, change `@keyframes` blobDrift to `@keyframes` blob-drift, and delete the
`@keyframes` shimmerBorder entirely; then update all references in
client/src/app/page.tsx replacing "fadeUp" with "fade-up-lg" (6 occurrences) and
"blobDrift" with "blob-drift" (3 occurrences) so the animation names match, and
ensure no other code references shimmerBorder before committing.
Comment on lines
26
to
37
| <div | ||
| key={i} | ||
| className="border border-zinc-200 dark:border-zinc-800 rounded-xl p-4 flex flex-col gap-3" | ||
| style={{ animationDelay: `${i * 80}ms` }} | ||
| className="rounded-xl border border-[var(--border)] bg-[var(--surface)] p-4 flex gap-4" | ||
| style={{ animationDelay: `${i * 60}ms` }} | ||
| > | ||
| {/* Title line */} | ||
| <div | ||
| className="h-4 bg-zinc-200 dark:bg-zinc-800 rounded animate-pulse" | ||
| style={{ width: `${65 + (i % 3) * 10}%` }} | ||
| /> | ||
| {/* Description line */} | ||
| <div className="h-3 w-4/5 bg-zinc-200 dark:bg-zinc-800 rounded animate-pulse" /> | ||
| {/* URL + tags row */} | ||
| <div className="flex gap-2"> | ||
| <div className="h-3 w-24 bg-zinc-200 dark:bg-zinc-800 rounded animate-pulse" /> | ||
| <div className="h-3 w-12 bg-zinc-200 dark:bg-zinc-800 rounded-full animate-pulse" /> | ||
| <div className="h-3 w-16 bg-zinc-200 dark:bg-zinc-800 rounded-full animate-pulse" /> | ||
| <div className="w-16 h-16 flex-shrink-0 rounded-lg bg-[var(--surface-2)] animate-skeleton" /> | ||
| <div className="flex flex-col gap-2 flex-1"> | ||
| <div className="h-4 bg-[var(--surface-2)] rounded animate-skeleton" style={{ width: `${55 + (i % 3) * 12}%` }} /> | ||
| <div className="h-3 w-4/5 bg-[var(--surface-2)] rounded animate-skeleton" /> | ||
| <div className="h-3 w-24 bg-[var(--surface-2)] rounded animate-skeleton" /> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
animationDelay has no effect on the parent container.
The animationDelay style on Line 29 is applied to the card container, but this element has no animation class. The animate-skeleton class is on the child elements, so the stagger effect won't work as intended.
Either move the animation to the parent or apply individual delays to the child elements:
🐛 Option 1: Apply delay to animated children
<div
key={i}
className="rounded-xl border border-[var(--border)] bg-[var(--surface)] p-4 flex gap-4"
- style={{ animationDelay: `${i * 60}ms` }}
>
- <div className="w-16 h-16 flex-shrink-0 rounded-lg bg-[var(--surface-2)] animate-skeleton" />
- <div className="flex flex-col gap-2 flex-1">
- <div className="h-4 bg-[var(--surface-2)] rounded animate-skeleton" style={{ width: `${55 + (i % 3) * 12}%` }} />
- <div className="h-3 w-4/5 bg-[var(--surface-2)] rounded animate-skeleton" />
- <div className="h-3 w-24 bg-[var(--surface-2)] rounded animate-skeleton" />
+ <div className="w-16 h-16 flex-shrink-0 rounded-lg bg-[var(--surface-2)] animate-skeleton" style={{ animationDelay: `${i * 60}ms` }} />
+ <div className="flex flex-col gap-2 flex-1">
+ <div className="h-4 bg-[var(--surface-2)] rounded animate-skeleton" style={{ width: `${55 + (i % 3) * 12}%`, animationDelay: `${i * 60}ms` }} />
+ <div className="h-3 w-4/5 bg-[var(--surface-2)] rounded animate-skeleton" style={{ animationDelay: `${i * 60}ms` }} />
+ <div className="h-3 w-24 bg-[var(--surface-2)] rounded animate-skeleton" style={{ animationDelay: `${i * 60}ms` }} />
</div>
</div>🐛 Option 2: Add animation class to parent container
<div
key={i}
- className="rounded-xl border border-[var(--border)] bg-[var(--surface)] p-4 flex gap-4"
+ className="rounded-xl border border-[var(--border)] bg-[var(--surface)] p-4 flex gap-4 animate-fade-in"
style={{ animationDelay: `${i * 60}ms` }}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/loading.tsx` around lines 26 - 37, The staggered delay is
applied to the parent div (the container with key={i}) but the actual CSS
animation class is on the child elements (animate-skeleton), so animationDelay
has no effect; fix by either moving the animationDelay style onto each animated
child (add style={{ animationDelay: `${i * 60}ms` }} to the three child divs
that have className "animate-skeleton") or give the parent container the
"animate-skeleton" class so its animation responds to its style; update the
container or the child elements accordingly to ensure the intended staggered
effect.
Comment on lines
+431
to
+443
| <div style={{ display: "flex", alignItems: "center", gap: 10 }}> | ||
| <span style={{ width: 16, height: 16, borderRadius: 4, background: "var(--lp-accent)", display: "inline-block" }} /> | ||
| <span style={{ fontSize: 13, color: "var(--lp-text-muted)" }}> | ||
| <span className="font-display" style={{ fontWeight: 700, color: "var(--lp-text-primary)", marginRight: 6 }}>Recall</span> | ||
| © 2025. All rights reserved. | ||
| </span> | ||
| </div> | ||
| <div style={{ display: "flex", gap: 24 }}> | ||
| {[["Privacy", "#"], ["Terms", "#"], ["GitHub", "https://github.com/MinitJain/recall"]].map(([label, href]) => ( | ||
| <a key={label} href={href} target={href.startsWith("http") ? "_blank" : undefined} rel="noopener noreferrer" | ||
| style={{ fontSize: 13, color: "var(--lp-text-muted)", textDecoration: "none" }}> | ||
| {label} | ||
| </a> |
There was a problem hiding this comment.
Replace the placeholder footer content before launch.
As of March 26, 2026, © 2025 is already stale, and href="#" for Privacy/Terms renders broken legal links on the public landing page. Either wire these to real routes or hide them until the pages exist.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/page.tsx` around lines 431 - 443, Update the stale footer text
and remove placeholder broken links: change the hardcoded "© 2025" inside the
footer span to the current year (or derive it dynamically), and replace or hide
any link entries in the mapped array that use the placeholder href "#" (the
array mapped in the JSX and the <a> render logic) — either point them to real
routes like "/privacy" and "/terms" or filter them out/conditionally render them
until those pages exist; keep the external GitHub link behavior (target/_blank +
rel) as-is.
Comment on lines
+452
to
+492
| <style>{` | ||
| .stats-grid { grid-template-columns: repeat(3, 1fr); } | ||
| .nav-buttons { display: flex; align-items: center; gap: 12px; } | ||
| .nav-signin { | ||
| font-size: 14px; font-weight: 500; color: var(--lp-text-secondary); | ||
| text-decoration: none; padding: 8px 16px; | ||
| border: 1px solid var(--lp-border-strong); border-radius: 10px; | ||
| transition: color 0.15s, border-color 0.15s; | ||
| } | ||
| .nav-signin:hover { color: var(--lp-text-primary); border-color: var(--lp-accent); } | ||
| .nav-cta { | ||
| font-size: 14px; font-weight: 600; color: #0A0A08; | ||
| text-decoration: none; padding: 8px 18px; | ||
| background: var(--lp-accent); border-radius: 10px; | ||
| box-shadow: var(--lp-shadow-glow); | ||
| transition: background 0.15s, transform 0.15s; | ||
| } | ||
| .nav-cta:hover { background: var(--lp-accent-warm); transform: scale(1.02); } | ||
| .problem-grid { grid-template-columns: 1fr 1fr; } | ||
| .features-grid { grid-template-columns: repeat(3, 1fr); } | ||
| .steps-grid { grid-template-columns: repeat(3, 1fr); } | ||
| .resurface-grid { grid-template-columns: repeat(3, 1fr); } | ||
| .steps-line { display: block; } | ||
| .feature-card:hover { | ||
| border-color: var(--lp-accent) !important; | ||
| box-shadow: var(--lp-shadow-glow) !important; | ||
| transform: translateY(-2px) !important; | ||
| } | ||
| @media (max-width: 768px) { | ||
| .stats-grid { grid-template-columns: 1fr !important; gap: 24px !important; } | ||
| .nav-buttons { display: none; } | ||
| .problem-grid { grid-template-columns: 1fr !important; gap: 40px !important; } | ||
| .features-grid { grid-template-columns: repeat(2, 1fr) !important; } | ||
| .steps-grid { grid-template-columns: 1fr !important; } | ||
| .resurface-grid { grid-template-columns: 1fr !important; } | ||
| .steps-line { display: none !important; } | ||
| } | ||
| @media (max-width: 480px) { | ||
| .features-grid { grid-template-columns: 1fr !important; } | ||
| } | ||
| `}</style> |
There was a problem hiding this comment.
Add a reduced-motion escape hatch for the new landing-page animations.
This page now ships continuous motion (blobDrift, float) plus repeated entrance animations, but the injected stylesheet never disables them for prefers-reduced-motion: reduce. Motion-sensitive users will still get the full animated experience.
Suggested fix
`@media` (max-width: 480px) {
.features-grid { grid-template-columns: 1fr !important; }
}
+ `@media` (prefers-reduced-motion: reduce) {
+ .landing-page *,
+ .landing-page *::before,
+ .landing-page *::after {
+ animation: none !important;
+ transition: none !important;
+ scroll-behavior: auto !important;
+ }
+ }
`}</style>📝 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.
Suggested change
| <style>{` | |
| .stats-grid { grid-template-columns: repeat(3, 1fr); } | |
| .nav-buttons { display: flex; align-items: center; gap: 12px; } | |
| .nav-signin { | |
| font-size: 14px; font-weight: 500; color: var(--lp-text-secondary); | |
| text-decoration: none; padding: 8px 16px; | |
| border: 1px solid var(--lp-border-strong); border-radius: 10px; | |
| transition: color 0.15s, border-color 0.15s; | |
| } | |
| .nav-signin:hover { color: var(--lp-text-primary); border-color: var(--lp-accent); } | |
| .nav-cta { | |
| font-size: 14px; font-weight: 600; color: #0A0A08; | |
| text-decoration: none; padding: 8px 18px; | |
| background: var(--lp-accent); border-radius: 10px; | |
| box-shadow: var(--lp-shadow-glow); | |
| transition: background 0.15s, transform 0.15s; | |
| } | |
| .nav-cta:hover { background: var(--lp-accent-warm); transform: scale(1.02); } | |
| .problem-grid { grid-template-columns: 1fr 1fr; } | |
| .features-grid { grid-template-columns: repeat(3, 1fr); } | |
| .steps-grid { grid-template-columns: repeat(3, 1fr); } | |
| .resurface-grid { grid-template-columns: repeat(3, 1fr); } | |
| .steps-line { display: block; } | |
| .feature-card:hover { | |
| border-color: var(--lp-accent) !important; | |
| box-shadow: var(--lp-shadow-glow) !important; | |
| transform: translateY(-2px) !important; | |
| } | |
| @media (max-width: 768px) { | |
| .stats-grid { grid-template-columns: 1fr !important; gap: 24px !important; } | |
| .nav-buttons { display: none; } | |
| .problem-grid { grid-template-columns: 1fr !important; gap: 40px !important; } | |
| .features-grid { grid-template-columns: repeat(2, 1fr) !important; } | |
| .steps-grid { grid-template-columns: 1fr !important; } | |
| .resurface-grid { grid-template-columns: 1fr !important; } | |
| .steps-line { display: none !important; } | |
| } | |
| @media (max-width: 480px) { | |
| .features-grid { grid-template-columns: 1fr !important; } | |
| } | |
| `}</style> | |
| <style>{` | |
| .stats-grid { grid-template-columns: repeat(3, 1fr); } | |
| .nav-buttons { display: flex; align-items: center; gap: 12px; } | |
| .nav-signin { | |
| font-size: 14px; font-weight: 500; color: var(--lp-text-secondary); | |
| text-decoration: none; padding: 8px 16px; | |
| border: 1px solid var(--lp-border-strong); border-radius: 10px; | |
| transition: color 0.15s, border-color 0.15s; | |
| } | |
| .nav-signin:hover { color: var(--lp-text-primary); border-color: var(--lp-accent); } | |
| .nav-cta { | |
| font-size: 14px; font-weight: 600; color: `#0A0A08`; | |
| text-decoration: none; padding: 8px 18px; | |
| background: var(--lp-accent); border-radius: 10px; | |
| box-shadow: var(--lp-shadow-glow); | |
| transition: background 0.15s, transform 0.15s; | |
| } | |
| .nav-cta:hover { background: var(--lp-accent-warm); transform: scale(1.02); } | |
| .problem-grid { grid-template-columns: 1fr 1fr; } | |
| .features-grid { grid-template-columns: repeat(3, 1fr); } | |
| .steps-grid { grid-template-columns: repeat(3, 1fr); } | |
| .resurface-grid { grid-template-columns: repeat(3, 1fr); } | |
| .steps-line { display: block; } | |
| .feature-card:hover { | |
| border-color: var(--lp-accent) !important; | |
| box-shadow: var(--lp-shadow-glow) !important; | |
| transform: translateY(-2px) !important; | |
| } | |
| `@media` (max-width: 768px) { | |
| .stats-grid { grid-template-columns: 1fr !important; gap: 24px !important; } | |
| .nav-buttons { display: none; } | |
| .problem-grid { grid-template-columns: 1fr !important; gap: 40px !important; } | |
| .features-grid { grid-template-columns: repeat(2, 1fr) !important; } | |
| .steps-grid { grid-template-columns: 1fr !important; } | |
| .resurface-grid { grid-template-columns: 1fr !important; } | |
| .steps-line { display: none !important; } | |
| } | |
| `@media` (max-width: 480px) { | |
| .features-grid { grid-template-columns: 1fr !important; } | |
| } | |
| `@media` (prefers-reduced-motion: reduce) { | |
| .landing-page *, | |
| .landing-page *::before, | |
| .landing-page *::after { | |
| animation: none !important; | |
| transition: none !important; | |
| scroll-behavior: auto !important; | |
| } | |
| } | |
| `}</style> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/page.tsx` around lines 452 - 492, The injected stylesheet
applies continuous animations (blobDrift, float) and repeated entrance
transitions but lacks a prefers-reduced-motion escape hatch; update the style
block in page.tsx to add a `@media` (prefers-reduced-motion: reduce) rule that
targets the animated selectors (e.g., elements using blobDrift, float, and
classes like .feature-card, .nav-cta, or any selector that uses
animation/transition) and force-disable motion by setting animation: none
!important; transition: none !important; transform: none !important; and reduce
any keyframe-driven motion, ensuring users with prefers-reduced-motion get a
static experience.
Comment on lines
+124
to
+130
| <button | ||
| type="button" | ||
| onClick={deleteBookmark} | ||
| disabled={deleting} | ||
| aria-label="Delete bookmark" | ||
| className="flex-shrink-0 opacity-0 group-hover:opacity-100 w-6 h-6 flex items-center justify-center rounded text-[var(--text-dim)] hover:text-red-400 hover:bg-[var(--error-bg)] transition-all duration-100 disabled:opacity-30" | ||
| > |
There was a problem hiding this comment.
Don’t hide the only delete affordance behind desktop hover.
opacity-0 group-hover:opacity-100 makes this action effectively invisible on touch devices and during keyboard navigation, so deleting a bookmark becomes hard to discover outside mouse hover. Make it visible on focus and keep it visible by default for non-hover interactions.
Suggested fix
- className="flex-shrink-0 opacity-0 group-hover:opacity-100 w-6 h-6 flex items-center justify-center rounded text-[var(--text-dim)] hover:text-red-400 hover:bg-[var(--error-bg)] transition-all duration-100 disabled:opacity-30"
+ className="flex-shrink-0 w-6 h-6 flex items-center justify-center rounded text-[var(--text-dim)] opacity-100 md:opacity-0 md:group-hover:opacity-100 md:group-focus-within:opacity-100 focus-visible:opacity-100 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[var(--accent)] hover:text-red-400 hover:bg-[var(--error-bg)] transition-all duration-100 disabled:opacity-30"📝 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.
Suggested change
| <button | |
| type="button" | |
| onClick={deleteBookmark} | |
| disabled={deleting} | |
| aria-label="Delete bookmark" | |
| className="flex-shrink-0 opacity-0 group-hover:opacity-100 w-6 h-6 flex items-center justify-center rounded text-[var(--text-dim)] hover:text-red-400 hover:bg-[var(--error-bg)] transition-all duration-100 disabled:opacity-30" | |
| > | |
| <button | |
| type="button" | |
| onClick={deleteBookmark} | |
| disabled={deleting} | |
| aria-label="Delete bookmark" | |
| className="flex-shrink-0 w-6 h-6 flex items-center justify-center rounded text-[var(--text-dim)] opacity-100 md:opacity-0 md:group-hover:opacity-100 md:group-focus-within:opacity-100 focus-visible:opacity-100 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[var(--accent)] hover:text-red-400 hover:bg-[var(--error-bg)] transition-all duration-100 disabled:opacity-30" | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/components/BookmarkCard.tsx` around lines 124 - 130, The delete
button is hidden with "opacity-0 group-hover:opacity-100", making it
inaccessible on touch and keyboard; update the class on the button (the element
using onClick={deleteBookmark}) to make it visible for focus and non-hover
interactions—replace "opacity-0 group-hover:opacity-100" with something like
"opacity-100 group-hover:opacity-100 focus:opacity-100 group-focus:opacity-100
focus-visible:opacity-100" (or remove the initial opacity-0 entirely) so the
delete affordance is discoverable for keyboard and touch users while preserving
the hover behavior.
Comment on lines
+87
to
+90
| <span className="text-sm font-medium text-[var(--text)] truncate"> | ||
| {b.title || hostname(b.url)} | ||
| </span> | ||
| <span className="text-xs text-zinc-400 truncate">{b.url}</span> | ||
| <span className="text-xs text-[var(--text-dim)] truncate">{hostname(b.url)}</span> |
There was a problem hiding this comment.
Keep more URL detail in the search results.
The new subtitle collapses everything to the bare hostname, which makes same-site bookmarks hard to tell apart when titles are missing or similar. Showing a truncated pathname or normalized full URL in the secondary row would keep the cleaner UI without losing disambiguation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/components/SearchBar.tsx` around lines 87 - 90, The subtitle
currently always renders hostname(b.url), which loses disambiguating path info;
update the subtitle rendering in SearchBar.tsx (the span that uses
hostname(b.url)) to instead display a normalized, truncated URL path or full URL
when the path is non-root: derive let u = new URL(b.url) and show u.pathname +
u.search (and u.hash optionally) truncated, falling back to hostname(b.url) when
pathname === '/' or empty; keep the existing CSS classes and truncation behavior
so the UI stays clean while preserving more URL detail for bookmarks with
identical hostnames.
Comment on lines
+6
to
+25
| const [dark, setDark] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| setDark(document.documentElement.classList.contains("dark")); | ||
| }, []); | ||
|
|
||
| function toggle() { | ||
| const next = !dark; | ||
| setDark(next); | ||
| document.documentElement.classList.toggle("dark", next); | ||
| try { | ||
| localStorage.setItem("recall-theme", next ? "dark" : "light"); | ||
| } catch {} | ||
| } | ||
|
|
||
| return ( | ||
| <button | ||
| onClick={toggle} | ||
| aria-label="Toggle theme" | ||
| className="w-8 h-8 flex items-center justify-center rounded-md text-[var(--text-muted)] hover:text-[var(--text)] hover:bg-[var(--surface-2)] transition-all duration-100 active:scale-95" |
There was a problem hiding this comment.
Initialize and expose the actual theme state, not a post-mount guess.
dark always starts as false during SSR, so dark-mode loads briefly render the moon icon until Line 9 corrects the state after mount. The button also never exposes its current toggle state to assistive tech. Rendering after mount (or hydrating the initial value from the same preload source as client/src/app/layout.tsx:39-49) plus aria-pressed={dark} would avoid the flicker and make the control state explicit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/components/ThemeToggle.tsx` around lines 6 - 25, Initialize the
ThemeToggle's dark state from the same preloaded/hydrated theme source used by
the app layout instead of defaulting to false on SSR: change the useState
initialization for dark to derive its initial value synchronously (e.g., via a
lazy initializer that reads the client/hydrated theme value or localStorage with
a window guard) so the correct theme is rendered on first paint, keep the
existing useEffect only for syncing if needed, and ensure the toggle function
(toggle) still updates document.documentElement.classList and localStorage; also
expose the control state to assistive tech by adding aria-pressed={dark} to the
button so screen readers see the actual pressed/state value.
Comment on lines
15
to
29
| setAll(cookiesToSet) { | ||
| cookiesToSet.forEach(({ name, value, options }) => | ||
| res.cookies.set(name, value, options), | ||
| res.cookies.set(name, value, options) | ||
| ); | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| const { | ||
| data: { user }, | ||
| } = await supabase.auth.getUser(); | ||
| // Calling getUser() triggers token refresh and writes the new cookie to `res` | ||
| const { data: { user } } = await supabase.auth.getUser(); | ||
|
|
||
| if (!user) { | ||
| // Only protect /app routes — landing page and auth page are public | ||
| if (!user && req.nextUrl.pathname.startsWith("/app")) { | ||
| return NextResponse.redirect(new URL("/auth", req.url)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "proxy.ts" -type f | head -20Repository: MinitJain/recall
Length of output: 81
🏁 Script executed:
cat -n ./client/src/proxy.tsRepository: MinitJain/recall
Length of output: 1363
🏁 Script executed:
# Let me check the Supabase SSR documentation pattern and Next.js redirect behavior
# to verify the concern about cookie propagation
cat -n ./client/src/proxy.ts | sed -n '15,32p'Repository: MinitJain/recall
Length of output: 722
🌐 Web query:
Supabase SSR package cookie propagation NextResponse middleware pattern
💡 Result:
The standard pattern for cookie propagation with the @supabase/ssr package in Next.js middleware uses NextResponse to create a proxy that refreshes Supabase auth sessions and propagates updated cookies (including refreshed auth tokens) to both the request and response objects. ## Key Components 1. Install packages: npm install @supabase/supabase-js @supabase/ssr 2. Environment variables: NEXT_PUBLIC_SUPABASE_URL=your_project_url NEXT_PUBLIC_SUPABASE_ANON_KEY=your_anon_key 3. Middleware utility (e.g., lib/supabase/middleware.ts): typescript import { createServerClient, type CookieOptions } from '@supabase/ssr' import { NextResponse, type NextRequest } from 'next/server' export async function updateSession(request: NextRequest) { let response = NextResponse.next({ request: { headers: request.headers }, }) const supabase = createServerClient( process.env.NEXT_PUBLIC_SUPABASE_URL!, process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!, { cookies: { getAll { return request.cookies.getAll }, setAll(cookiesToSet) { cookiesToSet.forEach(({ name, value, options }) => { request.cookies.set({ name, value, ...options }) }) cookiesToSet.forEach(({ name, value, options }) => { response.cookies.set({ name, value, ...options }) }) }, }, }, ) // Refresh session if needed - critical for token refresh await supabase.auth.getUser // Optional: Add route protection logic here const { data: { user } } = await supabase.auth.getUser if (!user && request.nextUrl.pathname.startsWith('/dashboard')) { return NextResponse.redirect(new URL('/login', request.url)) } return response } 4. Middleware entry point (middleware.ts): typescript import { type NextRequest } from 'next/server' import { updateSession } from '@/lib/supabase/middleware' export async function middleware(request: NextRequest) { return await updateSession(request) } export const config = { matcher: [ /* * Match all request paths except for the ones starting with: * - _next/static (static files) * - _next/image (image optimization files) * - favicon.ico (favicon file) / '/((?!_next/static|_next/image|favicon.ico|.\.(?:svg|png|jpg|jpeg|gif|webp)$).*)', ], } ## How Cookie Propagation Works 1. getAll: Reads incoming cookies from request.cookies.getAll 2. setAll: - Updates request.cookies (propagates to Server Components) - Updates response.cookies (propagates to browser) 3. Token refresh: Calling supabase.auth.getUser triggers automatic refresh if token is expired, updating cookies via setAll 4. Return response: Must return the modified response object containing updated cookies ## Server Components Client typescript // lib/supabase/server.ts import { createServerClient } from '@supabase/ssr' import { cookies } from 'next/headers' export function createClient { const cookieStore = cookies return createServerClient( process.env.NEXT_PUBLIC_SUPABASE_URL!, process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!, { cookies: { getAll { return cookieStore.getAll }, setAll(cookiesToSet) { cookiesToSet.forEach(({ name, value, options }) => cookieStore.set({ name, value, ...options }) ) }, }, } ) } ## Browser Client typescript // lib/supabase/browser.ts import { createBrowserClient } from '@supabase/ssr' export function createBrowserClient { return createBrowserClient( process.env.NEXT_PUBLIC_SUPABASE_URL!, process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY! ) } ## Important Notes - Always call getUser or getClaims in middleware to trigger session refresh - Return the response object - contains updated Set-Cookie headers - Use getAll/setAll - individual get/set methods are deprecated - Matcher excludes static assets to avoid unnecessary processing - Production caching: Add Cache-Control: private, no-store to prevent session leakage This pattern ensures seamless cookie propagation across SSR boundaries, automatic token refresh, and secure route protection.
Citations:
- 1: https://supabase.com/docs/guides/auth/server-side/nextjs
- 2: https://the-shubham.medium.com/next-js-supabase-cookie-based-auth-workflow-the-best-auth-solution-2025-guide-f6738b4673c1
- 3: https://coldfusion-example.blogspot.com/2026/02/fixing-authsessionmissingerror-in.html
- 4: https://supalaunch.com/blog/nextjs-middleware-supabase-auth
- 5: https://github.com/supabase/supabase/blob/master/examples/prompts/nextjs-supabase-auth.md
- 6: https://github.com/supabase/ssr
- 7: https://github.com/supabase/supabase/blob/master/examples/auth/nextjs/middleware.ts
Preserve Supabase's mutated response instead of replacing it on redirect.
setAll() only updates res.cookies, and line 29 returns a brand-new redirect response. The Supabase SSR pattern requires updating both request.cookies and response.cookies in setAll(), and any subsequent response (including redirects) must be the mutated res object. When a fresh redirect is created instead, all cookies set by Supabase's token refresh are dropped, causing immediate session termination.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/proxy.ts` around lines 15 - 29, The redirect is creating a fresh
NextResponse and losing cookies that Supabase set via the custom setAll
implementation; update setAll to write to both res.cookies and req.cookies (or
request.cookies) and when blocking unauthenticated access return the mutated res
object instead of calling NextResponse.redirect(...) directly (i.e. use the
existing res and set its status/location or use NextResponse.redirect but start
from the mutated res), ensuring supabase.auth.getUser()'s cookie mutations on
res are preserved; locate the setAll function, the res.cookies usage, the
supabase.auth.getUser() call, and the redirect branch to make this change.
Comment on lines
+27
to
+36
| // Only protect /app routes — landing page and auth page are public | ||
| if (!user && req.nextUrl.pathname.startsWith("/app")) { | ||
| return NextResponse.redirect(new URL("/auth", req.url)); | ||
| } | ||
|
|
||
| return res; | ||
| } | ||
|
|
||
| export const config = { | ||
| matcher: ["/"], | ||
| matcher: ["/((?!_next/static|_next/image|favicon.ico).*)"], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the proxy.ts file to see the full context
find . -path "*/client/src/proxy.ts" -o -path "*/src/proxy.ts" | head -5Repository: MinitJain/recall
Length of output: 81
🏁 Script executed:
# Let's also look for the file structure
fd proxy.tsRepository: MinitJain/recall
Length of output: 79
🏁 Script executed:
# Let's examine the full proxy.ts file
cat -n client/src/proxy.tsRepository: MinitJain/recall
Length of output: 1363
🏁 Script executed:
# Let's check what routes and public assets exist in the application
find . -type f -path "*/app/*" | grep -E "(apple|robots|sitemap)" | head -20Repository: MinitJain/recall
Length of output: 114
🏁 Script executed:
# Check for app directory structure
fd -t f "^page\." app/ | head -20Repository: MinitJain/recall
Length of output: 151
🏁 Script executed:
# Look for public assets and favicon configurations
fd "favicon|apple-icon|robots|sitemap" . --type f | head -20Repository: MinitJain/recall
Length of output: 171
🏁 Script executed:
# Let's check the app directory structure
find client/src/app -type f -name "*.ts*" -o -name "*.jsx" | head -30Repository: MinitJain/recall
Length of output: 534
🏁 Script executed:
# Check if there are any routes like apple-icon, icon, etc.
find client/src/app -type f \( -name "*.ico" -o -name "*icon*" \) | head -20Repository: MinitJain/recall
Length of output: 86
🏁 Script executed:
# Let's examine robots.ts and sitemap.ts to see how they're structured
cat -n client/src/app/robots.tsRepository: MinitJain/recall
Length of output: 255
🏁 Script executed:
cat -n client/src/app/sitemap.tsRepository: MinitJain/recall
Length of output: 533
🏁 Script executed:
# Let's check if there are any icon routes or other metadata files
find client/src/app -type f -name "icon.*" -o -name "opengraph-image.*" -o -name "twitter-image.*"Repository: MinitJain/recall
Length of output: 42
🏁 Script executed:
# Let's also check the main layout.tsx to understand the structure better
cat -n client/src/app/layout.tsx | head -50Repository: MinitJain/recall
Length of output: 1938
🏁 Script executed:
# Let's verify if robots.ts and sitemap.ts routes would actually match the current matcher
# by checking what paths they respond on
rg -A 5 "robots.ts|sitemap.ts" client/src/app/ --type tsRepository: MinitJain/recall
Length of output: 42
🏁 Script executed:
# Let's also check the Next.js configuration to see if there are any other middleware settings
cat -n client/next.config.* 2>/dev/null || echo "No next.config file found"Repository: MinitJain/recall
Length of output: 241
🏁 Script executed:
# Check if there's a middleware.ts file
find client -name "middleware.ts" -o -name "middleware.js"Repository: MinitJain/recall
Length of output: 42
🏁 Script executed:
# Let's check if there are any public routes or assets that might have the /app prefix
# First, let's see all the routes in the app
find client/src/app -type d | grep -v node_modules | head -20Repository: MinitJain/recall
Length of output: 317
🏁 Script executed:
# Check the page.tsx to understand the landing page structure
cat -n client/src/app/page.tsxRepository: MinitJain/recall
Length of output: 32235
Use segment-aware path check and expand matcher to exclude metadata routes.
pathname.startsWith("/app") incorrectly matches unintended routes like /application or /apple-icon if they're ever created. The matcher also runs middleware on /robots.txt and /sitemap.xml unnecessarily, adding auth work to metadata requests that should be public. Use path === "/app" || path.startsWith("/app/") for the check and broaden the matcher to exclude these metadata and common static file extensions.
Suggested direction
- if (!user && req.nextUrl.pathname.startsWith("/app")) {
+ const path = req.nextUrl.pathname;
+ if (!user && (path === "/app" || path.startsWith("/app/"))) {
return NextResponse.redirect(new URL("/auth", req.url));
}
export const config = {
- matcher: ["/((?!_next/static|_next/image|favicon.ico).*)"],
+ matcher: [
+ "/((?!_next/static|_next/image|favicon.ico|robots.txt|sitemap.xml).*)",
+ ],
};📝 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.
Suggested change
| // Only protect /app routes — landing page and auth page are public | |
| if (!user && req.nextUrl.pathname.startsWith("/app")) { | |
| return NextResponse.redirect(new URL("/auth", req.url)); | |
| } | |
| return res; | |
| } | |
| export const config = { | |
| matcher: ["/"], | |
| matcher: ["/((?!_next/static|_next/image|favicon.ico).*)"], | |
| // Only protect /app routes — landing page and auth page are public | |
| const path = req.nextUrl.pathname; | |
| if (!user && (path === "/app" || path.startsWith("/app/"))) { | |
| return NextResponse.redirect(new URL("/auth", req.url)); | |
| } | |
| return res; | |
| } | |
| export const config = { | |
| matcher: [ | |
| "/((?!_next/static|_next/image|favicon.ico|robots.txt|sitemap.xml).*)", | |
| ], | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/proxy.ts` around lines 27 - 36, The middleware path check using
req.nextUrl.pathname.startsWith("/app") should be made segment-aware: replace it
with a check like path === "/app" || path.startsWith("/app/") (i.e., update the
condition where user is checked and NextResponse.redirect is returned) to avoid
matching routes such as /application; also update export const config.matcher to
broaden exclusions so middleware does not run for metadata/static files (exclude
exact files like /robots.txt and /sitemap.xml and common static extensions such
as .txt, .xml, .png, .jpg, .jpeg, .svg, .ico, .webmanifest, etc.) so auth work
is skipped for those requests.
- Replace useEffect setState pattern with lazy useState initializer to fix react-hooks/set-state-in-effect ESLint error that was failing CI - Add Google and GitHub OAuth buttons to auth page - Add /auth/callback route to exchange OAuth code for session Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
client/src/app/auth/page.tsx (2)
195-205: Consider disabling the email/password form during OAuth redirect.The submit button is only disabled when
loadingis true, but not whenoauthLoadingis set. A user could potentially click "Log in" while an OAuth redirect is in progress. While the browser will navigate away shortly, disabling the form during OAuth would provide a more consistent UX.♻️ Proposed fix
<button type="submit" - disabled={loading} + disabled={loading || oauthLoading !== null} className="rounded-lg bg-[var(--accent)] px-4 py-2 text-sm font-semibold text-[var(--accent-text)] hover:bg-[var(--accent-hover)] disabled:opacity-50 transition-colors duration-100 active:scale-95 mt-1" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/auth/page.tsx` around lines 195 - 205, The submit button and form should also be disabled during OAuth redirects; update the disabled check on the submit button (and any related form controls) to consider oauthLoading in addition to loading (e.g., change usages of disabled={loading} to disabled={loading || oauthLoading}) and ensure any conditional label/UX that shows "Please wait…" or similar reflects oauthLoading as well so the form is inert while OAuth redirect is in progress; adjust references in the component where loading is used (button rendering block and any parent form handlers) to include oauthLoading.
81-83: Usenext/imagefor optimized image loading.The static analysis correctly identifies that using a raw
<img>element bypasses Next.js image optimization, potentially resulting in slower LCP and higher bandwidth usage.♻️ Proposed fix using next/image
+import Image from "next/image";- <div className="inline-flex items-center justify-center w-12 h-12 mb-4"> - <img src="/logo.svg" alt="Recall" width={48} height={48} /> - </div> + <div className="inline-flex items-center justify-center w-12 h-12 mb-4"> + <Image src="/logo.svg" alt="Recall" width={48} height={48} priority /> + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/auth/page.tsx` around lines 81 - 83, Replace the raw <img> element in the auth page with Next.js' Image component: add an import for Image from 'next/image' at the top of client/src/app/auth/page.tsx, then swap the <img src="/logo.svg" alt="Recall" width={48} height={48} /> usage inside the div for <Image src="/logo.svg" alt="Recall" width={48} height={48} /> (preserve any className on the wrapper div or pass it to Image if needed) so Next's image optimization is used for the logo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/app/auth/callback/route.ts`:
- Line 8: The user-controlled next query param (variable next from
searchParams.get("next")) can cause an open-redirect; validate it before using
it as a redirect target by ensuring it is a relative path that starts with a
single "/" and does not contain "://" or "//" (and reject or normalize values
like "//evil.com" or "https://..."), otherwise fall back to the safe default
"/app". Update the redirect logic in route.ts where next is read and used (and
apply the same validation at the other occurrence) to perform this check and
only use the sanitized/validated path for the redirect.
---
Nitpick comments:
In `@client/src/app/auth/page.tsx`:
- Around line 195-205: The submit button and form should also be disabled during
OAuth redirects; update the disabled check on the submit button (and any related
form controls) to consider oauthLoading in addition to loading (e.g., change
usages of disabled={loading} to disabled={loading || oauthLoading}) and ensure
any conditional label/UX that shows "Please wait…" or similar reflects
oauthLoading as well so the form is inert while OAuth redirect is in progress;
adjust references in the component where loading is used (button rendering block
and any parent form handlers) to include oauthLoading.
- Around line 81-83: Replace the raw <img> element in the auth page with
Next.js' Image component: add an import for Image from 'next/image' at the top
of client/src/app/auth/page.tsx, then swap the <img src="/logo.svg" alt="Recall"
width={48} height={48} /> usage inside the div for <Image src="/logo.svg"
alt="Recall" width={48} height={48} /> (preserve any className on the wrapper
div or pass it to Image if needed) so Next's image optimization is used for the
logo.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: df2f3c9e-cdf2-4317-bfbe-c25d7e08f36f
📒 Files selected for processing (3)
client/src/app/auth/callback/route.tsclient/src/app/auth/page.tsxclient/src/components/ThemeToggle.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- client/src/components/ThemeToggle.tsx
When the user navigates back from a provider's OAuth page, the bfcache restores the page with oauthLoading still set, disabling both buttons. Listen for pageshow to reset it on every page restore. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Validate the next query param before using it as a redirect target. Only accept relative paths starting with "/" that don't contain "://" or "//", falling back to "/app" for anything else. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
client/src/app/auth/page.tsx (1)
86-89: Use Next.jsImagecomponent for automatic optimization.The static
<img>element may result in slower LCP and higher bandwidth. Using the Next.jsImagecomponent provides automatic optimization including lazy loading and format conversion.♻️ Proposed fix to use Next.js Image
+"use client"; + +import Image from "next/image"; import { useState, useEffect } from "react";Then update the image element:
<div className="inline-flex items-center justify-center w-12 h-12 mb-4"> - <img src="/logo.svg" alt="Recall" width={48} height={48} /> + <Image src="/logo.svg" alt="Recall" width={48} height={48} priority /> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/auth/page.tsx` around lines 86 - 89, Replace the raw <img> with Next.js' Image component: import Image from "next/image" at the top of the module and swap the <img src="/logo.svg" alt="Recall" width={48} height={48} /> inside the auth page JSX (in page.tsx) for <Image src="/logo.svg" alt="Recall" width={48} height={48} /> (add priority if you want to ensure LCP loading). Ensure the Image import is present and remove the old <img> tag.client/src/app/auth/callback/route.ts (1)
17-32: Consider reusing the shared Supabase server client factory.The cookie handling logic here duplicates what's already in
client/src/lib/supabase/server.ts. You could import and usecreateClientfrom there instead of creating the client inline.♻️ Proposed refactor to reuse shared factory
import { createServerClient } from "@supabase/ssr"; -import { cookies } from "next/headers"; +import { createClient } from "@/lib/supabase/server"; import { NextResponse } from "next/server"; export async function GET(request: Request) { const { searchParams, origin } = new URL(request.url); const code = searchParams.get("code"); const rawNext = searchParams.get("next") ?? "/app"; // Reject anything that isn't a relative path to prevent open redirects const next = rawNext.startsWith("/") && !rawNext.startsWith("//") && !rawNext.includes("://") ? rawNext : "/app"; if (code) { - const cookieStore = await cookies(); - const supabase = createServerClient( - process.env.NEXT_PUBLIC_SUPABASE_URL!, - process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!, - { - cookies: { - getAll() { - return cookieStore.getAll(); - }, - setAll(cookiesToSet) { - cookiesToSet.forEach(({ name, value, options }) => - cookieStore.set(name, value, options) - ); - }, - }, - } - ); + const supabase = await createClient(); const { error } = await supabase.auth.exchangeCodeForSession(code);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/auth/callback/route.ts` around lines 17 - 32, The inline creation of a Supabase server client duplicates cookie handling logic; replace the createServerClient call with the shared factory by importing and invoking the exported createClient (or similarly named) from client/src/lib/supabase/server.ts, remove the local cookies block and any direct references to cookieStore here, and ensure the shared factory is passed the request/response or cookieStore context it expects so authentication still works (update the import statement and call site to use createClient instead of createServerClient and delete the duplicated cookie getAll/setAll logic).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@client/src/app/auth/callback/route.ts`:
- Around line 17-32: The inline creation of a Supabase server client duplicates
cookie handling logic; replace the createServerClient call with the shared
factory by importing and invoking the exported createClient (or similarly named)
from client/src/lib/supabase/server.ts, remove the local cookies block and any
direct references to cookieStore here, and ensure the shared factory is passed
the request/response or cookieStore context it expects so authentication still
works (update the import statement and call site to use createClient instead of
createServerClient and delete the duplicated cookie getAll/setAll logic).
In `@client/src/app/auth/page.tsx`:
- Around line 86-89: Replace the raw <img> with Next.js' Image component: import
Image from "next/image" at the top of the module and swap the <img
src="/logo.svg" alt="Recall" width={48} height={48} /> inside the auth page JSX
(in page.tsx) for <Image src="/logo.svg" alt="Recall" width={48} height={48} />
(add priority if you want to ensure LCP loading). Ensure the Image import is
present and remove the old <img> tag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dcb2fc44-4feb-49bc-aadd-bee4a990704d
📒 Files selected for processing (2)
client/src/app/auth/callback/route.tsclient/src/app/auth/page.tsx
- auth/callback/route.ts: replace inline createServerClient + cookie block with shared createClient from lib/supabase/server - auth/page.tsx: replace <img> with next/image <Image priority> for LCP Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
@CodeRabbit review |
- ThemeToggle: replace lazy useState with useSyncExternalStore to fix server/client hydration mismatch in dark mode. Uses MutationObserver on html[class] to stay in sync. No ESLint issues. - auth/page.tsx: make logo + wordmark a link back to / with amber hover Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
client/src/app/auth/page.tsx (1)
152-177: Add explicit autofill hints to the credential fields.The new inputs omit
autoComplete, which makes browser/password-manager behavior less reliable than it needs to be.current-password/new-passwordwill keep both login and signup flows working cleanly.✍️ Suggested tweak
<input id="email-input" type="email" + autoComplete="email" placeholder="you@example.com" value={email} onChange={(e) => setEmail(e.target.value)} required className="rounded-lg border border-[var(--border)] bg-[var(--surface-2)] px-3 py-2 text-sm text-[var(--text)] placeholder:text-[var(--text-dim)] focus:outline-none focus:border-[var(--accent)] transition-colors duration-100" /> ... <input id="password-input" type={showPassword ? "text" : "password"} + autoComplete={mode === "login" ? "current-password" : "new-password"} placeholder="••••••••" value={password} onChange={(e) => setPassword(e.target.value)} required className="w-full rounded-lg border border-[var(--border)] bg-[var(--surface-2)] px-3 py-2 pr-9 text-sm text-[var(--text)] placeholder:text-[var(--text-dim)] focus:outline-none focus:border-[var(--accent)] transition-colors duration-100" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/auth/page.tsx` around lines 152 - 177, Add explicit autoComplete attributes to the credential inputs to improve browser/password-manager behavior: add autoComplete="email" to the email input (id="email-input") and add autoComplete for the password input (id="password-input") such that it is "current-password" for sign-in flows and "new-password" for sign-up flows (e.g. autoComplete={isSignUp ? "new-password" : "current-password"}); if there is no signup flag available, default the password input to autoComplete="current-password". Use the existing state setters (setEmail, setPassword) and showPassword logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/app/auth/page.tsx`:
- Line 17: Replace the two separate flags (loading and oauthLoading) with a
single shared busy state (e.g., authBusy: null | "email" | "google" | "github")
and wire it into all auth entry points and UI controls; set authBusy to the
appropriate token at the start of handleSubmit (email/password), at the start of
the OAuth handlers (Google/GitHub functions), and clear it in both success and
all error/catch/finally paths so no other auth action can be started while one
is in progress; also use authBusy to disable mode switches, provider buttons,
and the email/password form so the entire auth surface is gated consistently.
- Around line 19-23: The page never reads the redirect query string so OAuth
callback failures are hidden; inside the existing useEffect (the one that
sets/reset pageshow listener) parse window.location.search (or use
URLSearchParams) and if error=oauth is present call the component state updater
(e.g., setOauthLoading or a new setOauthError) to mark the OAuth flow as failed
so the UI shows an error message; keep the pageshow reset behavior, but add this
one-time check in the same useEffect and ensure the component renders an error
state when that flag is set.
---
Nitpick comments:
In `@client/src/app/auth/page.tsx`:
- Around line 152-177: Add explicit autoComplete attributes to the credential
inputs to improve browser/password-manager behavior: add autoComplete="email" to
the email input (id="email-input") and add autoComplete for the password input
(id="password-input") such that it is "current-password" for sign-in flows and
"new-password" for sign-up flows (e.g. autoComplete={isSignUp ? "new-password" :
"current-password"}); if there is no signup flag available, default the password
input to autoComplete="current-password". Use the existing state setters
(setEmail, setPassword) and showPassword logic unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c90db76-161c-480f-ba4b-15205c5d85af
📒 Files selected for processing (2)
client/src/app/auth/page.tsxclient/src/components/ThemeToggle.tsx
| const [password, setPassword] = useState(""); | ||
| const [error, setError] = useState<string | null>(null); | ||
| const [loading, setLoading] = useState(false); | ||
| const [oauthLoading, setOauthLoading] = useState<"google" | "github" | null>(null); |
There was a problem hiding this comment.
Use one busy flag for the whole auth surface.
loading only gates the email/password submit, while oauthLoading only gates the provider buttons. Users can still switch modes or trigger a second auth path before the first one resolves, which makes the auth UI state racy.
🔒 Example of a shared busy state
const [loading, setLoading] = useState(false);
const [oauthLoading, setOauthLoading] = useState<"google" | "github" | null>(null);
+ const busy = loading || oauthLoading !== null;
...
- disabled={oauthLoading !== null}
+ disabled={busy}
...
+ disabled={busy}
className="rounded-lg border border-[var(--border)] bg-[var(--surface-2)] px-3 py-2 text-sm text-[var(--text)] placeholder:text-[var(--text-dim)] focus:outline-none focus:border-[var(--accent)] transition-colors duration-100"
/>
...
+ disabled={busy}
className="w-full rounded-lg border border-[var(--border)] bg-[var(--surface-2)] px-3 py-2 pr-9 text-sm text-[var(--text)] placeholder:text-[var(--text-dim)] focus:outline-none focus:border-[var(--accent)] transition-colors duration-100"
/>
...
- disabled={loading}
+ disabled={busy}
...
<button
+ type="button"
+ disabled={busy}
onClick={() => {
setMode(mode === "login" ? "signup" : "login");
setError(null);
setPassword("");
}}
- className="text-[var(--accent)] hover:underline font-medium"
+ className="text-[var(--accent)] hover:underline font-medium disabled:opacity-50 disabled:no-underline"
>Also applies to: 107-127, 144-177, 205-208, 222-228
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/auth/page.tsx` at line 17, Replace the two separate flags
(loading and oauthLoading) with a single shared busy state (e.g., authBusy: null
| "email" | "google" | "github") and wire it into all auth entry points and UI
controls; set authBusy to the appropriate token at the start of handleSubmit
(email/password), at the start of the OAuth handlers (Google/GitHub functions),
and clear it in both success and all error/catch/finally paths so no other auth
action can be started while one is in progress; also use authBusy to disable
mode switches, provider buttons, and the email/password form so the entire auth
surface is gated consistently.
Comment on lines
+19
to
+23
| useEffect(() => { | ||
| const reset = () => setOauthLoading(null); | ||
| window.addEventListener("pageshow", reset); | ||
| return () => window.removeEventListener("pageshow", reset); | ||
| }, []); |
There was a problem hiding this comment.
Show OAuth callback failures after the redirect.
The new callback flow sends failures back to /auth?error=oauth, but this page never reads that query param. Any server-side code-exchange failure or provider denial currently lands on the form with no feedback.
💡 Minimal fix
useEffect(() => {
const reset = () => setOauthLoading(null);
window.addEventListener("pageshow", reset);
+ const params = new URLSearchParams(window.location.search);
+ if (params.get("error") === "oauth") {
+ setError("OAuth sign-in failed. Please try again.");
+ }
return () => window.removeEventListener("pageshow", reset);
}, []);Also applies to: 27-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/auth/page.tsx` around lines 19 - 23, The page never reads the
redirect query string so OAuth callback failures are hidden; inside the existing
useEffect (the one that sets/reset pageshow listener) parse
window.location.search (or use URLSearchParams) and if error=oauth is present
call the component state updater (e.g., setOauthLoading or a new setOauthError)
to mark the OAuth flow as failed so the UI shows an error message; keep the
pageshow reset behavior, but add this one-time check in the same useEffect and
ensure the component renders an error state when that flag is set.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Type of change
Checklist
npm run build)npx tsc --noEmit)npm run lint)Summary by CodeRabbit
New Features
UI/UX Improvements