Skip to content

Conversation

@nbsp1221
Copy link
Owner

@nbsp1221 nbsp1221 commented Oct 13, 2025

Summary

  • add add-to-playlist dialog hooked into library cards and quick view
  • fetch and update playlists for the current user with success feedback
  • document current state and future follow-ups for playlist flow

Testing

  • bun run lint:fix
  • bun run typecheck
  • bun run test
  • bun run build
  • manual QA via Playwright: login as admin, add video to playlist from card menu and quick view, confirm playlist video counts

Summary by CodeRabbit

  • New Features

    • Added "Add to Playlist" dialog and panel to save videos to playlists with create-new, loading/empty/error states, per-item action feedback, retry/refresh, and inline create flow.
    • Added a collapsible "Save to playlist" section in the video player.
    • Video cards now use an actions menu; Quick View moved into the menu.
    • Video player shows a hydration/loading spinner placeholder and improved loading overlays.
  • Documentation

    • Added notes on the Add to Playlist flow, integration points, edge cases, and follow-ups.

@coderabbitai
Copy link

coderabbitai bot commented Oct 13, 2025

Walkthrough

Adds Add-to-Playlist hook, dialog, and panel; integrates a collapsible "Save to playlist" section in the video player; moves VideoCard's Quick view into a dropdown menu; and adds a hydration guard plus LoadingSpinner to the Vidstack player. (49 words)

Changes

Cohort / File(s) Summary
Video card actions refactor
app/entities/video/ui/VideoCard.tsx
Replaces inline Quick View button with a conditional DropdownMenu trigger (MoreVertical) and a "Quick view" menu item (Eye) when onQuickView exists; updates imports to include Eye and dropdown components.
Add-to-playlist model (hook)
app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts
New hook managing playlist loading (GET /api/playlists?limit=100&includeEmpty=true), per-playlist actionStates, add-to-playlist POST (/api/playlists/:id/items), refresh, loading/error handling, and exposes playlists, handleAdd, refresh, isLoading, error, actionStates.
Add-to-playlist UI: dialog & panel
app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx, app/features/playlist/add-to-playlist/ui/AddToPlaylistPanel.tsx
New AddToPlaylistDialog and AddToPlaylistPanel components. Consume the hook, render loading/empty/list states, show per-item Add/Added/Retry actions, support creating a new playlist (CreatePlaylistDialog), and refresh when creation closes.
Player integration (collapsible)
app/widgets/video-player-view/ui/VideoPlayerView.tsx
Adds isPlaylistSectionOpen state and a collapsible "Save to playlist" section with trigger (ChevronUp/ChevronDown); renders AddToPlaylistPanel when open; removes some header actions.
Vidstack player hydration & spinner
app/entities/video/ui/VidstackPlayer.tsx
Adds isHydrated state and effect, introduces LoadingSpinner component and props, early-return placeholder ("Preparing player…") until hydration completes, and uses LoadingSpinner for player loading overlays.
Documentation
docs/playlist-add-to-playlist-notes.md
New notes documenting dialog/panel behavior, integration points, API endpoints used, edge cases, and TODOs for future enhancements.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant VP as VideoPlayerView
  participant P as AddToPlaylistPanel/Dialog
  participant H as useAddToPlaylistDialog
  participant API as /api/playlists & /api/playlists/:id/items

  U->>VP: Open "Save to playlist"
  VP->>P: Render panel/dialog (video)
  P->>H: loadPlaylists()
  H->>API: GET /api/playlists?limit=100&includeEmpty=true
  API-->>H: Playlists (user-owned)
  H-->>P: playlists, isLoading=false

  U->>P: Click "Add" on a playlist
  P->>H: handleAdd(playlistId, video)
  H->>API: POST /api/playlists/:id/items
  alt Success
    API-->>H: 200 OK
    H-->>P: Update containsVideo/videoCount, actionStates=success
  else Error
    API-->>H: 4xx/5xx
    H-->>P: actionStates=error, error message
  end

  note over P,VP: On creating a new playlist, close create dialog -> H.refresh()
Loading
sequenceDiagram
  autonumber
  actor U as User
  participant VC as VideoCard
  participant DM as DropdownMenu

  U->>VC: Hover/Focus card
  U->>DM: Click More (⋮)
  DM-->>U: Show menu
  alt Quick view available
    U->>DM: Click "Quick view"
    DM-->>VC: onQuickView()
  else No action
    DM-->>U: No Quick view item
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

A nibble of code, a playlist to save,
I hop through dialogs, bold and brave.
Menus that shimmer, panels that gleam,
I tuck videos in like a carrot dream. 🥕
Quick views and playlists—hippity-hop, team!

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 succinctly describes the primary addition of an add-to-playlist flow in the library UI and closely aligns with the changeset by focusing on the core feature integration.
✨ 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 feat/add-to-playlist-flow

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0a229a and 8abb63e.

📒 Files selected for processing (1)
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
**/*.{ts,tsx,md,mdx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
app/**

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
app/features/**

📄 CodeRabbit inference engine (AGENTS.md)

Implement workflow logic within app/features/ and keep slice-specific UI under its feature directory

Files:

  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Split multiline JSX props across lines per Stylistic ESLint rules

Files:

  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
🧬 Code graph analysis (1)
app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx (3)
app/types/video.ts (1)
  • Video (1-10)
app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts (1)
  • useAddToPlaylistDialog (57-197)
app/features/playlist/create-playlist/ui/CreatePlaylistDialog.tsx (1)
  • CreatePlaylistDialog (17-54)
🔇 Additional comments (4)
app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx (4)

1-21: LGTM!

Imports are well-organized and the interface follows TypeScript strict mode with explicit types.


30-35: Good UX: refreshing playlists after create.

The callback correctly refreshes the playlist list when the CreatePlaylistDialog closes while the parent dialog remains open, ensuring newly created playlists appear immediately.


37-106: All past concerns addressed.

The implementation correctly handles all states (loading, empty, list) and the previously flagged issues have been resolved:

  • Line 41: Spinner includes aria-hidden="true" for accessibility
  • Line 90: Button is disabled when isBusy || (isAdded && !isErrored) to prevent duplicate POSTs
  • Lines 92-95: onClick defensively checks isBusy before invoking handleAdd, providing additional protection against race conditions
  • Line 98: Button spinner includes aria-hidden="true"

The state derivation logic (lines 65-73) is clear, and the error/retry flow is well-handled.


108-157: Well-structured dialog component.

The component integrates cleanly with both the useAddToPlaylistDialog hook and CreatePlaylistDialog, presents clear error states with retry actions, and follows all coding guidelines (explicit types, ~/ imports, proper naming conventions).


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.

❤️ Share

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

🧹 Nitpick comments (7)
app/widgets/library-view/model/useLibraryView.ts (2)

76-80: LGTM: clean add-to-playlist state and handlers; also close it on delete

Nice, isolated modal state and stable callbacks. One edge case: if a video is deleted while the add-to-playlist dialog is open for that video, also clear that state.

Apply this change inside handleDeleteVideo to close the add-to-playlist dialog when deleting its video, and update deps:

   setVideos(prev => prev.filter(video => video.id !== videoId));

   if (modalState.video?.id === videoId) {
     setModalState({ video: null, isOpen: false });
   }
+  if (addToPlaylistState.video?.id === videoId) {
+    setAddToPlaylistState({ video: null, isOpen: false });
+  }
-}, [modalState.video?.id]);
+}, [modalState.video?.id, addToPlaylistState.video?.id]);

Also applies to: 121-127, 196-206


27-27: Optional: unify modal state to reduce duplication

Consider a discriminated union for a single modal state (e.g., { kind: 'quickView' | 'addToPlaylist'; video; isOpen }) with helpers, to cut duplication and prevent state drift.

app/widgets/library-view/ui/LibraryView.tsx (1)

70-71: Dialog wiring is sound; consider optional UX tweak.

You close only on explicit close; consider auto-closing after a successful add (if desired) for faster flow.

Also applies to: 73-81

app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts (4)

104-106: Avoid logging raw error objects; keep logs free of sensitive details.

Log minimal, non‑PII messages and rely on user‑visible messages for details.

Apply:

-      console.error('Failed to fetch playlists for dialog:', loadError);
+      console.error('Failed to fetch playlists for dialog');
...
-      console.error('Failed to add video to playlist:', addError);
+      console.error('Failed to add video to playlist');

As per coding guidelines.

Also applies to: 160-163


71-110: Optional: add AbortController to cancel in‑flight loads on close/change.

Prevents late state updates after closing and saves network.

Sketch:

  • Inside loadPlaylists, accept an AbortSignal and pass { signal } to fetch.
  • In the effect, create const ac = new AbortController(), call loadPlaylists(ac.signal), and return () => ac.abort().

No behavior change when not closing mid‑request.


31-38: Type API responses for safer parsing.

Define interfaces for /api/playlists and add‑item responses; narrow data shape.

Example:

interface ListPlaylistsResponse { success: boolean; playlists: RawPlaylist[]; error?: string }
interface AddItemResponse { success: boolean; error?: string }

Then cast await response.json() as ListPlaylistsResponse.
As per coding guidelines.

Also applies to: 84-92, 142-146


170-176: Redundant normalization pass.

normalizedPlaylists clones without changes. Return playlists directly.

Apply:

-  const normalizedPlaylists = useMemo(() => {
-    if (!video) return [] as PlaylistSummary[];
-    return playlists.map(playlist => ({
-      ...playlist,
-      containsVideo: playlist.containsVideo,
-    }));
-  }, [playlists, video]);
+  const normalizedPlaylists = useMemo(
+    () => (video ? playlists : [] as PlaylistSummary[]),
+    [playlists, video]
+  );
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 92fa308 and ee5e12f.

📒 Files selected for processing (9)
  • app/entities/video/ui/VideoCard.tsx (3 hunks)
  • app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts (1 hunks)
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx (1 hunks)
  • app/pages/home/ui/HomePage.tsx (1 hunks)
  • app/widgets/library-view/model/useLibraryView.ts (4 hunks)
  • app/widgets/library-view/ui/LibraryView.tsx (5 hunks)
  • app/widgets/library-view/ui/VideoGrid.tsx (2 hunks)
  • app/widgets/library-view/ui/VideoModal.tsx (3 hunks)
  • docs/playlist-add-to-playlist-notes.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{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/widgets/library-view/ui/VideoModal.tsx
  • app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts
  • app/entities/video/ui/VideoCard.tsx
  • app/widgets/library-view/ui/LibraryView.tsx
  • app/widgets/library-view/model/useLibraryView.ts
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
  • app/pages/home/ui/HomePage.tsx
  • app/widgets/library-view/ui/VideoGrid.tsx
**/*.{ts,tsx,md,mdx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • app/widgets/library-view/ui/VideoModal.tsx
  • app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts
  • app/entities/video/ui/VideoCard.tsx
  • docs/playlist-add-to-playlist-notes.md
  • app/widgets/library-view/ui/LibraryView.tsx
  • app/widgets/library-view/model/useLibraryView.ts
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
  • app/pages/home/ui/HomePage.tsx
  • app/widgets/library-view/ui/VideoGrid.tsx
app/**

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • app/widgets/library-view/ui/VideoModal.tsx
  • app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts
  • app/entities/video/ui/VideoCard.tsx
  • app/widgets/library-view/ui/LibraryView.tsx
  • app/widgets/library-view/model/useLibraryView.ts
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
  • app/pages/home/ui/HomePage.tsx
  • app/widgets/library-view/ui/VideoGrid.tsx
app/widgets/**

📄 CodeRabbit inference engine (AGENTS.md)

Put UI compositions in app/widgets/

Files:

  • app/widgets/library-view/ui/VideoModal.tsx
  • app/widgets/library-view/ui/LibraryView.tsx
  • app/widgets/library-view/model/useLibraryView.ts
  • app/widgets/library-view/ui/VideoGrid.tsx
**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Split multiline JSX props across lines per Stylistic ESLint rules

Files:

  • app/widgets/library-view/ui/VideoModal.tsx
  • app/entities/video/ui/VideoCard.tsx
  • app/widgets/library-view/ui/LibraryView.tsx
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
  • app/pages/home/ui/HomePage.tsx
  • app/widgets/library-view/ui/VideoGrid.tsx
app/features/**

📄 CodeRabbit inference engine (AGENTS.md)

Implement workflow logic within app/features/ and keep slice-specific UI under its feature directory

Files:

  • app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
app/entities/**

📄 CodeRabbit inference engine (AGENTS.md)

Define domain models in app/entities/

Files:

  • app/entities/video/ui/VideoCard.tsx
docs/**

📄 CodeRabbit inference engine (AGENTS.md)

Put architectural notes and investigations in docs/

Files:

  • docs/playlist-add-to-playlist-notes.md
app/pages/**

📄 CodeRabbit inference engine (AGENTS.md)

app/pages/**: Keep route shells in app/pages/ (pages expose route shells only)
Keep routes thin in app/pages/ (delegate business logic to modules/hooks)

Files:

  • app/pages/home/ui/HomePage.tsx
🧬 Code graph analysis (7)
app/widgets/library-view/ui/VideoModal.tsx (1)
app/types/video.ts (1)
  • Video (1-10)
app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts (2)
app/types/video.ts (1)
  • Video (1-10)
app/stores/auth-store.ts (1)
  • useAuthUser (101-101)
app/entities/video/ui/VideoCard.tsx (1)
app/types/video.ts (1)
  • Video (1-10)
app/widgets/library-view/ui/LibraryView.tsx (3)
app/widgets/library-view/model/useLibraryView.ts (1)
  • ModalState (16-19)
app/types/video.ts (1)
  • Video (1-10)
app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx (1)
  • AddToPlaylistDialog (23-153)
app/widgets/library-view/model/useLibraryView.ts (1)
app/types/video.ts (1)
  • Video (1-10)
app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx (3)
app/types/video.ts (1)
  • Video (1-10)
app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts (1)
  • useAddToPlaylistDialog (57-186)
app/features/playlist/create-playlist/ui/CreatePlaylistDialog.tsx (1)
  • CreatePlaylistDialog (17-54)
app/widgets/library-view/ui/VideoGrid.tsx (1)
app/types/video.ts (1)
  • Video (1-10)
🔇 Additional comments (7)
docs/playlist-add-to-playlist-notes.md (1)

5-18: Documentation reads well and matches the implementation scope

Clear status and actionable follow-ups. No blockers.

app/widgets/library-view/ui/VideoModal.tsx (1)

26-27: LGTM: add-to-playlist action integrates cleanly

Prop threading and the button behavior are correct (invoke callback, then close). No issues spotted.

Also applies to: 181-194

app/pages/home/ui/HomePage.tsx (1)

33-35: LGTM: new props correctly wired to LibraryView

Prop names and types align with useLibraryView’s public API.

app/widgets/library-view/ui/LibraryView.tsx (1)

16-18: Public API extension looks consistent.

Props are typed and threaded to children as expected.

Also applies to: 32-35, 59-60

app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx (1)

38-46: UI states and error UX look solid.

Loading/empty/error states are handled cleanly with clear actions.

Also applies to: 130-141

app/widgets/library-view/ui/VideoGrid.tsx (1)

8-8: LGTM: Prop plumbed through cleanly; VideoCard’s onAddToPlaylist is optional and wired correctly.

app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts (1)

82-92: Add CSRF token header to cookie-based fetch requests. You’re using credentials: 'include' here, but there’s no evidence of a global fetch wrapper or header injection—ensure a valid anti-CSRF token header is added (either via a shared fetch utility or directly in this call).

Comment on lines +112 to +119
useEffect(() => {
if (open) {
void loadPlaylists();
}
else {
resetState();
}
}, [open, loadPlaylists, resetState]);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Two effects can double‑fetch when opening the dialog.

Both effects fire on open=true. Consolidate to one effect that loads when open && video, otherwise resets.

Apply:

-  useEffect(() => {
-    if (open) {
-      void loadPlaylists();
-    }
-    else {
-      resetState();
-    }
-  }, [open, loadPlaylists, resetState]);
-
-  useEffect(() => {
-    if (open && video) {
-      void loadPlaylists();
-    }
-  }, [open, video?.id, loadPlaylists]);
+  useEffect(() => {
+    if (open && video) {
+      void loadPlaylists();
+    } else if (!open) {
+      resetState();
+    }
+  }, [open, video?.id, loadPlaylists, resetState]);

Also applies to: 121-125

🤖 Prompt for AI Agents
In app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts around
lines 112-119 (and similarly for lines 121-125), two separate useEffect hooks
can both run when open becomes true and cause duplicate loads; consolidate them
into a single useEffect that depends on [open, video, loadPlaylists, resetState]
and runs loadPlaylists only when open && video, otherwise calls resetState —
ensure async call is properly voided or handled and remove the redundant effect
so only this consolidated effect triggers fetching/resetting.

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 (1)
app/widgets/video-player-view/ui/VideoPlayerView.tsx (1)

93-95: Remove redundant conditional render.

The conditional {isPlaylistSectionOpen ? ... : null} is unnecessary because CollapsibleContent already handles visibility based on the parent Collapsible's open prop. The panel will be unmounted automatically when collapsed.

Apply this diff to simplify:

 <CollapsibleContent className="mt-3">
-  {isPlaylistSectionOpen ? (
-    <AddToPlaylistPanel video={video} open={isPlaylistSectionOpen} />
-  ) : null}
+  <AddToPlaylistPanel video={video} open={isPlaylistSectionOpen} />
 </CollapsibleContent>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee5e12f and 84ffb7a.

📒 Files selected for processing (3)
  • app/entities/video/ui/VideoCard.tsx (3 hunks)
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistPanel.tsx (1 hunks)
  • app/widgets/video-player-view/ui/VideoPlayerView.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/entities/video/ui/VideoCard.tsx
🧰 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/widgets/video-player-view/ui/VideoPlayerView.tsx
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistPanel.tsx
**/*.{ts,tsx,md,mdx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • app/widgets/video-player-view/ui/VideoPlayerView.tsx
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistPanel.tsx
app/**

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • app/widgets/video-player-view/ui/VideoPlayerView.tsx
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistPanel.tsx
app/widgets/**

📄 CodeRabbit inference engine (AGENTS.md)

Put UI compositions in app/widgets/

Files:

  • app/widgets/video-player-view/ui/VideoPlayerView.tsx
**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Split multiline JSX props across lines per Stylistic ESLint rules

Files:

  • app/widgets/video-player-view/ui/VideoPlayerView.tsx
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistPanel.tsx
app/features/**

📄 CodeRabbit inference engine (AGENTS.md)

Implement workflow logic within app/features/ and keep slice-specific UI under its feature directory

Files:

  • app/features/playlist/add-to-playlist/ui/AddToPlaylistPanel.tsx
🧬 Code graph analysis (2)
app/widgets/video-player-view/ui/VideoPlayerView.tsx (1)
app/features/playlist/add-to-playlist/ui/AddToPlaylistPanel.tsx (1)
  • AddToPlaylistPanel (15-167)
app/features/playlist/add-to-playlist/ui/AddToPlaylistPanel.tsx (3)
app/types/video.ts (1)
  • Video (1-10)
app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts (1)
  • useAddToPlaylistDialog (57-186)
app/features/playlist/create-playlist/ui/CreatePlaylistDialog.tsx (1)
  • CreatePlaylistDialog (17-54)
🔇 Additional comments (7)
app/widgets/video-player-view/ui/VideoPlayerView.tsx (3)

1-9: LGTM!

The new imports are correctly used for the collapsible playlist feature.


29-29: LGTM!

State initialization is correct for managing collapsible visibility.


82-89: Verify Share and Download button implementations.

The Share and Download buttons lack onClick handlers. If these are intentional placeholders for future functionality, consider adding a TODO comment or disabling them to clarify intent.

app/features/playlist/add-to-playlist/ui/AddToPlaylistPanel.tsx (4)

1-13: LGTM!

Imports and interface definition are clean and well-typed.


15-32: LGTM!

The component setup correctly manages local dialog state and integrates with the playlist hook. The refresh logic appropriately triggers only when the create dialog closes while the panel remains open.


34-112: LGTM!

The renderPlaylists function correctly handles loading, empty, and populated states. The per-playlist action button logic properly reflects the state machine (idle → loading → success/error) and provides clear visual feedback.


114-167: LGTM!

The main render properly structures the panel with a clear header, conditional error handling with retry capability, and integrated playlist creation dialog. The error feedback and retry mechanism provide a good user experience.

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 (1)
app/entities/video/ui/VidstackPlayer.tsx (1)

53-53: Extract duplicated loading spinner into shared component The hydration gate is required by SSR (configured via reactRouterHonoServer in vite.config.ts), but the spinner markup in the gate (lines 239–243) and existing overlay (lines 262–265) is duplicated—extract into a shared <LoadingSpinner>.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 84ffb7a and d8f2408.

📒 Files selected for processing (1)
  • app/entities/video/ui/VidstackPlayer.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/entities/video/ui/VidstackPlayer.tsx
**/*.{ts,tsx,md,mdx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • app/entities/video/ui/VidstackPlayer.tsx
app/**

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • app/entities/video/ui/VidstackPlayer.tsx
app/entities/**

📄 CodeRabbit inference engine (AGENTS.md)

Define domain models in app/entities/

Files:

  • app/entities/video/ui/VidstackPlayer.tsx
**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Split multiline JSX props across lines per Stylistic ESLint rules

Files:

  • app/entities/video/ui/VidstackPlayer.tsx

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/entities/video/ui/VideoCard.tsx (1)

21-21: Add onAddToPlaylist support to VideoCard
The VideoCardProps interface (app/entities/video/ui/VideoCard.tsx) needs an optional onAddToPlaylist handler, and hasActions (line 33) should include onAddToPlaylist alongside onQuickView. Update VideoGrid (app/widgets/library-view/ui/VideoGrid.tsx) to pass the new prop.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b9a34f5 and f0a229a.

📒 Files selected for processing (3)
  • app/entities/video/ui/VideoCard.tsx (3 hunks)
  • app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts (1 hunks)
  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts
🧰 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/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
  • app/entities/video/ui/VideoCard.tsx
**/*.{ts,tsx,md,mdx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
  • app/entities/video/ui/VideoCard.tsx
app/**

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
  • app/entities/video/ui/VideoCard.tsx
app/features/**

📄 CodeRabbit inference engine (AGENTS.md)

Implement workflow logic within app/features/ and keep slice-specific UI under its feature directory

Files:

  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Split multiline JSX props across lines per Stylistic ESLint rules

Files:

  • app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx
  • app/entities/video/ui/VideoCard.tsx
app/entities/**

📄 CodeRabbit inference engine (AGENTS.md)

Define domain models in app/entities/

Files:

  • app/entities/video/ui/VideoCard.tsx
🧬 Code graph analysis (1)
app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx (3)
app/types/video.ts (1)
  • Video (1-10)
app/features/playlist/add-to-playlist/model/useAddToPlaylistDialog.ts (1)
  • useAddToPlaylistDialog (57-197)
app/features/playlist/create-playlist/ui/CreatePlaylistDialog.tsx (1)
  • CreatePlaylistDialog (17-54)
🔇 Additional comments (9)
app/entities/video/ui/VideoCard.tsx (2)

1-1: LGTM! Imports support the new dropdown menu functionality.

The new imports (Eye, MoreVertical icons and DropdownMenu components) are used appropriately to implement the actions menu that replaces the inline Quick View button.

Also applies to: 7-12


73-96: Excellent fix! Accessibility issue from past review has been resolved.

The dropdown menu is now correctly positioned outside the Link element (after line 71), resolving the nested interactive control issue flagged in the previous review. All recommended accessibility improvements have been implemented:

  • ✅ aria-label on the trigger button (line 80)
  • ✅ group-focus-within:opacity-100 for keyboard navigation visibility (line 74)
  • ✅ pointer-events-none/auto pattern to prevent blocking link interactions (lines 74, 81)

This ensures proper keyboard navigation and screen reader interaction.

app/features/playlist/add-to-playlist/ui/AddToPlaylistDialog.tsx (7)

1-15: LGTM!

Imports are well-organized and consistently use the ~/ path alias as per coding guidelines.


17-21: LGTM!

The interface is explicit and follows TypeScript strict mode guidelines.


23-28: LGTM!

Component setup correctly integrates with the useAddToPlaylistDialog hook.


30-35: LGTM!

The callback correctly refreshes the playlist list when the create dialog closes, ensuring newly created playlists appear immediately.


47-60: LGTM!

The empty state provides clear guidance and a direct action for users to create their first playlist.


62-105: LGTM!

The playlist list implementation is solid. All concerns from the previous review have been addressed:

  • Button is disabled while busy to prevent duplicate POSTs ✓
  • onClick handler guards against busy state ✓
  • Spinner has aria-hidden="true"

The per-item state management and UI feedback (button variants, labels, and disabled states) are implemented correctly.


108-157: LGTM!

The main render structure is well-organized:

  • Clear dialog header with contextual description
  • Error handling with retry action
  • "New playlist" action remains accessible even during errors
  • Proper integration with the CreatePlaylistDialog sub-dialog

@nbsp1221 nbsp1221 merged commit 183efc5 into main Dec 28, 2025
5 checks passed
@nbsp1221 nbsp1221 deleted the feat/add-to-playlist-flow branch December 28, 2025 19:14
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.

2 participants