bug: improve 404 page with search, back navigation and bug report#4376
bug: improve 404 page with search, back navigation and bug report#4376Ranjan-githu wants to merge 4 commits into
Conversation
|
@Ranjan-githu is attempting to deploy a commit to the Anurag Mishra's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
CodeAnt AI is reviewing your PR. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughAdds a Changes404 Page Enhancements
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant NotFound
participant SearchInput
participant ReportBugModal
User->>NotFound: Visits invalid URL
NotFound->>NotFound: Derive invalidPath, searchQuery
NotFound->>SearchInput: Bind searchQuery
User->>SearchInput: Types query
SearchInput->>NotFound: Update searchQuery
NotFound->>NotFound: Sync searchQuery to URL params
User->>NotFound: Clicks "Report this broken link"
NotFound->>ReportBugModal: Open with invalidPath
User->>ReportBugModal: Submit report
ReportBugModal->>ReportBugModal: Copy report to clipboard
ReportBugModal->>NotFound: Close modal
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| @@ -73,16 +85,53 @@ const NotFound = () => { | |||
| </div> | |||
| </div> | |||
|
|
|||
| {/* Action Button */} | |||
| <Link | |||
| to="/dashboard" | |||
| className="px-8 py-4 bg-[#00ffaa] text-[#0a0a0a] font-bold rounded-xl transition-all duration-300 transform hover:scale-105 hover:bg-[#00e699] active:scale-95 focus:outline-none focus:ring-2 focus:ring-[#00ffaa] focus:border-transparent flex items-center gap-2 shadow-[0_0_30px_rgba(0,255,170,0.2)]" | |||
| > | |||
| <span>git checkout dashboard</span> | |||
| <svg xmlns="http://www.w3.org/2000/svg" className="h-5 w-5" viewBox="0 0 20 20" fill="currentColor"> | |||
| <path fillRule="evenodd" d="M10.293 3.293a1 1 0 011.414 0l6 6a1 1 0 010 1.414l-6 6a1 1 0 01-1.414-1.414L14.586 11H3a1 1 0 110-2h11.586l-4.293-4.293a1 1 0 010-1.414z" clipRule="evenodd" /> | |||
| </svg> | |||
| </Link> | |||
| {/* Search + CTA Section */} | |||
| <div className="w-full max-w-xl space-y-6 mb-10"> | |||
| <div className="rounded-3xl border border-white/10 bg-[#111111]/80 p-6 shadow-2xl"> | |||
| <div className="mb-4 text-sm text-neutral-400">Search this path or refine your query</div> | |||
| <SearchInput | |||
| name="notfound-search" | |||
| value={searchQuery} | |||
| placeholder="Search for a page, feature, or help topic" | |||
| onChange={setSearchQuery} | |||
| wrapperClassName="w-full" | |||
| /> | |||
| </div> | |||
|
|
|||
| <div className="grid gap-3 sm:grid-cols-3"> | |||
| <button | |||
| type="button" | |||
| onClick={() => navigate(-1)} | |||
There was a problem hiding this comment.
Suggestion: Updating search params on each query change pushes new history entries by default, so the “Go Back” action can step through many local query updates instead of returning to the previous page. Use replace-mode updates for these in-page query sync writes to avoid polluting navigation history. [logic error]
Severity Level: Major ⚠️
- ❌ Go Back button often fails to leave 404 page.
- ⚠️ Browser back stack polluted by search param entries.
- ⚠️ Users must click back many times to exit 404.
- ⚠️ Navigation UX degraded specifically on NotFound route.Steps of Reproduction ✅
1. Navigate to any existing page in the app, then follow a link or manually enter a URL
that does not match any defined route (e.g. `/nonexistent-feature`); the catch-all route
`<Route path="*" element=<NotFound /> />` in `frontend/src/App.jsx:64-65` renders
`frontend/src/pages/NotFound.jsx`.
2. In `NotFound.jsx`, the search state and URL syncing are set up at lines 9-11 and 44-46:
`useSearchParams()` provides `[searchParams, setSearchParams]` (line 9), `searchQuery`
state is initialised from `searchParams.get('q') || invalidPath` (line 11), and the effect
at lines 44-46 calls `setSearchParams({ q: searchQuery });` on every `searchQuery` change.
3. Type a multi-character term into the `SearchInput` rendered at lines 88-99.
`SearchInput` internally debounces and calls the parent `onChange={setSearchQuery}`
(`frontend/src/components/SearchInput.jsx:62-67` and `NotFound.jsx:96`), causing
`searchQuery` in `NotFound` to change several times as the user types (e.g. "r", "re",
"res", "resume").
4. Each `searchQuery` update triggers the effect at `NotFound.jsx:44-46`, which calls
`setSearchParams({ q: searchQuery })` with default push behavior, creating a new browser
history entry for each intermediate query value on the same NotFound route. When the user
clicks the "Go Back" button at lines 102-107 (`onClick={() => navigate(-1)}`), navigation
steps back through these local query-only history entries instead of returning to the
previous page, so back navigation appears to "do nothing" or only change the URL’s `q`
parameter while keeping the user stuck on the 404 screen.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** frontend/src/pages/NotFound.jsx
**Line:** 44:104
**Comment:**
*Logic Error: Updating search params on each query change pushes new history entries by default, so the “Go Back” action can step through many local query updates instead of returning to the previous page. Use replace-mode updates for these in-page query sync writes to avoid polluting navigation history.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
done please review my changes
|
CodeAnt AI finished reviewing your PR. |
|
Hi @maintainers, I've submitted a PR for this issue: #4259 The PR includes:
I'd appreciate your feedback. Thank you! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/src/components/ReportBugModal.jsx (1)
35-48: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueExit animations never run — no
AnimatePresencein the tree.
exitprops on bothmotion.divs (lines 38, 46) are inert here since the component returnsnulldirectly (line 29-31) instead of being conditionally rendered inside<AnimatePresence>. Without it, the exit prop does absolutely nothing and the modal will disappear instantly rather than animating out.Fixing this requires the parent (
NotFound.jsx) to wrap the<ReportBugModal>render with<AnimatePresence>and gate rendering byisOpenthere, rather than short-circuiting inside this component.🤖 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/components/ReportBugModal.jsx` around lines 35 - 48, The modal’s `exit` animations in `ReportBugModal` are currently inert because the component short-circuits to `null` before Framer Motion can animate unmounting. Move the conditional rendering to the parent `NotFound` component so `<ReportBugModal>` is mounted/unmounted inside `<AnimatePresence>`, and gate it with `isOpen` there. Keep the `motion.div` `exit` props in `ReportBugModal`, but ensure the parent controls visibility rather than returning `null` inside the modal itself.frontend/src/pages/NotFound.jsx (1)
101-121: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLGTM on the CTA wiring for Go Back / dashboard link / report trigger — the duplicated Tailwind class strings between the two secondary buttons (Lines 102-108, 115-121) could be extracted into a shared class constant, but this is purely cosmetic.
🤖 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/NotFound.jsx` around lines 101 - 121, The two secondary buttons in NotFound share the same Tailwind class string, so extract that repeated styling into a shared constant within NotFound.jsx and reuse it for both the Go Back and Report this broken link buttons. Keep the existing button behavior and text unchanged, and use the shared class name near the button JSX to make the duplication easy to spot.
🤖 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/components/ReportBugModal.jsx`:
- Around line 33-48: The ReportBugModal overlay currently lacks accessible modal
behavior, so update the modal container in ReportBugModal to expose proper
dialog semantics and keyboard support. Add dialog attributes and a
label/description hookup on the main motion.div, wire Escape key handling to
call onClose, and ensure the modal grabs initial focus when opened and keeps
focus trapped within it until dismissed. Use the existing onClose prop and the
modal wrapper elements in ReportBugModal to implement this cleanly.
In `@frontend/src/pages/NotFound.jsx`:
- Around line 2-12: Freeze the originally requested URL in NotFound and stop
deriving it from the live location/search state after mount. Use a stable ref or
memo in NotFound (around useLocation, useSearchParams, and invalidPath) to
preserve the initial path/query for the report modal, and change the
search-param sync so setSearchParams for q uses replace: true instead of pushing
a new history entry. Keep searchQuery initialization and modal prefill tied to
the frozen initial value so edits are not wiped when location.search changes.
---
Nitpick comments:
In `@frontend/src/components/ReportBugModal.jsx`:
- Around line 35-48: The modal’s `exit` animations in `ReportBugModal` are
currently inert because the component short-circuits to `null` before Framer
Motion can animate unmounting. Move the conditional rendering to the parent
`NotFound` component so `<ReportBugModal>` is mounted/unmounted inside
`<AnimatePresence>`, and gate it with `isOpen` there. Keep the `motion.div`
`exit` props in `ReportBugModal`, but ensure the parent controls visibility
rather than returning `null` inside the modal itself.
In `@frontend/src/pages/NotFound.jsx`:
- Around line 101-121: The two secondary buttons in NotFound share the same
Tailwind class string, so extract that repeated styling into a shared constant
within NotFound.jsx and reuse it for both the Go Back and Report this broken
link buttons. Keep the existing button behavior and text unchanged, and use the
shared class name near the button JSX to make the duplication easy to spot.
🪄 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: 84832d5e-3232-4c1d-b7b9-ff931b1806e3
📒 Files selected for processing (2)
frontend/src/components/ReportBugModal.jsxfrontend/src/pages/NotFound.jsx
|
CodeAnt AI is running Incremental review |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:L
|
CodeAnt AI Incremental review completed. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/__tests__/NotFound.test.jsx (2)
11-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfusing/unrealistic mock:
location.searchembedsq=abcwhileuseSearchParams()is mocked empty.In real react-router,
useLocation().searchanduseSearchParams()are derived from the same URL and would be consistent. Herelocation.searchis'?q=abc'(looking like an existing query param) whileuseSearchParams()returns an emptyURLSearchParams, so the assertion only passes becausesearchParams.get('q')falls through toinvalidPath. Consider using a search string without aq=substring (e.g.,?foo=bar) to avoid the coincidental overlap and make the test's intent clearer.Proposed clarification
- useLocation: () => ({ pathname: '/missing', search: '?q=abc' }), + useLocation: () => ({ pathname: '/missing', search: '?foo=bar' }),and update the assertion to
'/missing?foo=bar'accordingly.🤖 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/__tests__/NotFound.test.jsx` around lines 11 - 13, The NotFound test mock is inconsistent because NotFound’s location search and useSearchParams values should reflect the same URL, but the current pathname/search combination creates a misleading q=abc overlap. Update the mocked useLocation() search in NotFound.test.jsx to a query string without q=... (for example, a different param), and adjust the expectation in the NotFound assertion so it matches the new URL shape used by the component.
30-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest only covers query-sync effect; missing coverage for Go Back and Report Broken Link CTAs.
Per the linked issue's acceptance criteria, the 404 page must provide three CTAs: search, go back, and report (with the attempted URL pre-filled). This suite verifies only the
setSearchParamssync behavior — there's no assertion that clicking "Go Back" callsnavigate(-1), or that opening the report modal passes the invalid URL through. Consider adding tests for these paths given they're the PR's core acceptance criteria.🤖 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/__tests__/NotFound.test.jsx` around lines 30 - 38, The NotFound test suite only covers the query sync behavior, but it still needs coverage for the other acceptance-criteria CTAs. Add tests around the NotFound component that verify clicking the Go Back CTA triggers navigate(-1), and that the Report Broken Link CTA opens the report flow with the attempted URL passed through as the prefilled value. Use the existing NotFound, setSearchParams, and navigate/report-related mocks to locate and assert these interactions.
🤖 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.
Nitpick comments:
In `@frontend/src/__tests__/NotFound.test.jsx`:
- Around line 11-13: The NotFound test mock is inconsistent because NotFound’s
location search and useSearchParams values should reflect the same URL, but the
current pathname/search combination creates a misleading q=abc overlap. Update
the mocked useLocation() search in NotFound.test.jsx to a query string without
q=... (for example, a different param), and adjust the expectation in the
NotFound assertion so it matches the new URL shape used by the component.
- Around line 30-38: The NotFound test suite only covers the query sync
behavior, but it still needs coverage for the other acceptance-criteria CTAs.
Add tests around the NotFound component that verify clicking the Go Back CTA
triggers navigate(-1), and that the Report Broken Link CTA opens the report flow
with the attempted URL passed through as the prefilled value. Use the existing
NotFound, setSearchParams, and navigate/report-related mocks to locate and
assert these interactions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aad1fe1f-45ba-46c9-b4be-2a1a0333c816
📒 Files selected for processing (3)
frontend/src/__tests__/NotFound.test.jsxfrontend/src/pages/NotFound.jsxfrontend/vitest-notfound.json
✅ Files skipped from review due to trivial changes (1)
- frontend/vitest-notfound.json
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/pages/NotFound.jsx
1 participant
User description
Description
Summary
This PR improves the 404 (Not Found) page by adding helpful navigation and recovery options for users.
Changes Made
SearchInputcomponent.navigate(-1).ReportBugModal.Why
The previous 404 page only provided a single action (
git checkout dashboard), making it difficult for users to recover from navigation errors. These additions improve the user experience by providing multiple recovery options while maintaining the existing look and feel.Acceptance Criteria
useSearchParamsType of Change
Bug fix
Related Issue
Fixes #4259
Testing
Unit tests pass
Tested Locally
Screenshots (MANDATORY for UI/UX changes)
before
After
Checklist
Code follows project style guidelines
Self-reviewed my code
Added comments where necessary
No new warnings generated
Summary by CodeRabbit
CodeAnt-AI Description
Make the 404 page help users search, go back, or report a broken link
What Changed
Impact
✅ Easier recovery from broken links✅ Faster navigation back to a working page✅ Clearer broken-link reports💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.