From ff99b10faa071e7328b2b05f53aa61c962c42254 Mon Sep 17 00:00:00 2001 From: Frank Stellmacher <171614930+Psychotoxical@users.noreply.github.com> Date: Thu, 14 May 2026 01:19:58 +0200 Subject: [PATCH] =?UTF-8?q?refactor(album-detail):=20I.7=20=E2=80=94=20spl?= =?UTF-8?q?it=20AlbumDetail.tsx=20511=20=E2=86=92=20369=20LOC=20across=205?= =?UTF-8?q?=20files=20(#679)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(album-detail): extract sanitizeFilename + useAlbumDetailData Move sanitizeFilename into utils/albumDetailHelpers.ts. Pull the album + related-albums fetch and the starred state seeds (isStarred, starredSongs) into hooks/useAlbumDetailData.ts. AlbumDetail.tsx: 511 → 483 LOC. * refactor(album-detail): extract useAlbumOfflineState hook Move the four primitive-selector offline status reads (cache map + job-status filters + progress totals) into hooks/useAlbumOfflineState.ts. Keeps the re-render minimisation comment with the code that needs it. AlbumDetail.tsx: 483 → 461 LOC. * refactor(album-detail): extract useAlbumDetailSort hook Pull sortKey/Dir/clickCount state, the 3-click natural-reset cycle in handleSort, and the displayedSongs memo (filter + sort comparator) into hooks/useAlbumDetailSort.ts. Rating comparator keeps the same priority chain as the row renderer. AlbumDetail.tsx: 461 → 414 LOC. * refactor(album-detail): extract AlbumDetailToolbar subcomponent Pull the search input + bulk-action cluster (selection count, add-to- playlist popover, clear-selection button) into components/albumDetail/AlbumDetailToolbar.tsx. Parent retains showPlPicker to coordinate the popover close with selection clears. AlbumDetail.tsx: 414 → 369 LOC. --- .../albumDetail/AlbumDetailToolbar.tsx | 90 ++++++++ src/hooks/useAlbumDetailData.ts | 58 +++++ src/hooks/useAlbumDetailSort.ts | 95 +++++++++ src/hooks/useAlbumOfflineState.ts | 51 +++++ src/pages/AlbumDetail.tsx | 198 +++--------------- src/utils/albumDetailHelpers.ts | 15 ++ 6 files changed, 337 insertions(+), 170 deletions(-) create mode 100644 src/components/albumDetail/AlbumDetailToolbar.tsx create mode 100644 src/hooks/useAlbumDetailData.ts create mode 100644 src/hooks/useAlbumDetailSort.ts create mode 100644 src/hooks/useAlbumOfflineState.ts create mode 100644 src/utils/albumDetailHelpers.ts diff --git a/src/components/albumDetail/AlbumDetailToolbar.tsx b/src/components/albumDetail/AlbumDetailToolbar.tsx new file mode 100644 index 00000000..f0d39c59 --- /dev/null +++ b/src/components/albumDetail/AlbumDetailToolbar.tsx @@ -0,0 +1,90 @@ +import React from 'react'; +import { ListPlus, Search, X } from 'lucide-react'; +import type { TFunction } from 'i18next'; +import { useSelectionStore } from '../../store/selectionStore'; +import { AddToPlaylistSubmenu } from '../ContextMenu'; + +interface Props { + filterText: string; + setFilterText: (v: string) => void; + inSelectMode: boolean; + selectedCount: number; + showPlPicker: boolean; + setShowPlPicker: React.Dispatch>; + t: TFunction; +} + +/** + * Toolbar above the album tracklist. The filter input narrows the + * visible songs by title/artist; the bulk-action cluster only appears + * while the global selection store has at least one item picked. + * + * The "Add to playlist" picker is a portal-aware popover (closes on + * outside mousedown, see AlbumDetail page effect) so the parent owns + * `showPlPicker` to coordinate that close with selection clears. + */ +export function AlbumDetailToolbar({ + filterText, + setFilterText, + inSelectMode, + selectedCount, + showPlPicker, + setShowPlPicker, + t, +}: Props) { + return ( +
+
+ + setFilterText(e.target.value)} + /> + {filterText && ( + + )} +
+
+ {inSelectMode && ( + <> + + {t('common.bulkSelected', { count: selectedCount })} + +
+ + {showPlPicker && ( + { setShowPlPicker(false); useSelectionStore.getState().clearAll(); }} + dropDown + /> + )} +
+ + + )} +
+
+ ); +} diff --git a/src/hooks/useAlbumDetailData.ts b/src/hooks/useAlbumDetailData.ts new file mode 100644 index 00000000..6cc800a3 --- /dev/null +++ b/src/hooks/useAlbumDetailData.ts @@ -0,0 +1,58 @@ +import { useEffect, useState } from 'react'; +import { getAlbum } from '../api/subsonicLibrary'; +import { getArtist } from '../api/subsonicArtists'; +import type { SubsonicAlbum } from '../api/subsonicTypes'; + +type AlbumPayload = Awaited>; + +interface UseAlbumDetailDataResult { + album: AlbumPayload | null; + setAlbum: React.Dispatch>; + relatedAlbums: SubsonicAlbum[]; + loading: boolean; + isStarred: boolean; + setIsStarred: (v: boolean) => void; + starredSongs: Set; + setStarredSongs: React.Dispatch>>; +} + +/** + * Load an album payload by id, then resolve the artist's other albums in + * a follow-up call so the related-albums grid can render without blocking + * the initial paint. + * + * On every id change we reset `relatedAlbums` to an empty array so the + * grid doesn't briefly show the previous album's neighbours while the + * new fetch is in flight. The two starred state pieces (`isStarred`, + * `starredSongs`) are seeded from the response so optimistic toggles + * have a baseline to revert to. + */ +export function useAlbumDetailData(id: string | undefined): UseAlbumDetailDataResult { + const [album, setAlbum] = useState(null); + const [relatedAlbums, setRelatedAlbums] = useState([]); + const [loading, setLoading] = useState(true); + const [isStarred, setIsStarred] = useState(false); + const [starredSongs, setStarredSongs] = useState>(new Set()); + + useEffect(() => { + if (!id) return; + setLoading(true); + setRelatedAlbums([]); + getAlbum(id).then(async data => { + setAlbum(data); + setIsStarred(!!data.album.starred); + const initialStarred = new Set(); + data.songs.forEach(s => { if (s.starred) initialStarred.add(s.id); }); + setStarredSongs(initialStarred); + setLoading(false); + try { + const artistData = await getArtist(data.album.artistId); + setRelatedAlbums(artistData.albums.filter(a => a.id !== id)); + } catch (e) { + console.error('Failed to fetch related albums', e); + } + }).catch(() => setLoading(false)); + }, [id]); + + return { album, setAlbum, relatedAlbums, loading, isStarred, setIsStarred, starredSongs, setStarredSongs }; +} diff --git a/src/hooks/useAlbumDetailSort.ts b/src/hooks/useAlbumDetailSort.ts new file mode 100644 index 00000000..b41d2047 --- /dev/null +++ b/src/hooks/useAlbumDetailSort.ts @@ -0,0 +1,95 @@ +import { useMemo, useState } from 'react'; +import type { SubsonicSong } from '../api/subsonicTypes'; + +export type AlbumSortKey = 'natural' | 'title' | 'artist' | 'album' | 'favorite' | 'rating' | 'duration'; + +interface UseAlbumDetailSortArgs { + songs: SubsonicSong[] | undefined; + filterText: string; + starredSongs: Set; + ratings: Record; + userRatingOverrides: Record; +} + +interface UseAlbumDetailSortResult { + sortKey: AlbumSortKey; + sortDir: 'asc' | 'desc'; + handleSort: (key: AlbumSortKey) => void; + displayedSongs: SubsonicSong[]; +} + +/** + * Sort + text-filter pipeline for the album track list. Click cycle on a + * header column: asc → desc → off (back to `natural` order). Other + * columns reset to asc on first click. + * + * Rating reads use the same priority as the row renderer + * (`ratings[id] ?? userRatingOverrides[id] ?? song.userRating`) so the + * sort matches the visible stars during optimistic updates. + */ +export function useAlbumDetailSort({ + songs, + filterText, + starredSongs, + ratings, + userRatingOverrides, +}: UseAlbumDetailSortArgs): UseAlbumDetailSortResult { + const [sortKey, setSortKey] = useState('natural'); + const [sortDir, setSortDir] = useState<'asc' | 'desc'>('asc'); + const [sortClickCount, setSortClickCount] = useState(0); + + const handleSort = (key: AlbumSortKey) => { + if (key === 'natural') return; + if (sortKey === key) { + const nextCount = sortClickCount + 1; + if (nextCount >= 3) { + setSortKey('natural'); + setSortDir('asc'); + setSortClickCount(0); + } else { + setSortDir(d => d === 'asc' ? 'desc' : 'asc'); + setSortClickCount(nextCount); + } + } else { + setSortKey(key); + setSortDir('asc'); + setSortClickCount(1); + } + }; + + const displayedSongs = useMemo(() => { + if (!songs) return []; + const q = filterText.trim().toLowerCase(); + if (!q && sortKey === 'natural') return songs; + let result = [...songs]; + if (q) result = result.filter(s => s.title.toLowerCase().includes(q) || (s.artist ?? '').toLowerCase().includes(q)); + if (sortKey !== 'natural') { + result.sort((a, b) => { + let av: string | number; + let bv: string | number; + switch (sortKey) { + case 'title': av = a.title; bv = b.title; break; + case 'artist': av = a.artist ?? ''; bv = b.artist ?? ''; break; + case 'album': av = a.album ?? ''; bv = b.album ?? ''; break; + case 'favorite': + av = starredSongs.has(a.id) ? 1 : 0; + bv = starredSongs.has(b.id) ? 1 : 0; + break; + case 'rating': + av = ratings[a.id] ?? userRatingOverrides[a.id] ?? a.userRating ?? 0; + bv = ratings[b.id] ?? userRatingOverrides[b.id] ?? b.userRating ?? 0; + break; + case 'duration': av = a.duration ?? 0; bv = b.duration ?? 0; break; + default: av = a.title; bv = b.title; + } + if (typeof av === 'number' && typeof bv === 'number') { + return sortDir === 'asc' ? av - bv : bv - av; + } + return sortDir === 'asc' ? (av as string).localeCompare(bv as string) : (bv as string).localeCompare(av as string); + }); + } + return result; + }, [songs, filterText, sortKey, sortDir, starredSongs, ratings, userRatingOverrides]); + + return { sortKey, sortDir, handleSort, displayedSongs }; +} diff --git a/src/hooks/useAlbumOfflineState.ts b/src/hooks/useAlbumOfflineState.ts new file mode 100644 index 00000000..a4bea4c4 --- /dev/null +++ b/src/hooks/useAlbumOfflineState.ts @@ -0,0 +1,51 @@ +import { useOfflineStore } from '../store/offlineStore'; +import { useOfflineJobStore } from '../store/offlineJobStore'; + +interface UseAlbumOfflineStateResult { + resolvedOfflineStatus: 'none' | 'downloading' | 'cached'; + offlineProgress: { done: number; total: number } | null; +} + +/** + * Combined offline-cache status for an album. Splits the read across + * three primitive Zustand selectors so the page only re-renders when + * one of the resolved scalars (status / done count / total count) + * actually flips — not on every `jobs` array mutation during batch + * downloads (each track flip would otherwise trigger a full page render). + * + * Resolution rules: + * - If there's any queued / downloading job for this album, status is + * `downloading` and we expose a `{ done, total }` progress tuple. + * - Else we look at the persisted cache map: a fully-cached album is + * one where every trackId in the album-meta has a matching track entry. + * - Else `none`. + * + * `albumId` is allowed to be empty (e.g. while the page is still + * fetching) — in that case every selector short-circuits to a benign + * default. + */ +export function useAlbumOfflineState(albumId: string, serverId: string): UseAlbumOfflineStateResult { + const offlineStatus = useOfflineStore((s): 'none' | 'downloading' | 'cached' => { + if (!albumId) return 'none'; + const meta = s.albums[`${serverId}:${albumId}`]; + const isDownloaded = meta && meta.trackIds.length > 0 && meta.trackIds.every(tid => !!s.tracks[`${serverId}:${tid}`]); + return isDownloaded ? 'cached' : 'none'; + }); + const isOfflineDownloading = useOfflineJobStore(s => + !!albumId && s.jobs.some(j => j.albumId === albumId && (j.status === 'queued' || j.status === 'downloading')), + ); + const offlineProgressDone = useOfflineJobStore(s => { + if (!albumId) return 0; + return s.jobs.filter(j => j.albumId === albumId && (j.status === 'done' || j.status === 'error')).length; + }); + const offlineProgressTotal = useOfflineJobStore(s => { + if (!albumId) return 0; + return s.jobs.filter(j => j.albumId === albumId).length; + }); + const resolvedOfflineStatus = isOfflineDownloading ? 'downloading' : offlineStatus; + const offlineProgress = offlineProgressTotal > 0 + ? { done: offlineProgressDone, total: offlineProgressTotal } + : null; + + return { resolvedOfflineStatus, offlineProgress }; +} diff --git a/src/pages/AlbumDetail.tsx b/src/pages/AlbumDetail.tsx index 74d744b1..d8734caa 100644 --- a/src/pages/AlbumDetail.tsx +++ b/src/pages/AlbumDetail.tsx @@ -1,37 +1,30 @@ import { buildCoverArtUrl, coverArtCacheKey, buildDownloadUrl } from '../api/subsonicStreamUrl'; import { setRating, star, unstar } from '../api/subsonicStarRating'; -import { getArtist, getArtistInfo } from '../api/subsonicArtists'; -import { getAlbum } from '../api/subsonicLibrary'; -import type { SubsonicSong, SubsonicAlbum } from '../api/subsonicTypes'; +import { getArtistInfo } from '../api/subsonicArtists'; +import type { SubsonicSong } from '../api/subsonicTypes'; import { songToTrack } from '../utils/songToTrack'; import React, { useEffect, useState, useCallback, useMemo } from 'react'; import { useParams, useNavigate } from 'react-router-dom'; -import { Search, X, ListPlus } from 'lucide-react'; import { invoke } from '@tauri-apps/api/core'; import { usePlayerStore } from '../store/playerStore'; import { useAuthStore } from '../store/authStore'; import { useOrbitSongRowBehavior } from '../hooks/useOrbitSongRowBehavior'; +import { useAlbumDetailData } from '../hooks/useAlbumDetailData'; +import { useAlbumOfflineState } from '../hooks/useAlbumOfflineState'; +import { useAlbumDetailSort } from '../hooks/useAlbumDetailSort'; import { useDownloadModalStore } from '../store/downloadModalStore'; import { useOfflineStore } from '../store/offlineStore'; -import { useOfflineJobStore } from '../store/offlineJobStore'; import { join } from '@tauri-apps/api/path'; import { useZipDownloadStore } from '../store/zipDownloadStore'; import AlbumCard from '../components/AlbumCard'; import AlbumHeader from '../components/AlbumHeader'; import AlbumTrackList from '../components/AlbumTrackList'; -import { AddToPlaylistSubmenu } from '../components/ContextMenu'; +import { AlbumDetailToolbar } from '../components/albumDetail/AlbumDetailToolbar'; import { useCachedUrl } from '../components/CachedImage'; import { useTranslation } from 'react-i18next'; import { showToast } from '../utils/toast'; import { useSelectionStore } from '../store/selectionStore'; - -function sanitizeFilename(name: string): string { - return name - .replace(/[/\\?%*:|"<>]/g, '-') - .replace(/\.{2,}/g, '.') - .replace(/^[\s.]+|[\s.]+$/g, '') - .substring(0, 200) || 'download'; -} +import { sanitizeFilename } from '../utils/albumDetailHelpers'; export default function AlbumDetail() { const { t } = useTranslation(); @@ -48,14 +41,13 @@ export default function AlbumDetail() { const currentTrack = usePlayerStore(s => s.currentTrack); const isPlaying = usePlayerStore(s => s.isPlaying); - const [album, setAlbum] = useState> | null>(null); - const [relatedAlbums, setRelatedAlbums] = useState([]); + const { + album, setAlbum, relatedAlbums, loading, + isStarred, setIsStarred, starredSongs, setStarredSongs, + } = useAlbumDetailData(id); const [ratings, setRatings] = useState>({}); const [bio, setBio] = useState(null); const [bioOpen, setBioOpen] = useState(false); - const [loading, setLoading] = useState(true); - const [isStarred, setIsStarred] = useState(false); - const [starredSongs, setStarredSongs] = useState>(new Set()); const [offlineStorageFull, setOfflineStorageFull] = useState(false); const downloadAlbum = useOfflineStore(s => s.downloadAlbum); @@ -67,9 +59,6 @@ export default function AlbumDetail() { const [albumEntityRating, setAlbumEntityRating] = useState(0); const [filterText, setFilterText] = useState(''); - const [sortKey, setSortKey] = useState<'natural' | 'title' | 'artist' | 'album' | 'favorite' | 'rating' | 'duration'>('natural'); - const [sortDir, setSortDir] = useState<'asc' | 'desc'>('asc'); - const [sortClickCount, setSortClickCount] = useState(0); const [showPlPicker, setShowPlPicker] = useState(false); const selectedCount = useSelectionStore(s => s.selectedIds.size); const inSelectMode = selectedCount > 0; @@ -77,49 +66,7 @@ export default function AlbumDetail() { // Derive a stable albumId for the selectors below (empty string when not yet loaded). const albumId = album?.album.id ?? ''; - // Selectors return primitives so Zustand only triggers a re-render when the VALUE - // actually changes — not on every `jobs` array mutation during batch downloads. - const offlineStatus = useOfflineStore((s): 'none' | 'downloading' | 'cached' => { - if (!albumId) return 'none'; - const meta = s.albums[`${serverId}:${albumId}`]; - const isDownloaded = meta && meta.trackIds.length > 0 && meta.trackIds.every(tid => !!s.tracks[`${serverId}:${tid}`]); - return isDownloaded ? 'cached' : 'none'; - }); - const isOfflineDownloading = useOfflineJobStore(s => - !!albumId && s.jobs.some(j => j.albumId === albumId && (j.status === 'queued' || j.status === 'downloading')) - ); - const offlineProgressDone = useOfflineJobStore(s => { - if (!albumId) return 0; - return s.jobs.filter(j => j.albumId === albumId && (j.status === 'done' || j.status === 'error')).length; - }); - const offlineProgressTotal = useOfflineJobStore(s => { - if (!albumId) return 0; - return s.jobs.filter(j => j.albumId === albumId).length; - }); - const resolvedOfflineStatus = isOfflineDownloading ? 'downloading' : offlineStatus; - const offlineProgress = offlineProgressTotal > 0 - ? { done: offlineProgressDone, total: offlineProgressTotal } - : null; - - useEffect(() => { - if (!id) return; - setLoading(true); - setRelatedAlbums([]); - getAlbum(id).then(async data => { - setAlbum(data); - setIsStarred(!!data.album.starred); - const initialStarred = new Set(); - data.songs.forEach(s => { if (s.starred) initialStarred.add(s.id); }); - setStarredSongs(initialStarred); - setLoading(false); - try { - const artistData = await getArtist(data.album.artistId); - setRelatedAlbums(artistData.albums.filter(a => a.id !== id)); - } catch (e) { - console.error('Failed to fetch related albums', e); - } - }).catch(() => setLoading(false)); - }, [id]); + const { resolvedOfflineStatus, offlineProgress } = useAlbumOfflineState(albumId, serverId); useEffect(() => { if (!id) return; @@ -296,64 +243,19 @@ const handleShuffleAll = () => { deleteAlbum(album.album.id, serverId); }; - const handleSort = (key: typeof sortKey) => { - if (key === 'natural') return; - if (sortKey === key) { - const nextCount = sortClickCount + 1; - if (nextCount >= 3) { - setSortKey('natural'); - setSortDir('asc'); - setSortClickCount(0); - } else { - setSortDir(d => d === 'asc' ? 'desc' : 'asc'); - setSortClickCount(nextCount); - } - } else { - setSortKey(key); - setSortDir('asc'); - setSortClickCount(1); - } - }; - // Must be before early returns — hooks must be called unconditionally. const mergedStarredSongs = useMemo(() => new Set([ ...[...starredSongs].filter(id => starredOverrides[id] !== false), ...Object.entries(starredOverrides).filter(([, v]) => v).map(([k]) => k), ]), [starredSongs, starredOverrides]); - const displayedSongs = useMemo(() => { - if (!album) return []; - const q = filterText.trim().toLowerCase(); - if (!q && sortKey === 'natural') return album.songs; - let result = [...album.songs]; - if (q) result = result.filter(s => s.title.toLowerCase().includes(q) || (s.artist ?? '').toLowerCase().includes(q)); - if (sortKey !== 'natural') { - result.sort((a, b) => { - let av: string | number; - let bv: string | number; - switch (sortKey) { - case 'title': av = a.title; bv = b.title; break; - case 'artist': av = a.artist ?? ''; bv = b.artist ?? ''; break; - case 'album': av = a.album ?? ''; bv = b.album ?? ''; break; - case 'favorite': - av = mergedStarredSongs.has(a.id) ? 1 : 0; - bv = mergedStarredSongs.has(b.id) ? 1 : 0; - break; - case 'rating': - av = ratings[a.id] ?? userRatingOverrides[a.id] ?? a.userRating ?? 0; - bv = ratings[b.id] ?? userRatingOverrides[b.id] ?? b.userRating ?? 0; - break; - case 'duration': av = a.duration ?? 0; bv = b.duration ?? 0; break; - default: av = a.title; bv = b.title; - } - if (typeof av === 'number' && typeof bv === 'number') { - return sortDir === 'asc' ? av - bv : bv - av; - } - return sortDir === 'asc' ? (av as string).localeCompare(bv as string) : (bv as string).localeCompare(av as string); - }); - } - return result; - }, [album, filterText, sortKey, sortDir, mergedStarredSongs, ratings, userRatingOverrides]); + const { sortKey, sortDir, handleSort, displayedSongs } = useAlbumDetailSort({ + songs: album?.songs, + filterText, + starredSongs: mergedStarredSongs, + ratings, + userRatingOverrides, + }); // Hooks must be called unconditionally — derive from nullable album state. // useMemo is required: buildCoverArtUrl generates a new salt on every call, so without @@ -423,59 +325,15 @@ const handleShuffleAll = () => { )} {songs.length > 0 && ( -
-
- - setFilterText(e.target.value)} - /> - {filterText && ( - - )} -
-
- {inSelectMode && ( - <> - - {t('common.bulkSelected', { count: selectedCount })} - -
- - {showPlPicker && ( - { setShowPlPicker(false); useSelectionStore.getState().clearAll(); }} - dropDown - /> - )} -
- - - )} -
-
+ )} ]/g, '-') + .replace(/\.{2,}/g, '.') + .replace(/^[\s.]+|[\s.]+$/g, '') + .substring(0, 200) || 'download'; +}