feat: redesign NotFoundPage with modern brutalist responsive layout#86
feat: redesign NotFoundPage with modern brutalist responsive layout#86banthevaishnavi wants to merge 1 commit into
Conversation
🚀 PR Received SuccessfullyHello @banthevaishnavi, Thank you for taking the initiative to contribute to this project. Please ensure that your PR follows all project guidelines properly before requesting review.
|
📝 WalkthroughWalkthroughThe NotFoundPage component is completely redesigned from a simple layout into a full-page, modern 404 experience. The new design introduces a background grid, dual-panel organization with left and right sections, multiple detail cards, and expanded navigation options including new routes to Changes404 Page Redesign
Sequence Diagram(s)Not applicable. This change is a self-contained UI component redesign with Tailwind styling and static JSX markup, with no multi-component interactions or complex control flow to visualize. Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/pages/NotFoundPage.jsx (1)
191-196:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winLogin route does not exist.
The "Login" link points to
/login, but this route is not defined in the routing configuration. Based on the router context, authentication appears to use GitHub OAuth with a callback at/auth/github/callback, but there's no dedicated/loginroute.Action required: Either add a
/loginroute or update this link to point to the correct authentication entry point for your application.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/NotFoundPage.jsx` around lines 191 - 196, The NotFoundPage.jsx Link currently points to a non-existent "/login" route; update the Link in NotFoundPage.jsx (the Link element that renders "Login") to point to your actual auth entrypoint (e.g., the OAuth initiation path such as "/auth/github" or whatever route starts the GitHub OAuth flow) OR add a new "/login" route that triggers the existing GitHub OAuth flow (so it redirects to the same initiation endpoint or invokes the OAuth handler). Ensure the Link's "to" value matches the real authentication entrypoint used by your router (not "/login").
🧹 Nitpick comments (1)
frontend/src/pages/NotFoundPage.jsx (1)
8-14: ⚡ Quick winGrid column count mismatch with rendered divs.
The grid uses 6 columns on smaller screens but always renders 12 divs. The extra 6 divs add unnecessary DOM nodes on mobile and tablet viewports.
♻️ Proposed fix to match rendered divs to visible columns
- <div className="absolute inset-0 grid grid-cols-6 lg:grid-cols-12 pointer-events-none"> - {Array.from({ length: 12 }).map((_, i) => ( + <div className="absolute inset-0 grid grid-cols-12 pointer-events-none"> + {Array.from({ length: 12 }).map((_, i) => ( <div key={i} - className="border-r border-black/10 h-full" + className="border-r border-black/10 h-full hidden lg:block [&:nth-child(-n+6)]:block" /> ))} </div>Alternatively, keep all 12 columns visible on all screen sizes if the design permits.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/NotFoundPage.jsx` around lines 8 - 14, The grid renders 12 divider divs regardless of viewport while the CSS grid uses 6 columns by default and 12 at lg; update the Array.from({ length: 12 }) in NotFoundPage.jsx so the rendered count matches the visible columns (e.g., compute columnsCount = isLargeScreen ? 12 : 6 using a small responsive check like window.matchMedia or a useMedia hook inside the NotFoundPage component, then use Array.from({ length: columnsCount }) for the mapped divs), ensuring the key/layout code that currently maps the divs remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/pages/NotFoundPage.jsx`:
- Around line 24-26: The decorative background paragraph in NotFoundPage.jsx
(the <p> that renders the large "404" with classes including "absolute inset-0
... pointer-events-none select-none") is announced by screen readers; add
aria-hidden="true" to that <p> so it is ignored by assistive technology and only
the main accessible "404" heading is read.
---
Duplicate comments:
In `@frontend/src/pages/NotFoundPage.jsx`:
- Around line 191-196: The NotFoundPage.jsx Link currently points to a
non-existent "/login" route; update the Link in NotFoundPage.jsx (the Link
element that renders "Login") to point to your actual auth entrypoint (e.g., the
OAuth initiation path such as "/auth/github" or whatever route starts the GitHub
OAuth flow) OR add a new "/login" route that triggers the existing GitHub OAuth
flow (so it redirects to the same initiation endpoint or invokes the OAuth
handler). Ensure the Link's "to" value matches the real authentication
entrypoint used by your router (not "/login").
---
Nitpick comments:
In `@frontend/src/pages/NotFoundPage.jsx`:
- Around line 8-14: The grid renders 12 divider divs regardless of viewport
while the CSS grid uses 6 columns by default and 12 at lg; update the
Array.from({ length: 12 }) in NotFoundPage.jsx so the rendered count matches the
visible columns (e.g., compute columnsCount = isLargeScreen ? 12 : 6 using a
small responsive check like window.matchMedia or a useMedia hook inside the
NotFoundPage component, then use Array.from({ length: columnsCount }) for the
mapped divs), ensuring the key/layout code that currently maps the divs remains
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce707229-b96b-49db-8d12-d7e7ca0765e7
📒 Files selected for processing (1)
frontend/src/pages/NotFoundPage.jsx
| <p className="absolute inset-0 flex items-center justify-center text-[55vw] lg:text-[30vw] font-black tracking-tighter leading-none text-black opacity-[0.04] pointer-events-none select-none"> | ||
| 404 | ||
| </p> |
There was a problem hiding this comment.
Missing aria-hidden on decorative background text.
The decorative background "404" will be announced by screen readers, creating redundant content alongside the main "404" heading. This degrades the experience for users relying on assistive technology.
♿ Proposed accessibility fix
- <p className="absolute inset-0 flex items-center justify-center text-[55vw] lg:text-[30vw] font-black tracking-tighter leading-none text-black opacity-[0.04] pointer-events-none select-none">
+ <p aria-hidden="true" className="absolute inset-0 flex items-center justify-center text-[55vw] lg:text-[30vw] font-black tracking-tighter leading-none text-black opacity-[0.04] pointer-events-none select-none">
404
</p>📝 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.
| <p className="absolute inset-0 flex items-center justify-center text-[55vw] lg:text-[30vw] font-black tracking-tighter leading-none text-black opacity-[0.04] pointer-events-none select-none"> | |
| 404 | |
| </p> | |
| <p aria-hidden="true" className="absolute inset-0 flex items-center justify-center text-[55vw] lg:text-[30vw] font-black tracking-tighter leading-none text-black opacity-[0.04] pointer-events-none select-none"> | |
| 404 | |
| </p> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/NotFoundPage.jsx` around lines 24 - 26, The decorative
background paragraph in NotFoundPage.jsx (the <p> that renders the large "404"
with classes including "absolute inset-0 ... pointer-events-none select-none")
is announced by screen readers; add aria-hidden="true" to that <p> so it is
ignored by assistive technology and only the main accessible "404" heading is
read.
|
@banthevaishnavi its good but it seems to non-centric I hopw in next updation this PR will get merged |
|
Please perform a very deep and brutally honest production-level review of this entire NotFoundPage redesign from both an engineering and modern UI/UX perspective. Do not only review the code correctness — also critically evaluate the visual quality, modernness, design maturity, spacing system, responsiveness, and overall user experience of the page. Currently the implementation still does not feel fully modern, premium, minimal, or visually refined enough for a production-grade frontend experience. Please deeply analyze whether the page genuinely achieves a clean modern brutalist aesthetic or whether it still feels crowded, unbalanced, non-centric, or visually noisy. Please specifically review:
Also please deeply analyze from a pure UI/UX perspective:
Please point out every area that can still be improved cosmetically, structurally, visually, logically, or architecturally so the final implementation becomes genuinely production-ready, modern, polished, and merge-worthy according to high-end frontend engineering and UI/UX standards. Review the FINAL current state holistically instead of only incremental diffs. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
fd "NotFoundPage.jsx" --exec cat {}Length of output: 8227 Image descriptions: PR description image 1 (full viewport): The redesigned 404 page in a split-screen layout. Left half: small "ERROR / ROUTE UNAVAILABLE" label top-left, a ghost light-grey "404" watermark filling the background, a large bold "404" heading, a thick horizontal rule, "PAGE NOT FOUND." in massive black text, and a bold description sentence bottom-left. Right half: a bold navigation prompt at the top, three CTA buttons (GO HOME filled black, EXPLORE outlined, DASHBOARD outlined) in a row, then a large vertical gap, followed by two bordered information panels side by side: "SYSTEM STATUS" (Error Code: 404 / Requested Status: ROUTE NOT FOUND / Suggested Action) and "PLATFORM ACCESS" (CodeLens branding + HOME EXPLORE LOGIN links). PR description image 2 (footer area): Shows the scrolled-down portion — bottom of the info panels, then the full site footer with CODE LENS branding/tagline, four-column link navigation (PLATFORM, INTEGRATIONS, RESOURCES, LEGAL), CONNECT section (GitHub, Twitter, LinkedIn, Discord), and a large light-grey "CODELENS" watermark below the footer.
🔴 Verdict: Not production-ready. Needs a focused second pass.The structural bones are solid — the brutalist split-screen concept is valid and the left panel's typography has genuine impact. But the right panel collapses into an information dashboard that contradicts everything a 404 page should be: simple, intentional, and immediately actionable. The page currently reads like a developer demonstrating all possible Tailwind features rather than a purposefully designed user experience. 1. 🏗️ Layout & Visual BalanceIssue — The right panel has a structural void. - <div className="mt-28 border-t-4 border-black pt-12 grid ...">
+ <div className="mt-auto border-t-4 border-black pt-12 grid ...">Or better yet, use a proper flex column with Issue — Issue — The layout does not feel visually centered. 2. 📦 Content Density — The Core ProblemThis is the single biggest issue and the root cause of what The System Status panel is noise. It tells the user three things they already know:
The Platform Access panel duplicates the CTA buttons. HOME / EXPLORE / LOGIN is a second set of navigation links that repeats GO HOME / EXPLORE which are 50px above on the same page. The "404" appears three times:
Recommendation: Remove both bottom panels entirely, or if the client insists on one, keep only a stripped-down single row of plain text links — not full bordered card components. A world-class 404 page (Vercel, Stripe, Notion, Linear) shows: error + one sentence + one CTA. That's it. 3. ✏️ Typography HierarchyIssue — Everything is
Issue — Tracking inconsistency. Issue — The ghost 404 background at 4. 🎨 Does It Feel Premium / Brutalist?Partially. The left panel has genuine brutalist impact — the scale, weight, and border are correct. But premium brutalism is defined by intentional emptiness. Linear's 404, Vercel's error pages, Clerk's UI — they all have large amounts of deliberate, confident whitespace surrounding a single focal element. This page fills every quadrant with content, which is the opposite of that aesthetic. What's working:
What's not working:
5. 📱 Responsiveness & Overflow Edge CasesCritical:
- className="mt-28 border-t-4 ..."
+ className="mt-16 lg:mt-28 border-t-4 ..."Ultra-wide (2560px+): The max-width is unconstrained. On 4K the split-screen columns become very wide. Consider adding a Mobile stacking: When the layout goes to column on mobile, the left panel (404 content) stacks above the right panel (CTAs + info panels). The info panels in a single column on mobile beneath the CTAs create a very long scroll for a 404 page. This is the strongest argument for removing the info panels. 6. ♿ Accessibility & Semantic HTML
- <section className="relative min-h-screen ...">
+ <main className="relative min-h-screen ..." aria-label="404 Not Found">- <p className="absolute inset-0 ... pointer-events-none select-none">
+ <p className="absolute inset-0 ... pointer-events-none select-none" aria-hidden="true">// Buttons need focus-visible ring:
className="... focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-black"7. 🔧 Engineering QualityRedundant - <div className="relative z-10 min-h-screen flex flex-col lg:flex-row">
+ <div className="relative z-10 flex flex-col lg:flex-row flex-1">Background grid generates 12 DOM nodes for a decorative element. // In Tailwind via arbitrary values or a style prop:
style={{ backgroundImage: 'repeating-linear-gradient(90deg, rgba(0,0,0,0.06) 0, rgba(0,0,0,0.06) 1px, transparent 0, transparent calc(100% / 12))' }}Magic number explosion in font sizes: className="text-[clamp(5rem,12vw,12rem)] font-black uppercase tracking-tighter leading-[0.85]"Maintainability: The System Status and Platform Access panels, if kept, should be extracted into 8. 🎯 Specific Recommendations Summary
9. 📐 What a Truly Polished Version Would Look Like
This is what 🐰 ✧˖°. |
|
@banthevaishnavi if you start the work than ping me |
Related Issue
issue #30
Overview
Redesigned the
NotFoundPage.jsxwith a modern full-screen brutalist-inspired responsive layout while maintaining consistency with the existing CodeLens visual system.Changes Made
UI Improvements
Testing
Screenshots
Summary by CodeRabbit
New Features
Styling