feat(library): open genre detail page from tags grid#23
Merged
Conversation
genre tiles in the library grid were styled clickable but had no handler. add a spotify-style detail view (header + play/shuffle + track table) backed by a new `get_genre_detail` command, plumb the navigation through ViewId, and translate the new keys across the 17 locales.
📝 WalkthroughWalkthroughThis PR implements genre detail browsing, enabling users to click a genre tile in the Library view to navigate to a dedicated page showing all tracks for that genre. The feature spans backend query logic, frontend routing, component rendering, and internationalized UI strings across 16 locales. ChangesGenre Detail Browsing Feature
Sequence DiagramsequenceDiagram
participant User
participant AppLayout
participant LibraryView
participant GenreDetailView
participant Backend
User->>LibraryView: Click genre tile
LibraryView->>AppLayout: onNavigateToGenre(genreId)
AppLayout->>AppLayout: Set activeGenreId, push genre-detail view
AppLayout->>GenreDetailView: Render with genreId
GenreDetailView->>Backend: getGenreDetail(genreId)
Backend-->>GenreDetailView: GenreDetail{name, tracks, duration}
GenreDetailView->>GenreDetailView: Render genre header and track table
User->>GenreDetailView: Double-click track or click play
GenreDetailView->>Backend: Execute playback action
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
scope: frontend
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@src/components/layout/AppLayout.tsx`:
- Around line 239-245: The navigation currently only pushes the view id into
history, leaving activeGenreId out of the history payload; update the
navigateToGenre callback to push a history entry that includes both the view id
and the genre id (e.g. { id: "genre-detail", genreId }) instead of just
"genre-detail", and remove reliance on external activeGenreId when rendering by
reading the genreId from the history entry payload to set the shown genre; also
apply the same pattern to the similar handler referenced around lines 351-358 so
back/forward restores the correct detail entry.
In `@src/components/views/GenreDetailView.tsx`:
- Around line 223-377: GenreTrackTable currently renders all rows with
tracks.map (no virtualization) — replace the full mapping with a virtualized
list using `@tanstack/react-virtual` and consume the shared scroll element from
usePageScroll() so the table participates in the global page scroller.
Concretely: in GenreTrackTable import and use useVirtualizer (or
createVirtualizer) from `@tanstack/react-virtual`, call usePageScroll() to get the
scrollElement and pass it as the parentRef/scrollElement to the virtualizer,
compute virtualItems from tracks.length, and render only virtualItems instead of
tracks.map; keep existing row behavior (onDoubleClick -> onPlayTrack(index),
onContextMenu -> onContextMenuRow, onToggleLike, onNavigateToAlbum/Artist,
PlayingIndicator, Artwork, formatDuration, likedIds checks) and ensure each
virtual row key still uniquely references track.id and index and that container
height/style matches prior grid layout so measuring stays correct.
- Around line 90-98: The handleToggleLike handler currently awaits
toggleLikeTrack(trackId) with no error handling which can cause unhandled
rejections and leave setLikedIds out-of-sync; update handleToggleLike to wrap
the await in try/catch, perform the optimistic update only after success (or do
an optimistic update but revert on error), call setLikedIds inside the success
path, and log or surface the error (using console.error or an existing UI error
handler) so failures from toggleLikeTrack are caught and the UI remains
consistent.
- Around line 120-124: The Shuffle button currently calls toggleShuffle() inside
handleShufflePlay which will disable shuffle if it is already on; change
handleShufflePlay to ensure shuffle is explicitly enabled instead of toggled:
use the player state from usePlayer (e.g., the current shuffle boolean) and
either call a dedicated setShuffle(true) API if available or gate
toggleShuffle() behind a check (only call toggleShuffle() when shuffle is false)
after invoking playTracks(tracks, 0, { type: "library", id: null }); ensure you
reference handleShufflePlay and toggleShuffle (and the shuffle state from
usePlayer) when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 09599abb-4abf-4a1d-84f7-024727a82cf7
📒 Files selected for processing (25)
docs/features/library.mdsrc-tauri/src/commands/browse.rssrc-tauri/src/lib.rssrc/components/layout/AppLayout.tsxsrc/components/views/GenreDetailView.tsxsrc/components/views/LibraryView.tsxsrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/id.jsonsrc/i18n/locales/it.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/kr.jsonsrc/i18n/locales/nl.jsonsrc/i18n/locales/pt-BR.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/ru.jsonsrc/i18n/locales/tr.jsonsrc/i18n/locales/zh-CN.jsonsrc/i18n/locales/zh-TW.jsonsrc/lib/tauri/detail.tssrc/types/index.ts
Comment on lines
+239
to
+245
| const navigateToGenre = useCallback( | ||
| (genreId: number) => { | ||
| setActiveGenreId(genreId); | ||
| setActiveView("genre-detail"); | ||
| }, | ||
| [setActiveView], | ||
| ); |
There was a problem hiding this comment.
Back/forward cannot restore previously opened genre details.
Line 239 pushes only "genre-detail" into history, while the active id lives outside history (activeGenreId). If users open Genre A then Genre B, Back lands on "genre-detail" but still shows Genre B.
Suggested direction
- const [viewHistory, setViewHistory] = useState<ViewId[]>(["home"]);
+ type ViewState =
+ | { id: "home" }
+ | { id: "library" }
+ | { id: "genre-detail"; genreId: number }
+ | { id: "album-detail"; albumId: number }
+ | { id: "artist-detail"; artistId: number }
+ // ...other views
+ ;
+ const [viewHistory, setViewHistory] = useState<ViewState[]>([{ id: "home" }]);Then push {"id":"genre-detail","genreId"} instead of only "genre-detail", and render from the history entry payload.
Also applies to: 351-358
🤖 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 `@src/components/layout/AppLayout.tsx` around lines 239 - 245, The navigation
currently only pushes the view id into history, leaving activeGenreId out of the
history payload; update the navigateToGenre callback to push a history entry
that includes both the view id and the genre id (e.g. { id: "genre-detail",
genreId }) instead of just "genre-detail", and remove reliance on external
activeGenreId when rendering by reading the genreId from the history entry
payload to set the shown genre; also apply the same pattern to the similar
handler referenced around lines 351-358 so back/forward restores the correct
detail entry.
Comment on lines
+90
to
+98
| const handleToggleLike = async (trackId: number) => { | ||
| const nowLiked = await toggleLikeTrack(trackId); | ||
| setLikedIds((prev) => { | ||
| const next = new Set(prev); | ||
| if (nowLiked) next.add(trackId); | ||
| else next.delete(trackId); | ||
| return next; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Missing error handling for like toggle path.
Line 91 can reject (IPC/backend failure), but the handler has no catch path. That can surface unhandled promise rejections and leave UI state inconsistent.
Suggested fix
- const handleToggleLike = async (trackId: number) => {
- const nowLiked = await toggleLikeTrack(trackId);
- setLikedIds((prev) => {
- const next = new Set(prev);
- if (nowLiked) next.add(trackId);
- else next.delete(trackId);
- return next;
- });
- };
+ const handleToggleLike = async (trackId: number) => {
+ try {
+ const nowLiked = await toggleLikeTrack(trackId);
+ setLikedIds((prev) => {
+ const next = new Set(prev);
+ if (nowLiked) next.add(trackId);
+ else next.delete(trackId);
+ return next;
+ });
+ } catch (err) {
+ console.error("[GenreDetailView] toggle like failed", err);
+ }
+ };🤖 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 `@src/components/views/GenreDetailView.tsx` around lines 90 - 98, The
handleToggleLike handler currently awaits toggleLikeTrack(trackId) with no error
handling which can cause unhandled rejections and leave setLikedIds out-of-sync;
update handleToggleLike to wrap the await in try/catch, perform the optimistic
update only after success (or do an optimistic update but revert on error), call
setLikedIds inside the success path, and log or surface the error (using
console.error or an existing UI error handler) so failures from toggleLikeTrack
are caught and the UI remains consistent.
Comment on lines
+120
to
+124
| const handleShufflePlay = async () => { | ||
| if (tracks.length === 0) return; | ||
| await playTracks(tracks, 0, { type: "library", id: null }); | ||
| await toggleShuffle(); | ||
| }; |
There was a problem hiding this comment.
Shuffle action is toggle-based and can disable shuffle.
Line 123 uses toggleShuffle() after starting playback. If shuffle is already enabled, this button turns it off, which contradicts a deterministic “Shuffle” action.
Suggested fix
- await playTracks(tracks, 0, { type: "library", id: null });
- await toggleShuffle();
+ await playTracks(tracks, 0, { type: "library", id: null });
+ // Ensure shuffle is enabled, do not toggle blindly.
+ await ensureShuffleEnabled();(Or gate toggleShuffle() behind current shuffle state if that state is available in usePlayer.)
🤖 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 `@src/components/views/GenreDetailView.tsx` around lines 120 - 124, The Shuffle
button currently calls toggleShuffle() inside handleShufflePlay which will
disable shuffle if it is already on; change handleShufflePlay to ensure shuffle
is explicitly enabled instead of toggled: use the player state from usePlayer
(e.g., the current shuffle boolean) and either call a dedicated setShuffle(true)
API if available or gate toggleShuffle() behind a check (only call
toggleShuffle() when shuffle is false) after invoking playTracks(tracks, 0, {
type: "library", id: null }); ensure you reference handleShufflePlay and
toggleShuffle (and the shuffle state from usePlayer) when making the change.
Comment on lines
+223
to
+377
| function GenreTrackTable({ | ||
| tracks, | ||
| isLoading, | ||
| currentTrackId, | ||
| isPlaying, | ||
| likedIds, | ||
| onToggleLike, | ||
| onPlayTrack, | ||
| onNavigateToAlbum, | ||
| onNavigateToArtist, | ||
| onContextMenuRow, | ||
| t, | ||
| }: { | ||
| tracks: Track[]; | ||
| isLoading: boolean; | ||
| currentTrackId: number | null; | ||
| isPlaying: boolean; | ||
| likedIds: Set<number>; | ||
| onToggleLike: (trackId: number) => void; | ||
| onPlayTrack: (index: number) => void; | ||
| onNavigateToAlbum: (albumId: number) => void; | ||
| onNavigateToArtist: (artistId: number) => void; | ||
| onContextMenuRow: (event: React.MouseEvent, track: Track) => void; | ||
| t: (key: string, opts?: Record<string, unknown>) => string; | ||
| }) { | ||
| const gridCols = "grid-cols-[3rem_2.75rem_1.5fr_1fr_1fr_5rem_2rem]"; | ||
| return ( | ||
| <div className="rounded-2xl border border-zinc-200 bg-white dark:border-zinc-800 dark:bg-zinc-800/40 overflow-hidden"> | ||
| <div | ||
| className={`grid ${gridCols} gap-4 px-5 py-3 text-[10px] font-bold tracking-widest text-zinc-400 uppercase border-b border-zinc-100 dark:border-zinc-800`} | ||
| > | ||
| <span className="text-right">{t("library.table.number")}</span> | ||
| <span aria-hidden="true" /> | ||
| <span>{t("library.table.title")}</span> | ||
| <span>{t("library.table.artist")}</span> | ||
| <span>{t("library.table.album")}</span> | ||
| <span | ||
| className="flex justify-end" | ||
| aria-label={t("library.table.duration")} | ||
| > | ||
| <Clock size={14} /> | ||
| </span> | ||
| <span aria-hidden="true" /> | ||
| </div> | ||
|
|
||
| <ul | ||
| className={`divide-y divide-zinc-100 dark:divide-zinc-800/60 ${ | ||
| isLoading ? "opacity-50" : "" | ||
| }`} | ||
| > | ||
| {tracks.map((track, index) => { | ||
| const isCurrent = track.id === currentTrackId; | ||
| return ( | ||
| <li | ||
| key={`${track.id}-${index}`} | ||
| onDoubleClick={() => onPlayTrack(index)} | ||
| onContextMenu={(e) => onContextMenuRow(e, track)} | ||
| className={`grid ${gridCols} gap-4 px-5 py-2 items-center select-none transition-colors cursor-pointer ${ | ||
| isCurrent | ||
| ? "bg-emerald-50 dark:bg-emerald-900/20" | ||
| : "hover:bg-zinc-50 dark:hover:bg-zinc-800/60" | ||
| }`} | ||
| > | ||
| <span | ||
| className={`text-right text-sm tabular-nums flex items-center justify-end ${ | ||
| isCurrent ? "text-emerald-500 font-semibold" : "text-zinc-400" | ||
| }`} | ||
| > | ||
| {isCurrent ? ( | ||
| <PlayingIndicator isPlaying={isPlaying} /> | ||
| ) : ( | ||
| index + 1 | ||
| )} | ||
| </span> | ||
| <Artwork | ||
| path={track.artwork_path} | ||
| path1x={track.artwork_path_1x} | ||
| path2x={track.artwork_path_2x} | ||
| size="1x" | ||
| className="w-10 h-10" | ||
| iconSize={18} | ||
| alt={track.album_title ?? track.title} | ||
| rounded="md" | ||
| /> | ||
| <span | ||
| className={`text-sm truncate flex items-center gap-2 ${ | ||
| isCurrent | ||
| ? "text-emerald-600 dark:text-emerald-400 font-semibold" | ||
| : "text-zinc-800 dark:text-zinc-200" | ||
| }`} | ||
| > | ||
| <span className="truncate">{track.title}</span> | ||
| <HiResBadge | ||
| bitDepth={track.bit_depth} | ||
| sampleRate={track.sample_rate} | ||
| codec={track.codec} | ||
| variant="inline" | ||
| /> | ||
| </span> | ||
| <span className="text-sm text-zinc-500 truncate"> | ||
| <ArtistLink | ||
| name={track.artist_name} | ||
| artistIds={track.artist_ids} | ||
| onNavigate={onNavigateToArtist} | ||
| fallback={t("library.table.unknown")} | ||
| /> | ||
| </span> | ||
| <span className="text-sm text-zinc-500 truncate"> | ||
| {track.album_id != null && track.album_title ? ( | ||
| <button | ||
| type="button" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| onNavigateToAlbum(track.album_id!); | ||
| }} | ||
| className="hover:underline truncate text-left" | ||
| > | ||
| {track.album_title} | ||
| </button> | ||
| ) : ( | ||
| (track.album_title ?? t("library.table.unknown")) | ||
| )} | ||
| </span> | ||
| <span className="text-sm tabular-nums text-zinc-400 text-right"> | ||
| {formatDuration(track.duration_ms)} | ||
| </span> | ||
| <div className="flex justify-center"> | ||
| <button | ||
| type="button" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| onToggleLike(track.id); | ||
| }} | ||
| aria-label={ | ||
| likedIds.has(track.id) ? t("liked.unlike") : t("liked.like") | ||
| } | ||
| className={`p-1 rounded-full transition-colors ${ | ||
| likedIds.has(track.id) | ||
| ? "text-pink-500" | ||
| : "text-zinc-300 dark:text-zinc-600 hover:text-pink-500" | ||
| }`} | ||
| > | ||
| <Heart | ||
| size={14} | ||
| className={likedIds.has(track.id) ? "fill-current" : ""} | ||
| /> | ||
| </button> | ||
| </div> | ||
| </li> | ||
| ); | ||
| })} | ||
| </ul> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Track table is not virtualized.
GenreTrackTable renders the full list (tracks.map) and does not use the shared page scroller. This is a performance/compliance gap for large genres.
As per coding guidelines, src/**/*{Table,List,View}.{ts,tsx} requires: "Virtual scroll must be used everywhere for performance — TrackTable uses @tanstack/react-virtual and virtualized tables must consume usePageScroll() for the scroll element instead of nesting their own overflow-y-auto".
🤖 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 `@src/components/views/GenreDetailView.tsx` around lines 223 - 377,
GenreTrackTable currently renders all rows with tracks.map (no virtualization) —
replace the full mapping with a virtualized list using `@tanstack/react-virtual`
and consume the shared scroll element from usePageScroll() so the table
participates in the global page scroller. Concretely: in GenreTrackTable import
and use useVirtualizer (or createVirtualizer) from `@tanstack/react-virtual`, call
usePageScroll() to get the scrollElement and pass it as the
parentRef/scrollElement to the virtualizer, compute virtualItems from
tracks.length, and render only virtualItems instead of tracks.map; keep existing
row behavior (onDoubleClick -> onPlayTrack(index), onContextMenu ->
onContextMenuRow, onToggleLike, onNavigateToAlbum/Artist, PlayingIndicator,
Artwork, formatDuration, likedIds checks) and ensure each virtual row key still
uniquely references track.id and index and that container height/style matches
prior grid layout so measuring stays correct.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
get_genre_detail(genre_id)command that joinstrack_genreand returns tracks sorted Artist → Album → Disc → Track. Newgenre-detailViewId plumbed through AppLayout's history stack so back/forward navigation works.genreDetaili18n block translated across all 17 locales.Test plan
bun run typecheck✅cargo check --manifest-path src-tauri/Cargo.toml --all-targets✅Summary by CodeRabbit
Release Notes
New Features
Documentation