-
Notifications
You must be signed in to change notification settings - Fork 777
bug: improve 404 page with search, back navigation and bug report #4376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import { render } from '@testing-library/react'; | ||
| import { MemoryRouter } from 'react-router-dom'; | ||
| import NotFound from '../pages/NotFound'; | ||
|
|
||
| const setSearchParams = vi.fn(); | ||
|
|
||
| vi.mock('react-router-dom', async () => { | ||
| const actual = await vi.importActual('react-router-dom'); | ||
| return { | ||
| ...actual, | ||
| useLocation: () => ({ pathname: '/missing', search: '?q=abc' }), | ||
| useNavigate: () => vi.fn(), | ||
| useSearchParams: () => [new URLSearchParams(), setSearchParams], | ||
| }; | ||
| }); | ||
|
|
||
| vi.mock('../components/SearchInput', () => ({ | ||
| default: ({ value, onChange }) => <input value={value} onChange={(event) => onChange(event.target.value)} />, | ||
| })); | ||
|
|
||
| vi.mock('../components/ReportBugModal', () => ({ | ||
| default: () => null, | ||
| })); | ||
|
|
||
| describe('NotFound page', () => { | ||
| beforeEach(() => { | ||
| setSearchParams.mockClear(); | ||
| }); | ||
|
|
||
| test('syncs the search query without adding a history entry', () => { | ||
| render( | ||
| <MemoryRouter> | ||
| <NotFound /> | ||
| </MemoryRouter> | ||
| ); | ||
|
|
||
| expect(setSearchParams).toHaveBeenCalledWith({ q: '/missing?q=abc' }, { replace: true }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import { fireEvent, render, screen } from '@testing-library/react' | ||
| import ReportBugModal from '../components/ReportBugModal' | ||
|
|
||
| describe('ReportBugModal', () => { | ||
| test('exposes dialog semantics and closes on Escape', () => { | ||
| const onClose = vi.fn() | ||
|
|
||
| render(<ReportBugModal isOpen onClose={onClose} invalidPath="/missing" />) | ||
|
|
||
| const dialog = screen.getByRole('dialog', { name: /report broken link/i }) | ||
| expect(dialog).toHaveAttribute('aria-modal', 'true') | ||
| expect(dialog).toHaveFocus() | ||
|
|
||
| fireEvent.keyDown(document, { key: 'Escape' }) | ||
| expect(onClose).toHaveBeenCalledTimes(1) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,124 +1,135 @@ | ||
| import React, { useState } from 'react'; | ||
| import { motion, AnimatePresence } from 'framer-motion'; | ||
| import { X, Bug, Loader2 } from 'lucide-react'; | ||
| import toast from 'react-hot-toast'; | ||
| import { bugsApi } from '../services/api'; | ||
| import { useEffect, useRef, useState } from 'react' | ||
| import { motion } from 'framer-motion' | ||
| import { X, Bug } from 'lucide-react' | ||
| import toast from 'react-hot-toast' | ||
|
|
||
| export default function ReportBugModal({ isOpen, onClose }) { | ||
| const [title, setTitle] = useState(''); | ||
| const [description, setDescription] = useState(''); | ||
| const [isSubmitting, setIsSubmitting] = useState(false); | ||
| export default function ReportBugModal({ isOpen, onClose, invalidPath }) { | ||
| const [reportBody, setReportBody] = useState('') | ||
| const dialogRef = useRef(null) | ||
|
|
||
| const handleSubmit = async (e) => { | ||
| e.preventDefault(); | ||
|
|
||
| if (!title.trim() || !description.trim()) { | ||
| toast.error('Please fill out both title and description.'); | ||
| return; | ||
| useEffect(() => { | ||
| if (!isOpen) return | ||
|
|
||
| setReportBody( | ||
| `Invalid URL: ${invalidPath}\n\nPlease describe what you were expecting to find and any additional details that would help us fix this broken link.` | ||
| ) | ||
|
|
||
| const dialog = dialogRef.current | ||
| if (dialog) { | ||
| dialog.focus() | ||
| } | ||
| }, [isOpen, invalidPath]) | ||
|
|
||
| useEffect(() => { | ||
| if (!isOpen) return | ||
|
|
||
| const handleKeyDown = (event) => { | ||
| if (event.key === 'Escape') { | ||
| event.preventDefault() | ||
| onClose() | ||
| } | ||
| } | ||
|
|
||
| document.addEventListener('keydown', handleKeyDown) | ||
| return () => document.removeEventListener('keydown', handleKeyDown) | ||
| }, [isOpen, onClose]) | ||
|
|
||
| const handleSubmit = async (event) => { | ||
| event.preventDefault() | ||
|
|
||
| setIsSubmitting(true); | ||
| try { | ||
| await bugsApi.submitBug(title, description); | ||
| toast.success('Bug reported successfully! Thank you.'); | ||
| setTitle(''); | ||
| setDescription(''); | ||
| onClose(); | ||
| await navigator.clipboard.writeText(reportBody) | ||
| toast.success('Bug report copied to clipboard!') | ||
| onClose() | ||
| } catch (error) { | ||
| console.error('Error reporting bug:', error); | ||
| toast.error(error.message || 'Failed to report bug. Please try again later.'); | ||
| } finally { | ||
| setIsSubmitting(false); | ||
| toast.error('Unable to copy report to clipboard. Please try again.') | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| if (!isOpen) { | ||
| return null | ||
| } | ||
|
|
||
| return ( | ||
| <AnimatePresence> | ||
| {isOpen && ( | ||
| <div className="fixed inset-0 z-50 flex items-center justify-center p-4"> | ||
| <motion.div | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| exit={{ opacity: 0 }} | ||
| <div className="fixed inset-0 z-50 flex items-center justify-center p-4 overflow-y-auto"> | ||
| <motion.div | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| exit={{ opacity: 0 }} | ||
| onClick={onClose} | ||
| className="fixed inset-0 bg-black/70 backdrop-blur-sm" | ||
| /> | ||
|
|
||
| <motion.div | ||
| initial={{ opacity: 0, scale: 0.96, y: 20 }} | ||
| animate={{ opacity: 1, scale: 1, y: 0 }} | ||
| exit={{ opacity: 0, scale: 0.96, y: 20 }} | ||
| ref={dialogRef} | ||
| role="dialog" | ||
| aria-modal="true" | ||
| aria-labelledby="report-bug-title" | ||
| aria-describedby="report-bug-description" | ||
| tabIndex={-1} | ||
| className="relative w-full max-w-2xl rounded-3xl border border-white/10 bg-[#0c0c0c] shadow-2xl outline-none" | ||
| > | ||
| <div className="flex items-start justify-between gap-4 border-b border-white/10 px-6 py-5"> | ||
| <div className="flex items-center gap-3"> | ||
| <div className="rounded-2xl bg-[#00ffaa]/10 p-3 text-[#00ffaa]"> | ||
| <Bug className="h-5 w-5" /> | ||
| </div> | ||
| <div> | ||
| <h2 id="report-bug-title" className="text-lg font-semibold text-white">Report broken link</h2> | ||
| <p id="report-bug-description" className="text-sm text-neutral-400">The invalid URL is prefilled so your report is ready to share.</p> | ||
| </div> | ||
| </div> | ||
| <button | ||
| type="button" | ||
| onClick={onClose} | ||
| className="absolute inset-0 bg-black/60 backdrop-blur-sm" | ||
| /> | ||
|
|
||
| <motion.div | ||
| initial={{ opacity: 0, scale: 0.95, y: 20 }} | ||
| animate={{ opacity: 1, scale: 1, y: 0 }} | ||
| exit={{ opacity: 0, scale: 0.95, y: 20 }} | ||
| className="relative w-full max-w-lg bg-card rounded-2xl shadow-xl overflow-hidden border border-border" | ||
| className="rounded-full p-2 text-neutral-400 transition hover:bg-white/10 hover:text-white" | ||
| aria-label="Close report modal" | ||
| > | ||
| <div className="flex items-center justify-between px-6 py-4 border-b border-border"> | ||
| <div className="flex items-center gap-2 text-foreground"> | ||
| <Bug className="w-5 h-5 text-red-500" /> | ||
| <h2 className="text-lg font-bold">Report a Bug</h2> | ||
| </div> | ||
| <button | ||
| onClick={onClose} | ||
| className="p-2 rounded-xl hover:bg-muted text-muted-foreground transition-colors" | ||
| > | ||
| <X className="w-5 h-5" /> | ||
| </button> | ||
| <X className="h-5 w-5" /> | ||
| </button> | ||
| </div> | ||
|
|
||
| <form onSubmit={handleSubmit} className="space-y-4 px-6 py-6"> | ||
| <div> | ||
| <label className="mb-2 block text-sm font-semibold text-neutral-300">Invalid URL</label> | ||
| <div className="rounded-2xl border border-white/10 bg-[#111111] px-4 py-3 text-sm text-[#00ffaa] font-mono break-words"> | ||
| {invalidPath} | ||
| </div> | ||
| </div> | ||
|
|
||
| <form onSubmit={handleSubmit} className="p-6 space-y-4"> | ||
| <div> | ||
| <label className="block text-sm font-semibold text-foreground mb-1"> | ||
| Title | ||
| </label> | ||
| <input | ||
| type="text" | ||
| value={title} | ||
| onChange={(e) => setTitle(e.target.value)} | ||
| placeholder="E.g., Resume builder fails to save" | ||
| className="w-full px-4 py-2 bg-background border border-border rounded-xl text-foreground placeholder:text-muted-foreground focus:outline-none focus:ring-2 focus:ring-primary/50" | ||
| maxLength={100} | ||
| required | ||
| /> | ||
| </div> | ||
|
|
||
| <div> | ||
| <label className="block text-sm font-semibold text-foreground mb-1"> | ||
| Description | ||
| </label> | ||
| <textarea | ||
| value={description} | ||
| onChange={(e) => setDescription(e.target.value)} | ||
| placeholder="Please describe what you were doing when the issue occurred..." | ||
| className="w-full px-4 py-2 h-32 resize-none bg-background border border-border rounded-xl text-foreground placeholder:text-muted-foreground focus:outline-none focus:ring-2 focus:ring-primary/50" | ||
| required | ||
| /> | ||
| </div> | ||
| <div> | ||
| <label htmlFor="bug-report" className="mb-2 block text-sm font-semibold text-neutral-300"> | ||
| Report details | ||
| </label> | ||
| <textarea | ||
| id="bug-report" | ||
| value={reportBody} | ||
| onChange={(event) => setReportBody(event.target.value)} | ||
| rows={7} | ||
| className="w-full resize-none rounded-2xl border border-white/10 bg-[#111111] px-4 py-3 text-sm text-white outline-none transition focus:border-[#00ffaa] focus:ring-2 focus:ring-[#00ffaa]/20" | ||
| /> | ||
| </div> | ||
|
|
||
| <div className="pt-2 flex justify-end gap-3"> | ||
| <button | ||
| type="button" | ||
| onClick={onClose} | ||
| className="px-4 py-2 text-sm font-semibold text-foreground hover:bg-muted rounded-xl transition-colors" | ||
| > | ||
| Cancel | ||
| </button> | ||
| <button | ||
| type="submit" | ||
| disabled={isSubmitting} | ||
| className="flex items-center gap-2 px-6 py-2 bg-primary text-primary-foreground text-sm font-semibold rounded-xl hover:opacity-90 transition-opacity disabled:opacity-50 disabled:cursor-not-allowed" | ||
| > | ||
| {isSubmitting ? ( | ||
| <> | ||
| <Loader2 className="w-4 h-4 animate-spin" /> | ||
| Submitting... | ||
| </> | ||
| ) : ( | ||
| 'Submit Bug' | ||
| )} | ||
| </button> | ||
| </div> | ||
| </form> | ||
| </motion.div> | ||
| </div> | ||
| )} | ||
| </AnimatePresence> | ||
| ); | ||
| <div className="flex flex-col gap-3 sm:flex-row sm:justify-end"> | ||
| <button | ||
| type="button" | ||
| onClick={onClose} | ||
| className="rounded-2xl border border-white/10 bg-[#111111] px-5 py-3 text-sm text-neutral-200 transition hover:border-[#00ffaa] hover:text-white" | ||
| > | ||
| Cancel | ||
| </button> | ||
| <button | ||
| type="submit" | ||
| className="rounded-2xl bg-[#00ffaa] px-5 py-3 text-sm font-semibold text-[#0a0a0a] transition hover:bg-[#00e699]" | ||
| > | ||
| Copy report | ||
| </button> | ||
| </div> | ||
| </form> | ||
| </motion.div> | ||
| </div> | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,15 @@ | ||
| import React, { useState, useEffect } from 'react'; | ||
| import { Link } from 'react-router-dom'; | ||
| import { Link, useLocation, useNavigate, useSearchParams } from 'react-router-dom'; | ||
| import SearchInput from '../components/SearchInput'; | ||
| import ReportBugModal from '../components/ReportBugModal'; | ||
|
|
||
| const NotFound = () => { | ||
| const location = useLocation(); | ||
| const navigate = useNavigate(); | ||
| const [searchParams, setSearchParams] = useSearchParams(); | ||
| const invalidPath = `${location.pathname}${location.search}`; | ||
| const [searchQuery, setSearchQuery] = useState(searchParams.get('q') || invalidPath); | ||
| const [isReportOpen, setIsReportOpen] = useState(false); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| const [terminalText, setTerminalText] = useState(''); | ||
| const fullText = [ | ||
| '> git checkout page', | ||
|
|
@@ -33,6 +41,10 @@ const NotFound = () => { | |
| return () => clearInterval(typingInterval); | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| setSearchParams({ q: searchQuery }, { replace: true }); | ||
| }, [searchQuery, setSearchParams]); | ||
|
|
||
| return ( | ||
| <div className="min-h-screen flex flex-col items-center justify-center bg-[#0a0a0a] text-white p-6 font-mono overflow-hidden"> | ||
| {/* Glitching 404 Heading */} | ||
|
|
@@ -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)} | ||
|
Comment on lines
44
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
|
||
| className="w-full rounded-2xl border border-white/10 bg-[#111111]/90 px-5 py-4 text-sm font-semibold text-white transition hover:border-[#00ffaa] hover:text-[#00ffaa]" | ||
| > | ||
| Go Back | ||
| </button> | ||
| <Link | ||
| to="/dashboard" | ||
| className="w-full rounded-2xl bg-[#00ffaa] px-5 py-4 text-sm font-semibold text-[#0a0a0a] transition hover:bg-[#00e699]" | ||
| > | ||
| git checkout dashboard | ||
| </Link> | ||
| <button | ||
| type="button" | ||
| onClick={() => setIsReportOpen(true)} | ||
| className="w-full rounded-2xl border border-white/10 bg-[#111111]/90 px-5 py-4 text-sm font-semibold text-white transition hover:border-[#00ffaa] hover:text-[#00ffaa]" | ||
| > | ||
| Report this broken link | ||
| </button> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="w-full max-w-xl rounded-3xl border border-white/10 bg-[#111111]/80 p-6 text-left text-sm text-neutral-300 shadow-2xl mb-8"> | ||
| <p className="text-xs uppercase tracking-[0.25em] text-[#00ffaa]/80 mb-2">Invalid URL</p> | ||
| <p className="break-words font-mono text-sm text-white">{invalidPath}</p> | ||
| </div> | ||
|
|
||
| <ReportBugModal | ||
| isOpen={isReportOpen} | ||
| onClose={() => setIsReportOpen(false)} | ||
| invalidPath={invalidPath} | ||
| /> | ||
|
|
||
| {/* Subtle background element */} | ||
| <div className="absolute top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2 w-[500px] h-[500px] bg-[#00ffaa] opacity-[0.03] blur-[100px] -z-10 rounded-full"></div> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| {"numTotalTestSuites":2,"numPassedTestSuites":2,"numFailedTestSuites":0,"numPendingTestSuites":0,"numTotalTests":1,"numPassedTests":1,"numFailedTests":0,"numPendingTests":0,"numTodoTests":0,"snapshot":{"added":0,"failure":false,"filesAdded":0,"filesRemoved":0,"filesRemovedList":[],"filesUnmatched":0,"filesUpdated":0,"matched":0,"total":0,"unchecked":0,"uncheckedKeysByFile":[],"unmatched":0,"updated":0,"didUpdate":false},"startTime":1783100119860,"success":true,"testResults":[{"assertionResults":[{"ancestorTitles":["NotFound page"],"fullName":"NotFound page syncs the search query without adding a history entry","status":"passed","title":"syncs the search query without adding a history entry","duration":146.39840000000004,"failureMessages":[],"meta":{},"tags":[]}],"startTime":1783100126320,"endTime":1783100126466.3984,"status":"passed","message":"","name":"C:/careerpilot/career-pilot/frontend/src/__tests__/NotFound.test.jsx"}]} |
Uh oh!
There was an error while loading. Please reload this page.