From 6cc227d761296faef89f28adc6a35a8df16f901a Mon Sep 17 00:00:00 2001 From: Frank Stellmacher <171614930+Psychotoxical@users.noreply.github.com> Date: Sun, 17 May 2026 00:43:05 +0200 Subject: [PATCH] fix(artist-info): id-gate fetchers, share ArtistCard, square hero (#739) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(artist-info): id-gate fetcher tuples, reuse ArtistCard on ArtistDetail The cache-mismatch bug PR #732 fixed in `NowPlayingInfo.tsx` had the same shape inside `ArtistCard` on the NowPlaying page: `useNowPlayingFetchers` returned `artistInfo` for the previously-current artist for one render after `artistId` changed, and `CachedImage` persisted that mismatched blob under the new `artistInfo::hero` key in IndexedDB — sticky "previous artist" image on every subsequent track. Apply the same `{ id, value }` tuple pattern from PR #732 inside the hooks themselves so every consumer is safe by construction: - `useNowPlayingFetchers`: gate `artistInfo`, `songMeta`, `albumData`, `discography` on id-match at the return. Late-arriving resolves for a stale id can no longer overwrite the displayed value. - `useArtistDetailData`: same for `info`. Required because the `ArtistDetail` bio card now uses `CachedImage` via the shared `ArtistCard` (previously raw ``, no persistence hazard). Unify `ArtistDetail`'s inline "About the Artist" block onto the same shared `ArtistCard` so there is one source of truth for hero / bio / similar rendering. New optional props: `onNavigate?` (omitted on `/artist/:id` since the user is already there), `coverFallback` (coverArt fallback when artistInfo has no hero image), `hideArtistName` (avoid duplicating the hero name), `hideSimilar` (ArtistDetail has its own similar-artists section). Tests cover the gating contract for both hooks (incl. stale-resolve race) and the `ArtistCard` prop matrix. * fix(artist-image): square queue info hero, drop artist-avatar glow - Queue Info bar's artist hero was rendered in a 16:10 wrap with `object-fit: cover`, so portrait photos lost top/bottom equally while landscape ones lost the sides — perceived as cropped even on roughly square sources. Set the wrap to 1:1 so the crop is symmetric and matches the typical square framing of artist photography. - `ArtistDetail` extracted the cover's accent colour on every image load and rendered a 36px / 8px-spread `boxShadow` ring around the avatar. Drop the glow, the state, the one-shot reset effect, the prop-passing through `ArtistDetailHero`, and the now-orphaned `extractCoverColors` import on this page. `extractCoverColors` itself stays in place (still used by `useFsDynamicAccent`). * docs(changelog): artist-info image fix extension + UI tweaks (PR #739) --- CHANGELOG.md | 8 + .../artistDetail/ArtistDetailHero.tsx | 15 +- src/components/nowPlaying/ArtistCard.test.tsx | 80 +++++++ src/components/nowPlaying/ArtistCard.tsx | 29 ++- src/hooks/useArtistDetailData.test.ts | 98 ++++++++ src/hooks/useArtistDetailData.ts | 13 +- src/hooks/useNowPlayingFetchers.test.ts | 211 ++++++++++++++++++ src/hooks/useNowPlayingFetchers.ts | 78 ++++--- src/pages/ArtistDetail.tsx | 47 +--- src/styles/components/mini-player-window.css | 2 +- 10 files changed, 493 insertions(+), 88 deletions(-) create mode 100644 src/components/nowPlaying/ArtistCard.test.tsx create mode 100644 src/hooks/useArtistDetailData.test.ts create mode 100644 src/hooks/useNowPlayingFetchers.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b388ac9b..6a885d58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -603,6 +603,14 @@ Foundational work: faster reviews, narrower diffs, and a safety net under the pa * The **Artist Biography** modal lived under the album page tree, where an ancestor broke **`position: fixed`** on the overlay — opening a long bio scrolled the whole page instead of staying pinned to the viewport, and the modal itself stretched past the visible area. It now mounts via **`createPortal(..., document.body)`** (same approach as **`RadioEditModal`** / **`CoverLightbox`**), so the overlay always pins to the viewport. * A new **`.modal-content.bio-modal`** variant turns the modal into a flex column with **`overflow: hidden`** and an inner **`.bio-modal-body`** that handles the scrolling. The existing **`max-height: 80vh`** cap is now honored, and the title + close button stay pinned while the bio scrolls. +### Artist info — image-mismatch fix extended; square Queue Info hero; ArtistDetail glow removed + +**By [@Psychotoxical](https://github.com/Psychotoxical), PR [#739](https://github.com/Psychotoxical/psysonic/pull/739)** + +* The cache-mismatch shape PR [#732](https://github.com/Psychotoxical/psysonic/pull/732) fixed inside the Queue Info panel was latent in the **About the Artist** card on the **NowPlaying** page and would have surfaced there the moment that card used **`CachedImage`**. Fixed at the source: **`useNowPlayingFetchers`** and **`useArtistDetailData`** hold id-keyed entities as **`{ id, value }`** tuples and gate them on id-match at the return — every consumer is safe by construction. ArtistDetail's inline bio block is now the shared **`ArtistCard`** so there is a single rendering path. +* The artist hero in the **Queue Info** panel was rendered in a **16:10** wrap with **`object-fit: cover`**, so portrait photos always lost top and bottom equally — perceived as cropped even on roughly square sources. Now **1:1**, symmetric crop. +* The **ArtistDetail** avatar extracted the cover's accent colour on every image load and painted a 36px **`boxShadow`** ring around the photo. The glow is gone (the state, the reset effect, and the per-page **`extractCoverColors`** import are dropped too; the utility itself stays in place for **`useFsDynamicAccent`**). + ## [1.45.0] - 2026-05-04 ## Added diff --git a/src/components/artistDetail/ArtistDetailHero.tsx b/src/components/artistDetail/ArtistDetailHero.tsx index b32a1cd5..2513f02b 100644 --- a/src/components/artistDetail/ArtistDetailHero.tsx +++ b/src/components/artistDetail/ArtistDetailHero.tsx @@ -10,7 +10,6 @@ import { useOfflineStore } from '../../store/offlineStore'; import { useOfflineJobStore } from '../../store/offlineJobStore'; import { useAuthStore } from '../../store/authStore'; import { useIsMobile } from '../../hooks/useIsMobile'; -import { extractCoverColors } from '../../utils/ui/dynamicColors'; import CachedImage from '../CachedImage'; import CoverLightbox from '../CoverLightbox'; import LastfmIcon from '../LastfmIcon'; @@ -42,8 +41,6 @@ interface Props { coverRevision: number; headerCoverFailed: boolean; setHeaderCoverFailed: React.Dispatch>; - avatarGlow: string; - setAvatarGlow: React.Dispatch>; lightboxOpen: boolean; setLightboxOpen: React.Dispatch>; } @@ -55,7 +52,7 @@ export default function ArtistDetailHero({ openedLink, openLink, coverId, artistCover300Src, artistCover300Key, artistCover2000Src, coverRevision, headerCoverFailed, setHeaderCoverFailed, - avatarGlow, setAvatarGlow, lightboxOpen, setLightboxOpen, + lightboxOpen, setLightboxOpen, }: Props) { const { t } = useTranslation(); const navigate = useNavigate(); @@ -88,14 +85,7 @@ export default function ArtistDetailHero({ )}
-
+
{coverId ? ( @@ -60,7 +73,7 @@ const ArtistCard = memo(function ArtistCard({ artistName, artistId, artistInfo, /> )}
-
{artistName}
+ {!hideArtistName &&
{artistName}
} {bioHtml && ( <>
{similar.slice(0, 12).map((a, idx) => ( a.id && onNavigate(`/artist/${a.id}`)} + onClick={() => a.id && onNavigate?.(`/artist/${a.id}`)} data-tooltip={t('nowPlaying.goToArtist')}> {a.name} diff --git a/src/hooks/useArtistDetailData.test.ts b/src/hooks/useArtistDetailData.test.ts new file mode 100644 index 00000000..85f6c87b --- /dev/null +++ b/src/hooks/useArtistDetailData.test.ts @@ -0,0 +1,98 @@ +/** + * Regression tests for the id-gating tuple pattern in `useArtistDetailData`. + * + * `info` (the SubsonicArtistInfo returned by getArtistInfo) is held as a + * `{ id, value }` tuple internally and gated on id-match at the return + * statement. Without this gate, navigating between /artist/A → /artist/B + * would render one frame with A's `largeImageUrl` paired with B's id — + * exactly the cache-mismatch shape that PR #732 fixed for the queue info + * panel and that the shared ArtistCard (now used on this page) would + * otherwise persist into IndexedDB. + */ +import { renderHook, act, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { SubsonicArtistInfo } from '../api/subsonicTypes'; + +vi.mock('../api/subsonicArtists'); +vi.mock('../api/subsonicSearch'); + +import { getArtist, getArtistInfo, getTopSongs } from '../api/subsonicArtists'; +import { search } from '../api/subsonicSearch'; +import { useArtistDetailData } from './useArtistDetailData'; + +const mockArtistInfo = vi.mocked(getArtistInfo) as unknown as { + mockImplementation: (impl: (id: string) => Promise) => void; +}; + +beforeEach(() => { + vi.mocked(getTopSongs).mockResolvedValue([]); + vi.mocked(search).mockResolvedValue({ songs: [], albums: [], artists: [] } as any); +}); + +afterEach(() => { + vi.clearAllMocks(); +}); + +function deferred() { + let resolve!: (v: T) => void; + let reject!: (e?: unknown) => void; + const promise = new Promise((res, rej) => { resolve = res; reject = rej; }); + return { promise, resolve, reject }; +} + +describe('useArtistDetailData — id-gated info', () => { + it('returns null info when id changes before the new fetch resolves', async () => { + vi.mocked(getArtist).mockImplementation(async (id) => ( + { artist: { id, name: id }, albums: [] } as any + )); + const a = deferred(); + const b = deferred(); + mockArtistInfo.mockImplementation(async (id) => { + if (id === 'A') return a.promise; + if (id === 'B') return b.promise; + return null; + }); + + const { result, rerender } = renderHook( + ({ id }: { id: string }) => useArtistDetailData(id), + { initialProps: { id: 'A' } }, + ); + + await act(async () => { a.resolve({ largeImageUrl: 'A.jpg' } as SubsonicArtistInfo); }); + await waitFor(() => expect(result.current.info).toEqual({ largeImageUrl: 'A.jpg' })); + + // Switch to artist B. info must flip to null until B's fetch resolves — + // it must never carry A's largeImageUrl paired with B's id, since the + // shared ArtistCard would build a `coverArtCacheKey(B, 80)` from `id` + // and pair it with A's URL inside CachedImage. + rerender({ id: 'B' }); + expect(result.current.info).toBeNull(); + + await act(async () => { b.resolve({ largeImageUrl: 'B.jpg' } as SubsonicArtistInfo); }); + await waitFor(() => expect(result.current.info).toEqual({ largeImageUrl: 'B.jpg' })); + }); + + it('ignores a late-arriving resolve for a stale id', async () => { + vi.mocked(getArtist).mockImplementation(async (id) => ( + { artist: { id, name: id }, albums: [] } as any + )); + const a = deferred(); + mockArtistInfo.mockImplementation(async (id) => { + if (id === 'A') return a.promise; + if (id === 'B') return { largeImageUrl: 'B.jpg' } as SubsonicArtistInfo; + return null; + }); + + const { result, rerender } = renderHook( + ({ id }: { id: string }) => useArtistDetailData(id), + { initialProps: { id: 'A' } }, + ); + + rerender({ id: 'B' }); + await waitFor(() => expect(result.current.info).toEqual({ largeImageUrl: 'B.jpg' })); + + // A's late resolve must not overwrite B's info. + await act(async () => { a.resolve({ largeImageUrl: 'A.jpg' } as SubsonicArtistInfo); }); + expect(result.current.info).toEqual({ largeImageUrl: 'B.jpg' }); + }); +}); diff --git a/src/hooks/useArtistDetailData.ts b/src/hooks/useArtistDetailData.ts index 8e970dd9..af63a115 100644 --- a/src/hooks/useArtistDetailData.ts +++ b/src/hooks/useArtistDetailData.ts @@ -30,7 +30,10 @@ export function useArtistDetailData(id: string | undefined): ArtistDetailDataRes const [albums, setAlbums] = useState([]); const [featuredAlbums, setFeaturedAlbums] = useState([]); const [topSongs, setTopSongs] = useState([]); - const [info, setInfo] = useState(null); + // Tuple gates `info` on id-match so a CachedImage-style consumer (shared + // ArtistCard) can never see info from a previously-viewed artist paired + // with the current `id`. Same pattern as `useNowPlayingFetchers`. + const [infoEntry, setInfoEntry] = useState<{ id: string; value: SubsonicArtistInfo | null } | null>(null); const [loading, setLoading] = useState(true); const [isStarred, setIsStarred] = useState(false); const [artistInfoLoading, setArtistInfoLoading] = useState(false); @@ -40,7 +43,7 @@ export function useArtistDetailData(id: string | undefined): ArtistDetailDataRes if (!id) return; let cancelled = false; setLoading(true); - setInfo(null); + setInfoEntry(null); setTopSongs([]); setFeaturedAlbums([]); getArtist(id).then(artistData => { @@ -66,10 +69,10 @@ export function useArtistDetailData(id: string | undefined): ArtistDetailDataRes setArtistInfoLoading(true); getArtistInfo(id, { similarArtistCount: audiomuseNavidromeEnabled ? 24 : undefined }) .then(artistInfo => { - if (!cancelled) setInfo(artistInfo ?? null); + if (!cancelled) setInfoEntry({ id, value: artistInfo ?? null }); }) .catch(() => { - if (!cancelled) setInfo(null); + if (!cancelled) setInfoEntry({ id, value: null }); }) .finally(() => { if (!cancelled) setArtistInfoLoading(false); @@ -113,6 +116,8 @@ export function useArtistDetailData(id: string | undefined): ArtistDetailDataRes // eslint-disable-next-line react-hooks/exhaustive-deps }, [artist?.id, musicLibraryFilterVersion]); + const info = infoEntry && infoEntry.id === id ? infoEntry.value : null; + return { artist, setArtist, albums, topSongs, info, featuredAlbums, loading, artistInfoLoading, featuredLoading, diff --git a/src/hooks/useNowPlayingFetchers.test.ts b/src/hooks/useNowPlayingFetchers.test.ts new file mode 100644 index 00000000..e4f3b249 --- /dev/null +++ b/src/hooks/useNowPlayingFetchers.test.ts @@ -0,0 +1,211 @@ +/** + * Regression tests for the id-gating tuple pattern in `useNowPlayingFetchers`. + * + * Each id-keyed slot (`artistInfo`, `songMeta`, `albumData`, `discography`) is + * held as a `{ id, value }` tuple internally and gated on id-match at the + * return statement. This guarantees that consumers building a `cacheKey` from + * the current id can never receive a value paired with a previously-current + * id — the bug that PR #732 fixed inside `NowPlayingInfo.tsx` and that this + * hook would otherwise leak into every other consumer (e.g. ArtistCard on the + * NowPlaying page). + */ +import { renderHook, act, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { SubsonicArtistInfo, SubsonicSong, SubsonicAlbum } from '../api/subsonicTypes'; + +vi.mock('../api/subsonicArtists'); +vi.mock('../api/subsonicLibrary'); +vi.mock('../api/bandsintown'); +vi.mock('../api/lastfm'); + +import { getArtist, getArtistInfo, getTopSongs } from '../api/subsonicArtists'; +import { getAlbum, getSong } from '../api/subsonicLibrary'; +import { fetchBandsintownEvents } from '../api/bandsintown'; +import { lastfmGetArtistStats, lastfmGetTrackInfo, lastfmIsConfigured } from '../api/lastfm'; +import { useNowPlayingFetchers, type NowPlayingFetchersDeps } from './useNowPlayingFetchers'; + +// The real getArtistInfo signature returns `Promise`, but +// the hook treats `null` as the "no info available" case and stores it as +// such in its tuple. The tests mirror that — cast to a nullable-returning +// shape so we can mock the empty case without `as any` at every site. +const mockArtistInfo = vi.mocked(getArtistInfo) as unknown as { + mockImplementation: (impl: (id: string) => Promise) => void; + mockResolvedValue: (v: SubsonicArtistInfo | null) => void; +}; + +const baseDeps: NowPlayingFetchersDeps = { + songId: undefined, + artistId: undefined, + albumId: undefined, + artistName: '', + enableBandsintown: false, + audiomuseNavidromeEnabled: false, + lastfmUsername: '', + currentTrack: null, + subsonicServerId: 'srv1', + fetchEnabled: true, +}; + +beforeEach(() => { + vi.mocked(getTopSongs).mockResolvedValue([]); + vi.mocked(getArtist).mockResolvedValue({ albums: [] } as any); + vi.mocked(fetchBandsintownEvents).mockResolvedValue([]); + vi.mocked(lastfmIsConfigured).mockReturnValue(false); + vi.mocked(lastfmGetTrackInfo).mockResolvedValue(null); + vi.mocked(lastfmGetArtistStats).mockResolvedValue(null); +}); + +afterEach(() => { + vi.clearAllMocks(); +}); + +/** Deferred promise helper — lets a test step the fetch resolution manually. */ +function deferred() { + let resolve!: (v: T) => void; + let reject!: (e?: unknown) => void; + const promise = new Promise((res, rej) => { resolve = res; reject = rej; }); + return { promise, resolve, reject }; +} + +describe('useNowPlayingFetchers — id-gated artistInfo', () => { + it('returns null artistInfo while the previously-resolved info belongs to a different artistId', async () => { + const a = deferred(); + const b = deferred(); + mockArtistInfo.mockImplementation(async (id) => { + if (id === 'art-A') return a.promise; + if (id === 'art-B') return b.promise; + return null; + }); + + const { result, rerender } = renderHook( + ({ artistId }: { artistId: string }) => + useNowPlayingFetchers({ ...baseDeps, artistId, artistName: artistId }), + { initialProps: { artistId: 'art-A' } }, + ); + + // Before any resolve, no info yet. + expect(result.current.artistInfo).toBeNull(); + + // Resolve A — artistInfo becomes A's info. + await act(async () => { a.resolve({ largeImageUrl: 'A.jpg' } as SubsonicArtistInfo); }); + await waitFor(() => { + expect(result.current.artistInfo).toEqual({ largeImageUrl: 'A.jpg' }); + }); + + // Switch to artistId B. Without id-gating, artistInfo would still be A's + // info paired with the new B id — the bug that PR #732 fixed in the queue + // info panel. With gating, artistInfo flips to null immediately. + rerender({ artistId: 'art-B' }); + expect(result.current.artistInfo).toBeNull(); + + // Resolve B — artistInfo now becomes B's info, never paired with A. + await act(async () => { b.resolve({ largeImageUrl: 'B.jpg' } as SubsonicArtistInfo); }); + await waitFor(() => { + expect(result.current.artistInfo).toEqual({ largeImageUrl: 'B.jpg' }); + }); + }); + + it('does not leak a late-arriving resolve for a stale artistId', async () => { + // Race: artist A's fetch resolves AFTER the consumer switched to B. + const a = deferred(); + mockArtistInfo.mockImplementation(async (id) => { + if (id === 'art-A') return a.promise; + if (id === 'art-B') return { largeImageUrl: 'B.jpg' } as SubsonicArtistInfo; + return null; + }); + + const { result, rerender } = renderHook( + ({ artistId }: { artistId: string }) => + useNowPlayingFetchers({ ...baseDeps, artistId, artistName: artistId }), + { initialProps: { artistId: 'art-A' } }, + ); + + // Switch to B before A resolves. + rerender({ artistId: 'art-B' }); + await waitFor(() => { + expect(result.current.artistInfo).toEqual({ largeImageUrl: 'B.jpg' }); + }); + + // Late A resolve must not overwrite the displayed value for B. + await act(async () => { a.resolve({ largeImageUrl: 'A.jpg' } as SubsonicArtistInfo); }); + expect(result.current.artistInfo).toEqual({ largeImageUrl: 'B.jpg' }); + }); +}); + +describe('useNowPlayingFetchers — id-gated songMeta / albumData / discography', () => { + it('gates songMeta on songId match', async () => { + const s1 = deferred(); + const s2 = deferred(); + vi.mocked(getSong).mockImplementation(async (id) => { + if (id === 's1') return s1.promise; + if (id === 's2') return s2.promise; + return null; + }); + + const { result, rerender } = renderHook( + ({ songId }: { songId: string }) => + useNowPlayingFetchers({ ...baseDeps, songId }), + { initialProps: { songId: 's1' } }, + ); + + await act(async () => { s1.resolve({ id: 's1', title: 'Track 1' } as SubsonicSong); }); + await waitFor(() => expect(result.current.songMeta?.title).toBe('Track 1')); + + rerender({ songId: 's2' }); + expect(result.current.songMeta).toBeNull(); + + await act(async () => { s2.resolve({ id: 's2', title: 'Track 2' } as SubsonicSong); }); + await waitFor(() => expect(result.current.songMeta?.title).toBe('Track 2')); + }); + + it('gates albumData on albumId match', async () => { + const al1 = deferred<{ album: SubsonicAlbum; songs: SubsonicSong[] } | null>(); + const al2 = deferred<{ album: SubsonicAlbum; songs: SubsonicSong[] } | null>(); + vi.mocked(getAlbum).mockImplementation(async (id) => { + if (id === 'alb1') return al1.promise as any; + if (id === 'alb2') return al2.promise as any; + return null as any; + }); + + const { result, rerender } = renderHook( + ({ albumId }: { albumId: string }) => + useNowPlayingFetchers({ ...baseDeps, albumId }), + { initialProps: { albumId: 'alb1' } }, + ); + + await act(async () => { al1.resolve({ album: { id: 'alb1', name: 'A1' } as SubsonicAlbum, songs: [] }); }); + await waitFor(() => expect(result.current.albumData?.album.name).toBe('A1')); + + rerender({ albumId: 'alb2' }); + expect(result.current.albumData).toBeNull(); + + await act(async () => { al2.resolve({ album: { id: 'alb2', name: 'A2' } as SubsonicAlbum, songs: [] }); }); + await waitFor(() => expect(result.current.albumData?.album.name).toBe('A2')); + }); + + it('gates discography on artistId match (empty fallback while gated)', async () => { + const d1 = deferred<{ artist: any; albums: SubsonicAlbum[] }>(); + const d2 = deferred<{ artist: any; albums: SubsonicAlbum[] }>(); + vi.mocked(getArtist).mockImplementation(async (id) => { + if (id === 'art-D1') return d1.promise as any; + if (id === 'art-D2') return d2.promise as any; + return { albums: [] } as any; + }); + mockArtistInfo.mockResolvedValue(null); + + const { result, rerender } = renderHook( + ({ artistId }: { artistId: string }) => + useNowPlayingFetchers({ ...baseDeps, artistId, artistName: artistId }), + { initialProps: { artistId: 'art-D1' } }, + ); + + await act(async () => { d1.resolve({ artist: {}, albums: [{ id: 'al-D1' } as SubsonicAlbum] }); }); + await waitFor(() => expect(result.current.discography.map(a => a.id)).toEqual(['al-D1'])); + + rerender({ artistId: 'art-D2' }); + expect(result.current.discography).toEqual([]); + + await act(async () => { d2.resolve({ artist: {}, albums: [{ id: 'al-D2' } as SubsonicAlbum] }); }); + await waitFor(() => expect(result.current.discography.map(a => a.id)).toEqual(['al-D2'])); + }); +}); diff --git a/src/hooks/useNowPlayingFetchers.ts b/src/hooks/useNowPlayingFetchers.ts index b362f038..d6f7f4e7 100644 --- a/src/hooks/useNowPlayingFetchers.ts +++ b/src/hooks/useNowPlayingFetchers.ts @@ -50,62 +50,81 @@ function subsonicCacheKey(serverId: string, id: string): string { return serverId ? `${serverId}:${id}` : id; } +// id-keyed slots are held as `{ id, value }` tuples and gated on id-match in +// the return statement. Without the gate, a track switch renders one frame +// with the previous track's value paired with the new id — consumers that +// build a cacheKey from the new id (e.g. CachedImage) would persist a +// mismatched blob in IndexedDB and never recover. See PR #732 for the same +// fix inside `NowPlayingInfo.tsx`. +type IdSlot = { id: string; value: T } | null; +function seedSlot(id: string, lookup: (id: string) => T | undefined): IdSlot { + if (!id) return null; + const cached = lookup(id); + return cached === undefined ? null : { id, value: cached }; +} + export function useNowPlayingFetchers(deps: NowPlayingFetchersDeps): NowPlayingFetchersResult { const { songId, artistId, albumId, artistName, enableBandsintown, audiomuseNavidromeEnabled, lastfmUsername, currentTrack, subsonicServerId, fetchEnabled = true, } = deps; - // Entity state, seeded from TTL cache so same-artist song switches are instant - const [songMeta, setSongMeta] = useState(() => - songId && subsonicServerId ? songMetaCache.get(subsonicCacheKey(subsonicServerId, songId)) ?? null : null); - const [artistInfo, setArtistInfo] = useState(() => - artistId && subsonicServerId ? artistInfoCache.get(subsonicCacheKey(subsonicServerId, artistId)) ?? null : null); - const [albumData, setAlbumData] = useState<{ album: SubsonicAlbum; songs: SubsonicSong[] } | null>(() => - albumId && subsonicServerId ? albumCache.get(subsonicCacheKey(subsonicServerId, albumId)) ?? null : null); + // id-keyed entity state — seeded from TTL cache so same-artist song switches + // are instant. Held as `{ id, value }` tuples and gated below. + const [songMetaEntry, setSongMetaEntry] = useState>(() => + seedSlot(songId && subsonicServerId ? songId : '', k => songMetaCache.get(subsonicCacheKey(subsonicServerId, k)))); + const [artistInfoEntry, setArtistInfoEntry] = useState>(() => + seedSlot(artistId && subsonicServerId ? artistId : '', k => artistInfoCache.get(subsonicCacheKey(subsonicServerId, k)))); + const [albumDataEntry, setAlbumDataEntry] = useState>(() => + seedSlot(albumId && subsonicServerId ? albumId : '', k => albumCache.get(subsonicCacheKey(subsonicServerId, k)))); + const [discographyEntry, setDiscographyEntry] = useState>(() => + seedSlot(artistId && subsonicServerId ? artistId : '', k => discographyCache.get(subsonicCacheKey(subsonicServerId, k)))); + + // Name-keyed / global state — no cacheKey/persistence hazard, kept as plain state. const [topSongs, setTopSongs] = useState(() => artistName && subsonicServerId ? topSongsCache.get(subsonicCacheKey(subsonicServerId, artistName)) ?? [] : []); const [tourEvents, setTourEvents] = useState(() => artistName ? tourCache.get(artistName) ?? [] : []); const [tourLoading, setTourLoading] = useState(false); - const [discography, setDiscography] = useState(() => - artistId && subsonicServerId ? discographyCache.get(subsonicCacheKey(subsonicServerId, artistId)) ?? [] : []); const [lfmTrack, setLfmTrack] = useState(null); const [lfmArtist, setLfmArtist] = useState(null); // Fetch batch per entity change (not per song switch — same-artist songs share artist/top/tour fetches) useEffect(() => { - if (!fetchEnabled || !subsonicServerId || !songId) { setSongMeta(null); return; } + if (!fetchEnabled || !subsonicServerId || !songId) { setSongMetaEntry(null); return; } const cacheKey = subsonicCacheKey(subsonicServerId, songId); const cached = songMetaCache.get(cacheKey); - if (cached !== undefined) { setSongMeta(cached); return; } + if (cached !== undefined) { setSongMetaEntry({ id: songId, value: cached }); return; } + setSongMetaEntry(null); let cancelled = false; getSong(songId) - .then(v => { if (!cancelled) { songMetaCache.set(cacheKey, v ?? null); setSongMeta(v ?? null); } }) - .catch(() => { if (!cancelled) { songMetaCache.set(cacheKey, null); setSongMeta(null); } }); + .then(v => { if (!cancelled) { songMetaCache.set(cacheKey, v ?? null); setSongMetaEntry({ id: songId, value: v ?? null }); } }) + .catch(() => { if (!cancelled) { songMetaCache.set(cacheKey, null); setSongMetaEntry({ id: songId, value: null }); } }); return () => { cancelled = true; }; }, [fetchEnabled, subsonicServerId, songId]); useEffect(() => { - if (!fetchEnabled || !subsonicServerId || !artistId) { setArtistInfo(null); return; } + if (!fetchEnabled || !subsonicServerId || !artistId) { setArtistInfoEntry(null); return; } const cacheKey = subsonicCacheKey(subsonicServerId, artistId); const cached = artistInfoCache.get(cacheKey); - if (cached !== undefined) { setArtistInfo(cached); return; } + if (cached !== undefined) { setArtistInfoEntry({ id: artistId, value: cached }); return; } + setArtistInfoEntry(null); let cancelled = false; getArtistInfo(artistId, { similarArtistCount: audiomuseNavidromeEnabled ? 24 : undefined }) - .then(v => { if (!cancelled) { artistInfoCache.set(cacheKey, v ?? null); setArtistInfo(v ?? null); } }) - .catch(() => { if (!cancelled) { artistInfoCache.set(cacheKey, null); setArtistInfo(null); } }); + .then(v => { if (!cancelled) { artistInfoCache.set(cacheKey, v ?? null); setArtistInfoEntry({ id: artistId, value: v ?? null }); } }) + .catch(() => { if (!cancelled) { artistInfoCache.set(cacheKey, null); setArtistInfoEntry({ id: artistId, value: null }); } }); return () => { cancelled = true; }; }, [fetchEnabled, subsonicServerId, artistId, audiomuseNavidromeEnabled]); useEffect(() => { - if (!fetchEnabled || !subsonicServerId || !albumId) { setAlbumData(null); return; } + if (!fetchEnabled || !subsonicServerId || !albumId) { setAlbumDataEntry(null); return; } const cacheKey = subsonicCacheKey(subsonicServerId, albumId); const cached = albumCache.get(cacheKey); - if (cached !== undefined) { setAlbumData(cached); return; } + if (cached !== undefined) { setAlbumDataEntry({ id: albumId, value: cached }); return; } + setAlbumDataEntry(null); let cancelled = false; getAlbum(albumId) - .then(v => { if (!cancelled) { albumCache.set(cacheKey, v); setAlbumData(v); } }) - .catch(() => { if (!cancelled) { albumCache.set(cacheKey, null); setAlbumData(null); } }); + .then(v => { if (!cancelled) { albumCache.set(cacheKey, v); setAlbumDataEntry({ id: albumId, value: v }); } }) + .catch(() => { if (!cancelled) { albumCache.set(cacheKey, null); setAlbumDataEntry({ id: albumId, value: null }); } }); return () => { cancelled = true; }; }, [fetchEnabled, subsonicServerId, albumId]); @@ -135,14 +154,15 @@ export function useNowPlayingFetchers(deps: NowPlayingFetchersDeps): NowPlayingF // Discography via getArtist useEffect(() => { - if (!fetchEnabled || !subsonicServerId || !artistId) { setDiscography([]); return; } + if (!fetchEnabled || !subsonicServerId || !artistId) { setDiscographyEntry(null); return; } const cacheKey = subsonicCacheKey(subsonicServerId, artistId); const cached = discographyCache.get(cacheKey); - if (cached !== undefined) { setDiscography(cached); return; } + if (cached !== undefined) { setDiscographyEntry({ id: artistId, value: cached }); return; } + setDiscographyEntry(null); let cancelled = false; getArtist(artistId) - .then(v => { if (!cancelled) { discographyCache.set(cacheKey, v.albums); setDiscography(v.albums); } }) - .catch(() => { if (!cancelled) { discographyCache.set(cacheKey, []); setDiscography([]); } }); + .then(v => { if (!cancelled) { discographyCache.set(cacheKey, v.albums); setDiscographyEntry({ id: artistId, value: v.albums }); } }) + .catch(() => { if (!cancelled) { discographyCache.set(cacheKey, []); setDiscographyEntry({ id: artistId, value: [] }); } }); return () => { cancelled = true; }; }, [fetchEnabled, subsonicServerId, artistId]); @@ -172,5 +192,13 @@ export function useNowPlayingFetchers(deps: NowPlayingFetchersDeps): NowPlayingF return () => { cancelled = true; }; }, [lfmArtistKey, artistName, lastfmUsername]); + // Gate id-keyed slots on id-match so consumers never see a value paired + // with the wrong id, even on the single render between an id change and + // the next effect run. + const songMeta = songMetaEntry && songMetaEntry.id === songId ? songMetaEntry.value : null; + const artistInfo = artistInfoEntry && artistInfoEntry.id === artistId ? artistInfoEntry.value : null; + const albumData = albumDataEntry && albumDataEntry.id === albumId ? albumDataEntry.value : null; + const discography = discographyEntry && discographyEntry.id === artistId ? discographyEntry.value : []; + return { songMeta, artistInfo, albumData, topSongs, tourEvents, tourLoading, discography, lfmTrack, lfmArtist }; } diff --git a/src/pages/ArtistDetail.tsx b/src/pages/ArtistDetail.tsx index cc7c483c..d2031181 100644 --- a/src/pages/ArtistDetail.tsx +++ b/src/pages/ArtistDetail.tsx @@ -24,11 +24,9 @@ import LastfmIcon from '../components/LastfmIcon'; import { invalidateCoverArt } from '../utils/imageCache'; import { showToast } from '../utils/ui/toast'; import { copyEntityShareLink } from '../utils/share/copyEntityShareLink'; -import { extractCoverColors } from '../utils/ui/dynamicColors'; import StarRating from '../components/StarRating'; import { useArtistLayoutStore, type ArtistSectionId } from '../store/artistLayoutStore'; -import { sanitizeHtml } from '../utils/sanitizeHtml'; import { useArtistDetailData } from '../hooks/useArtistDetailData'; import { useArtistSimilarArtists } from '../hooks/useArtistSimilarArtists'; import { @@ -40,6 +38,7 @@ import { import ArtistDetailHero from '../components/artistDetail/ArtistDetailHero'; import ArtistDetailTopTracks from '../components/artistDetail/ArtistDetailTopTracks'; import ArtistDetailSimilarArtists from '../components/artistDetail/ArtistDetailSimilarArtists'; +import ArtistCard from '../components/nowPlaying/ArtistCard'; import { usePerfProbeFlags } from '../utils/perf/perfFlags'; import { VirtualCardGrid } from '../components/VirtualCardGrid'; @@ -59,12 +58,10 @@ export default function ArtistDetail() { const [openedLink, setOpenedLink] = useState(null); const { similarArtists, similarLoading } = useArtistSimilarArtists(artist, info, artistInfoLoading); const [lightboxOpen, setLightboxOpen] = useState(false); - const [bioExpanded, setBioExpanded] = useState(false); const [uploading, setUploading] = useState(false); const [similarCollapsed, setSimilarCollapsed] = useState(true); const isMobile = useIsMobile(); const [coverRevision, setCoverRevision] = useState(0); - const [avatarGlow, setAvatarGlow] = useState(''); /** True after header CachedImage onError — avoid `display:none` on the img (breaks recovery). */ const [headerCoverFailed, setHeaderCoverFailed] = useState(false); const imageInputRef = useRef(null); @@ -94,10 +91,6 @@ export default function ArtistDetail() { const [artistEntityRating, setArtistEntityRating] = useState(0); - useEffect(() => { - setAvatarGlow(''); - }, [id]); - useEffect(() => { if (!id) return; if (artist && artist.id === id) setArtistEntityRating(artist.userRating ?? 0); @@ -295,8 +288,6 @@ export default function ArtistDetail() { coverRevision={coverRevision} headerCoverFailed={headerCoverFailed} setHeaderCoverFailed={setHeaderCoverFailed} - avatarGlow={avatarGlow} - setAvatarGlow={setAvatarGlow} lightboxOpen={lightboxOpen} setLightboxOpen={setLightboxOpen} /> @@ -307,33 +298,15 @@ export default function ArtistDetail() { {renderableSectionIds.map(sectionId => { switch (sectionId) { case 'bio': return ( -
-
-

{t('nowPlaying.aboutArtist')}

-
-
- {(info?.largeImageUrl || coverId) && ( - {artist.name} { (e.target as HTMLImageElement).style.display = 'none'; }} - /> - )} -
-
- -
-
+
+
); diff --git a/src/styles/components/mini-player-window.css b/src/styles/components/mini-player-window.css index 94dc9bb0..b082d4fc 100644 --- a/src/styles/components/mini-player-window.css +++ b/src/styles/components/mini-player-window.css @@ -576,7 +576,7 @@ html[data-perf-disable-home-artwork-clip="true"] .home-lite-artwork .song-card-c /* Artist card */ .np-info-artist-image-wrap { width: 100%; - aspect-ratio: 16 / 10; + aspect-ratio: 1 / 1; border-radius: 12px; overflow: hidden; background: var(--bg-elevated);