diff --git a/ui/src/common/Artwork.jsx b/ui/src/common/Artwork.jsx index 92557920b..94c45e011 100644 --- a/ui/src/common/Artwork.jsx +++ b/ui/src/common/Artwork.jsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react' +import { useEffect, useState } from 'react' import PropTypes from 'prop-types' import clsx from 'clsx' import { makeStyles } from '@material-ui/core/styles' @@ -47,13 +47,8 @@ export const Artwork = ({ }) => { const classes = useStyles() const url = record ? subsonic.getCoverArtUrl(record, size, square) : '' - const { imgUrl } = useImageUrl(url) + const { imgUrl, fromCache } = useImageUrl(url) - // A blob already cached at mount paints on the first frame, so it skips the fade. - const cachedOnMount = useRef(null) - if (cachedOnMount.current === null) { - cachedOnMount.current = !!imgUrl - } const [decoded, setDecoded] = useState(false) const [faded, setFaded] = useState(false) useEffect(() => { @@ -70,7 +65,8 @@ export const Artwork = ({ if (!record) return null - const instant = cachedOnMount.current + // A blob already cached paints on the first frame, so it skips the fade. + const instant = fromCache // Kept mounted until the fade ends; swapping on blob arrival would flash an empty container. const showPlaceholder = !!record.thumbHash && !instant && !faded // A square request is padded, not cropped, so `contain` keeps placeholder and image aligned. diff --git a/ui/src/common/Artwork.test.jsx b/ui/src/common/Artwork.test.jsx index c64153547..ed40823c8 100644 --- a/ui/src/common/Artwork.test.jsx +++ b/ui/src/common/Artwork.test.jsx @@ -3,7 +3,12 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' vi.mock('./useImageUrl', () => ({ useImageUrl: vi.fn() })) vi.mock('../subsonic', () => ({ - default: { getCoverArtUrl: () => '/rest/getCoverArt?id=al-1' }, + default: { + // Mirrors the real hash suffix, so a refreshed record yields a different URL. + getCoverArtUrl: (record) => + '/rest/getCoverArt?id=al-1' + + (record.imageHash ? `_${record.imageHash}` : ''), + }, })) vi.mock('../config', () => ({ default: { uiCoverArtSize: 300 } })) @@ -69,7 +74,11 @@ describe('Artwork', () => { // The padded square is aspect-fit, so `square` overrides fit="cover" as it does for the canvas. it('fits the image itself with contain when the server padded to a square', () => { - useImageUrl.mockReturnValue({ imgUrl: 'blob:abc', loading: false }) + useImageUrl.mockReturnValue({ + imgUrl: 'blob:abc', + loading: false, + fromCache: true, + }) const { container } = render( , ) @@ -95,7 +104,11 @@ describe('Artwork', () => { }) it('mounts the image only once its blob is ready', () => { - useImageUrl.mockReturnValue({ imgUrl: 'blob:abc', loading: false }) + useImageUrl.mockReturnValue({ + imgUrl: 'blob:abc', + loading: false, + fromCache: true, + }) const { container } = render() const img = container.querySelector('img') expect(img).not.toBeNull() @@ -108,7 +121,11 @@ describe('Artwork', () => { useImageUrl.mockReturnValue({ imgUrl: null, loading: true }) const { container, rerender } = render() - useImageUrl.mockReturnValue({ imgUrl: 'blob:abc', loading: false }) + useImageUrl.mockReturnValue({ + imgUrl: 'blob:abc', + loading: false, + fromCache: false, + }) rerender() const img = container.querySelector('img') expect(img).not.toBeNull() @@ -133,7 +150,11 @@ describe('Artwork', () => { }) it('does not fade an image that was already cached on mount', () => { - useImageUrl.mockReturnValue({ imgUrl: 'blob:abc', loading: false }) + useImageUrl.mockReturnValue({ + imgUrl: 'blob:abc', + loading: false, + fromCache: true, + }) const { container } = render() expect(container.querySelector('canvas')).toBeNull() expect(container.querySelector('img').className).toContain('imgInstant') @@ -142,7 +163,11 @@ describe('Artwork', () => { it('keeps the placeholder visible when the image never decodes', () => { useImageUrl.mockReturnValue({ imgUrl: null, loading: true }) const { container, rerender } = render() - useImageUrl.mockReturnValue({ imgUrl: 'blob:abc', loading: false }) + useImageUrl.mockReturnValue({ + imgUrl: 'blob:abc', + loading: false, + fromCache: false, + }) rerender() expect(container.querySelector('canvas')).not.toBeNull() @@ -157,9 +182,34 @@ describe('Artwork', () => { container.firstChild.click() expect(onClick).not.toHaveBeenCalled() - useImageUrl.mockReturnValue({ imgUrl: 'blob:abc', loading: false }) + useImageUrl.mockReturnValue({ + imgUrl: 'blob:abc', + loading: false, + fromCache: false, + }) rerender() container.firstChild.click() expect(onClick).toHaveBeenCalledTimes(1) }) + + it('falls back to the placeholder when a refresh swaps in an uncached hash', () => { + useImageUrl.mockReturnValue({ + imgUrl: 'blob:old', + loading: false, + fromCache: true, + }) + const cached = { ...withArt, imageHash: 'aaaa' } + const { container, rerender } = render() + expect(container.querySelector('canvas')).toBeNull() + + useImageUrl.mockReturnValue({ + imgUrl: null, + loading: true, + fromCache: false, + }) + rerender() + + expect(container.querySelector('img')).toBeNull() + expect(container.querySelector('canvas')).not.toBeNull() + }) }) diff --git a/ui/src/common/useImageUrl.js b/ui/src/common/useImageUrl.js index 9bcc70d74..736c599d6 100644 --- a/ui/src/common/useImageUrl.js +++ b/ui/src/common/useImageUrl.js @@ -43,6 +43,18 @@ export const useImageUrl = (url) => { const [error, setError] = useState(cached?.error || false) const abortedRef = useRef(false) + // Resync during render, not in the effect below: for one render otherwise imgUrl still holds the + // previous url's blob, so only fromCache can answer "was this url already cached when it started". + const [fromCache, setFromCache] = useState(!!cached) + const [trackedUrl, setTrackedUrl] = useState(url) + if (trackedUrl !== url) { + setTrackedUrl(url) + setImgUrl(cached?.blobUrl || null) + setLoading(!!url && !cached) + setError(cached?.error || false) + setFromCache(!!cached) + } + useEffect(() => { abortedRef.current = false @@ -140,5 +152,5 @@ export const useImageUrl = (url) => { } }, [url]) - return { imgUrl, loading, error } + return { imgUrl, loading, error, fromCache } }