Skip to content

🚸 Polish route error experience with shared alert layout#57

Merged
nbsp1221 merged 2 commits intomainfrom
ux/route-error-view-rollout
Oct 12, 2025
Merged

🚸 Polish route error experience with shared alert layout#57
nbsp1221 merged 2 commits intomainfrom
ux/route-error-view-rollout

Conversation

@nbsp1221
Copy link
Owner

@nbsp1221 nbsp1221 commented Oct 7, 2025

Summary

  • replace the ad-hoc playlist error markup with a shared RouteErrorView component
  • enhance the RouteErrorView with tone-aware styling, consistent icon sizing, and updated English copy
  • opt-in to the shared error UX for playlist detail/index and player routes to keep layout and actions consistent

Testing

  • bun run lint:fix
  • bun run typecheck
  • bun run build
  • bun run test
  • manual: Playwright flow (login, private playlist, missing video)

Summary by CodeRabbit

  • New Features

    • New reusable error screen component for consistent, configurable error pages with title, description, tone, icon, and action buttons.
    • Route-level error screens on Player and Playlists routes with tailored messaging for 400, 403, 404, and other errors.
  • Refactor

    • Playlists and related routes switched from alert-based layouts to the unified error experience for consistency and smoother navigation.

@coderabbitai
Copy link

coderabbitai bot commented Oct 7, 2025

Walkthrough

Adds a new reusable RouteErrorView component and replaces/introduces route-level ErrorBoundary handlers in three routes to render standardized error UIs (mapping 400/403/404/other errors to tones, icons, and actions). No loader or core business logic changed; edits are limited to error rendering and imports.

Changes

Cohort / File(s) Summary of changes
UI: Route error view component
app/components/RouteErrorView.tsx
New exported RouteErrorView component and exported interfaces RouteErrorAction and RouteErrorViewProps. Supports configurable tone (neutral/warning/critical), optional icon, default action resolution, layout modes (standalone or AppLayout-wrapped), children and footnote rendering, and button/link actions.
Routes: player error boundary
app/routes/player.$id.tsx
Added export function ErrorBoundary() that uses useRouteError/isRouteErrorResponse and returns RouteErrorView with status-specific tones/icons/actions (handles 400/404 and generic errors).
Routes: playlists detail error handling
app/routes/playlists.$id.tsx
Rewrote error rendering to use RouteErrorView throughout: replaced prior AppLayout/Alert UI with RouteErrorView in both ErrorBoundary and render paths; added lucide-react icons and removed legacy error UI imports.
Routes: playlists index error boundary
app/routes/playlists._index.tsx
Added export function ErrorBoundary() to render RouteErrorView for route errors (warning vs critical tones), using useRouteError/isRouteErrorResponse and lucide-react icons; supplies navigation actions.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Router as React Router
  participant Route as Route Module
  participant EB as ErrorBoundary
  participant REV as RouteErrorView

  User->>Router: navigate to route
  Router->>Route: run loader/action
  alt loader/action throws or returns error
    Router->>EB: invoke ErrorBoundary
    EB->>EB: useRouteError() / isRouteErrorResponse()
    alt 404
      EB->>REV: render (tone=neutral, icon=FileWarning, actions=[Library, Playlists])
    else 400/403
      EB->>REV: render (tone=warning, icon=ShieldAlert/Lock, actions=[Refresh/Library])
    else other
      EB->>REV: render (tone=critical, icon=AlertTriangle, actions=[Home/Retry])
    end
    REV-->>User: standardized error UI
  else no error
    Route-->>User: normal route UI
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

I’m a rabbit with an icon bright,
I hop in when routes lose light.
With tones and links I stitch the way,
Click Home or Retry and be on your day. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% 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 succinctly conveys the primary change of polishing route error handling using a shared alert layout by introducing the new component and aligning the UX across routes, which matches the changeset’s objectives without listing file details or unrelated noise.
✨ 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/route-error-view-rollout

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 2f1499e and 45ce3b3.

📒 Files selected for processing (4)
  • app/components/RouteErrorView.tsx (1 hunks)
  • app/routes/player.$id.tsx (2 hunks)
  • app/routes/playlists.$id.tsx (2 hunks)
  • app/routes/playlists._index.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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._index.tsx
  • app/routes/player.$id.tsx
  • app/components/RouteErrorView.tsx
  • app/routes/playlists.$id.tsx
**/*.{ts,tsx,md,mdx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • app/routes/playlists._index.tsx
  • app/routes/player.$id.tsx
  • app/components/RouteErrorView.tsx
  • 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._index.tsx
  • app/routes/player.$id.tsx
  • app/components/RouteErrorView.tsx
  • 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._index.tsx
  • app/routes/player.$id.tsx
  • app/components/RouteErrorView.tsx
  • app/routes/playlists.$id.tsx
app/components/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

app/components/**/*.tsx: React 컴포넌트에서는 비즈니스 로직을 금지하고 커스텀 훅과 프리젠테이션만 담당한다 (직접 API 호출 금지)
UI 텍스트는 영어를 사용한다 (향후 로컬라이즈 가능)

Files:

  • app/components/RouteErrorView.tsx
app/components/**

📄 CodeRabbit inference engine (AGENTS.md)

Place reusable UI primitives in app/components/

Files:

  • app/components/RouteErrorView.tsx
🧬 Code graph analysis (4)
app/routes/playlists._index.tsx (3)
app/routes/player.$id.tsx (1)
  • ErrorBoundary (93-143)
app/routes/playlists.$id.tsx (1)
  • ErrorBoundary (84-139)
app/components/RouteErrorView.tsx (1)
  • RouteErrorView (56-140)
app/routes/player.$id.tsx (2)
app/routes/playlists.$id.tsx (1)
  • ErrorBoundary (84-139)
app/components/RouteErrorView.tsx (1)
  • RouteErrorView (56-140)
app/components/RouteErrorView.tsx (2)
app/lib/utils.ts (1)
  • cn (4-6)
app/components/AppLayout.tsx (1)
  • AppLayout (30-42)
app/routes/playlists.$id.tsx (1)
app/components/RouteErrorView.tsx (1)
  • RouteErrorView (56-140)

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: 0

🧹 Nitpick comments (2)
app/routes/playlists._index.tsx (2)

64-64: Add explicit return type annotation for strict TS.

As per coding guidelines, all functions should have comprehensive type hints in strict mode. Add an explicit return type annotation for clarity and type safety.

As per coding guidelines.

Apply this diff:

-export function ErrorBoundary() {
+export function ErrorBoundary(): React.ReactElement {

71-71: Consider using a conventional error state icon for consistency.

RefreshCw typically represents an action (refresh/retry) rather than an error state. Other routes in this PR use state indicators like AlertTriangle, Lock, or FileWarning. For visual consistency, consider using AlertCircle or AlertTriangle here instead.

Example:

-        icon={<RefreshCw className="h-6 w-6" aria-hidden />}
+        icon={<AlertCircle className="h-6 w-6" aria-hidden />}

Don't forget to import AlertCircle if you make this change.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 45ce3b3 and d4d3c13.

📒 Files selected for processing (1)
  • app/routes/playlists._index.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._index.tsx
**/*.{ts,tsx,md,mdx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • app/routes/playlists._index.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._index.tsx
**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Split multiline JSX props across lines per Stylistic ESLint rules

Files:

  • app/routes/playlists._index.tsx
🧬 Code graph analysis (1)
app/routes/playlists._index.tsx (2)
app/routes/playlists.$id.tsx (1)
  • ErrorBoundary (84-139)
app/components/RouteErrorView.tsx (1)
  • RouteErrorView (56-140)
🔇 Additional comments (3)
app/routes/playlists._index.tsx (3)

2-5: LGTM!

The imports follow best practices: per-icon imports from lucide-react for tree-shaking, proper React Router error handling utilities, and the path alias for internal imports.


73-77: LGTM! The error.data guard is correctly implemented.

The typeof error.data === 'string' guard properly addresses the previous review concern about non-string data breaking the React child rendering. The fallback to a generic message when data is not a string is a safe approach.


91-95: LGTM! Proper error instance checking.

The error instanceof Error check correctly handles both Error instances and other thrown values, with an appropriate fallback message. This follows the same pattern as other routes in the codebase.

@nbsp1221 nbsp1221 merged commit 92fa308 into main Oct 12, 2025
5 checks passed
@nbsp1221 nbsp1221 deleted the ux/route-error-view-rollout branch October 12, 2025 14:22
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