fix(dashboard): readable summary color + surface delete errors#9
Conversation
- MarkdownSummary: summary paragraphs were text-muted-foreground (too faint on white cards); use text-foreground/75 for a readable dark gray. Also drop a misleading text-secondary wrapper in the section tree (text-secondary maps to the light shadcn secondary, not dark ink). - Delete handlers (list + detail): stop swallowing failures silently; surface the HTTP status + error so a failed delete is visible instead of appearing to do nothing.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThis PR updates delete error handling in the document detail and documents list pages to surface HTTP status and error messages via alerts instead of failing silently, and adjusts MarkdownSummary text styling and its usage in the document detail page's section tree. ChangesDocument delete and summary UI updates
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/app/(dashboard)/dashboard/documents/[docId]/page.tsxOops! Something went wrong! :( ESLint: 9.39.1 TypeError: Converting circular structure to JSON ... [truncated 455 characters] ... c/dist/eslintrc.cjs:3261:25) apps/web/app/(dashboard)/dashboard/documents/page.tsxOops! Something went wrong! :( ESLint: 9.39.1 TypeError: Converting circular structure to JSON ... [truncated 455 characters] ... c/dist/eslintrc.cjs:3261:25) apps/web/components/ui/markdown.tsxOops! Something went wrong! :( ESLint: 9.39.1 TypeError: Converting circular structure to JSON ... [truncated 455 characters] ... c/dist/eslintrc.cjs:3261:25) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/app/(dashboard)/dashboard/documents/[docId]/page.tsx (1)
337-361: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting shared delete-error handling.
This exact error-parsing/alert pattern (parse optional JSON
error, alert with HTTP status, catch/alert on network error) is duplicated verbatim inapps/web/app/(dashboard)/dashboard/documents/page.tsx. Extracting a small shared helper (e.g.deleteDocument(docId)returning a result/throwing a formatted error) would avoid drift between the two copies.♻️ Suggested shared helper
// e.g. apps/web/lib/documents.ts export async function deleteDocument(docId: string): Promise<void> { const res = await fetch(`/api/dashboard/documents/${docId}`, { method: "DELETE" }); if (res.ok) return; const body = (await res.json().catch(() => null)) as { error?: string } | null; throw new Error(`Delete failed (HTTP ${res.status})${body?.error ? `: ${body.error}` : ""}`); }Both call sites then just
try { await deleteDocument(docId); ... } catch (e) { alert(e instanceof Error ? e.message : "Delete failed: network error"); }.🤖 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 `@apps/web/app/`(dashboard)/dashboard/documents/[docId]/page.tsx around lines 337 - 361, The delete flow in handleDelete duplicates the same HTTP/JSON error parsing and alert formatting used in the documents list page, so extract that logic into a shared helper such as deleteDocument(docId) and reuse it from both call sites. Move the fetch + optional body.error parsing into the helper, have it return success or throw a formatted Error, and keep the page-level try/catch in page.tsx limited to navigation and a single alert fallback for network errors.
🤖 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 `@apps/web/app/`(dashboard)/dashboard/documents/[docId]/page.tsx:
- Around line 337-361: The delete flow in handleDelete duplicates the same
HTTP/JSON error parsing and alert formatting used in the documents list page, so
extract that logic into a shared helper such as deleteDocument(docId) and reuse
it from both call sites. Move the fetch + optional body.error parsing into the
helper, have it return success or throw a formatted Error, and keep the
page-level try/catch in page.tsx limited to navigation and a single alert
fallback for network errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31b61644-b19f-47dc-958e-95f9320c6363
📒 Files selected for processing (3)
apps/web/app/(dashboard)/dashboard/documents/[docId]/page.tsxapps/web/app/(dashboard)/dashboard/documents/page.tsxapps/web/components/ui/markdown.tsx
1 participant
text-muted-foregroundon white cards). Nowtext-foreground/75— a readable dark gray. Also removed a misleadingtext-secondarywrapper (maps to the light shadcn secondary, not dark ink).catch {}, only actedif (res.ok)), so a failing delete looked like nothing happened. Now they surface the HTTP status + error message — so we can finally see why delete is failing.Summary by CodeRabbit