Skip to content

🚸 Improve private playlist access error UX#56

Merged
nbsp1221 merged 1 commit intomainfrom
ux/private-playlist-error-ux
Oct 6, 2025
Merged

🚸 Improve private playlist access error UX#56
nbsp1221 merged 1 commit intomainfrom
ux/private-playlist-error-ux

Conversation

@nbsp1221
Copy link
Owner

@nbsp1221 nbsp1221 commented Oct 5, 2025

Summary

  • add a route-level error boundary for private playlist access failures
  • present a shadcn alert with clear messaging and navigation options instead of a blank failure state

Testing

  • bun run lint
  • bun run test
  • bun run build
  • bun run typecheck
  • manual: Playwright E2E flow

Summary by CodeRabbit

  • New Features
    • Added a dedicated error page for playlist details with tailored messages for access denied (403), not found (404), and unexpected errors.
    • Provides clear guidance and two action buttons to help users recover or navigate.
    • Uses the app’s standard layout for a consistent look and feel across error states.

@coderabbitai
Copy link

coderabbitai bot commented Oct 5, 2025

Walkthrough

Introduces an ErrorBoundary for the playlists.$id route using react-router’s route error utilities. It renders an AppLayout with an Alert and action buttons, handling route error responses (403, 404, others) and generic Error instances. Imports are expanded to include required UI components and Link.

Changes

Cohort / File(s) Summary of Changes
Route error handling update
app/routes/playlists.$id.tsx
Added export function ErrorBoundary() using useRouteError and isRouteErrorResponse to render route-aware error UI via AppLayout, Alert, and Button. Expanded imports to include needed UI and navigation components. Replaces prior loader-based error handling with a dedicated boundary.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Router
  participant Route as playlists.$id Route
  participant EB as ErrorBoundary
  participant UI as AppLayout/Alert/Buttons

  User->>Router: Navigate to /playlists/:id
  Router->>Route: Load route
  alt Load succeeds
    Route-->>User: Render route content
  else Load throws route error
    Router->>EB: Provide error via useRouteError()
    EB->>EB: Detect 403/404/other via isRouteErrorResponse
    EB->>UI: Render AppLayout + Alert with title/description
    UI-->>User: Show error UI with action buttons/links
  else Generic Error
    Router->>EB: Provide Error instance
    EB->>UI: Render generic error alert in AppLayout
    UI-->>User: Show fallback UI
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through routes with careful cheer,
A boundary blooms when bugs appear;
403? I twitch an ear—
404? I nibble fear;
Wrapped in alerts, the path is clear,
Two buttons bright: to persevere—
Thump-thump! The playlist’s near.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely summarizes the primary change—enhancing the error UX for private playlist access—and directly relates to the added route-level error boundary while remaining focused and understandable to team members scanning history.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ux/private-playlist-error-ux

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e17fb10 and fa39cff.

📒 Files selected for processing (1)
  • app/routes/playlists.$id.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: TypeScript에서 any를 사용하지 않고 모든 함수에 포괄적인 타입 힌트를 제공한다 (strict TS)
로그에 비밀값을 남기지 않도록 모든 콘솔 출력은 민감정보를 마스킹/제거한다

**/*.{ts,tsx}: Use the ~/* path alias (from tsconfig.json) for internal imports
Prefer explicit interfaces when defining object shapes in TypeScript (strict mode)
Avoid using any in TypeScript (project runs in strict mode)
Use two-space indentation throughout TypeScript/TSX files
Name components, hooks, and providers with PascalCase
Name helpers and state setters with camelCase
File names should mirror their primary export

Files:

  • app/routes/playlists.$id.tsx
**/*.{ts,tsx,md,mdx}

📄 CodeRabbit inference engine (CLAUDE.md)

코드 주석과 문서는 영어로 작성한다

Files:

  • app/routes/playlists.$id.tsx
app/**

📄 CodeRabbit inference engine (AGENTS.md)

Place all React Router app source under app/ following the repository’s feature-sliced layout

Files:

  • app/routes/playlists.$id.tsx
**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Split multiline JSX props across lines per Stylistic ESLint rules

Files:

  • app/routes/playlists.$id.tsx
🧬 Code graph analysis (1)
app/routes/playlists.$id.tsx (1)
app/components/AppLayout.tsx (1)
  • AppLayout (30-42)

Comment on lines +112 to +129
<AppLayout>
<div className="mx-auto flex w-full max-w-2xl flex-col gap-6 px-6 py-16">
<Alert>
<AlertTitle>{title}</AlertTitle>
<AlertDescription>
<p>{description}</p>
</AlertDescription>
</Alert>
<div className="flex flex-wrap gap-3">
<Button asChild>
<Link to={primaryAction.to}>{primaryAction.label}</Link>
</Button>
<Button asChild variant="outline">
<Link to={secondaryAction.to}>{secondaryAction.label}</Link>
</Button>
</div>
</div>
</AppLayout>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Provide required AppLayout props.

AppLayout destructures searchQuery, onSearchChange, and pendingCount without defaults, so calling it with only children leaves those props undefined, triggering type errors and breaking the layout’s search bar logic. Please pass the same props the route normally supplies (e.g., searchQuery="", onSearchChange={() => {}}, pendingCount={0} or the real values from context) before returning the boundary UI.

🤖 Prompt for AI Agents
In app/routes/playlists.$id.tsx around lines 112 to 129, AppLayout is being
rendered without required props (searchQuery, onSearchChange, pendingCount)
which the component destructures with no defaults; update the AppLayout
invocation to pass those props — e.g. supply searchQuery="" (or the real query),
onSearchChange={() => {}} (or the real handler), and pendingCount={0} (or the
actual count from context) so the search bar logic and types are satisfied
before returning this UI.

@nbsp1221 nbsp1221 merged commit 2f1499e into main Oct 6, 2025
5 checks passed
@nbsp1221 nbsp1221 deleted the ux/private-playlist-error-ux branch October 6, 2025 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments