diff --git a/ui/src/album/AlbumGridView.jsx b/ui/src/album/AlbumGridView.jsx index e90e7a77..c8a16157 100644 --- a/ui/src/album/AlbumGridView.jsx +++ b/ui/src/album/AlbumGridView.jsx @@ -18,6 +18,7 @@ import { PlayButton, ArtistLinkField, OverflowTooltip, + useImageUrl, } from '../common' import { COVER_ART_SIZE, DraggableTypes } from '../consts' import clsx from 'clsx' @@ -105,7 +106,7 @@ const useCoverStyles = makeStyles({ transition: 'opacity 0.3s ease-in-out', }, coverLoading: { - opacity: 0.5, + opacity: 0, }, }) @@ -125,8 +126,6 @@ const Cover = withContentRect('bounds')(({ // Force height to be the same as the width determined by the GridList // noinspection JSSuspiciousNameCombination const classes = useCoverStyles({ height: contentRect.bounds.width }) - const [imageLoading, setImageLoading] = React.useState(true) - const [imageError, setImageError] = React.useState(false) const [, dragAlbumRef] = useDrag( () => ({ type: DraggableTypes.ALBUM, @@ -136,32 +135,16 @@ const Cover = withContentRect('bounds')(({ [record], ) - // Reset image state when record changes - React.useEffect(() => { - setImageLoading(true) - setImageError(false) - }, [record.id]) - - const handleImageLoad = React.useCallback(() => { - setImageLoading(false) - setImageError(false) - }, []) - - const handleImageError = React.useCallback(() => { - setImageLoading(false) - setImageError(true) - }, []) + const url = subsonic.getCoverArtUrl(record, COVER_ART_SIZE, true) + const { imgUrl, loading: imageLoading } = useImageUrl(url) return (
{record.name}
diff --git a/ui/src/album/AlbumList.jsx b/ui/src/album/AlbumList.jsx index d00d9770..28a24981 100644 --- a/ui/src/album/AlbumList.jsx +++ b/ui/src/album/AlbumList.jsx @@ -175,12 +175,12 @@ const AlbumListTitle = ({ albumListType }) => { return } -const AlbumListPagination = (props) => { +const AlbumListPagination = ({ albumListType, ...rest }) => { const { loading } = useListContext() - if (loading) { + if (loading && albumListType === 'random') { return null } - return <Pagination {...props} /> + return <Pagination {...rest} /> } const randomStartingSeed = Math.random().toString() @@ -243,7 +243,12 @@ const AlbumList = (props) => { actions={<AlbumListActions />} filters={<AlbumFilter />} perPage={perPage} - pagination={<AlbumListPagination rowsPerPageOptions={perPageOptions} />} + pagination={ + <AlbumListPagination + rowsPerPageOptions={perPageOptions} + albumListType={albumListType} + /> + } title={<AlbumListTitle albumListType={albumListType} />} > {albumView.grid ? ( diff --git a/ui/src/common/CoverArtAvatar.jsx b/ui/src/common/CoverArtAvatar.jsx index 5642f250..bc6dd8a3 100644 --- a/ui/src/common/CoverArtAvatar.jsx +++ b/ui/src/common/CoverArtAvatar.jsx @@ -4,12 +4,16 @@ import { makeStyles } from '@material-ui/core/styles' import clsx from 'clsx' import { COVER_ART_SIZE } from '../consts' import subsonic from '../subsonic' +import { useImageUrl } from './useImageUrl' const useStyles = makeStyles({ avatar: { width: '55px', height: '55px', }, + avatarEmpty: { + backgroundColor: 'transparent', + }, square: { borderRadius: '4px', }, @@ -22,15 +26,26 @@ export const CoverArtAvatar = ({ const classes = useStyles() const recordContext = useRecordContext() const record = recordProp || recordContext - if (!record) return null const square = variant !== 'circular' + const url = record + ? subsonic.getCoverArtUrl(record, COVER_ART_SIZE, square) + : null + const { imgUrl } = useImageUrl(url) + if (!record) return null return ( <Avatar - src={subsonic.getCoverArtUrl(record, COVER_ART_SIZE, square)} + src={imgUrl || undefined} variant={variant} - className={clsx(classes.avatar, square && classes.square)} + className={clsx( + classes.avatar, + square && classes.square, + !imgUrl && classes.avatarEmpty, + )} alt={record.name} - /> + > + {/* Empty child prevents default person icon while loading */} + {!imgUrl && <span />} + </Avatar> ) } diff --git a/ui/src/common/index.js b/ui/src/common/index.js index a7d6a43c..362a0ced 100644 --- a/ui/src/common/index.js +++ b/ui/src/common/index.js @@ -46,3 +46,4 @@ export * from './useSearchRefocus' export * from './ImageUploadOverlay' export * from './CoverArtAvatar' export * from './useImageLoadingState' +export * from './useImageUrl' diff --git a/ui/src/common/useImageUrl.js b/ui/src/common/useImageUrl.js new file mode 100644 index 00000000..9bcc70d7 --- /dev/null +++ b/ui/src/common/useImageUrl.js @@ -0,0 +1,144 @@ +import { useEffect, useState, useRef } from 'react' + +// Persists across component mount/unmount cycles so that +// React Admin refreshes (which remount list items) don't re-fetch images. +const cache = new Map() +const MAX_CACHE_SIZE = 300 + +// Limit concurrent fetches to leave browser connections free for API requests. +// Browsers allow ~6 connections per origin on HTTP/1.1; reserving 2 for API +// calls prevents image fetches from blocking pagination/data requests. +const MAX_CONCURRENT = 4 +let activeFetches = 0 +const pendingQueue = [] + +const processQueue = () => { + while (pendingQueue.length > 0 && activeFetches < MAX_CONCURRENT) { + const next = pendingQueue.shift() + next() + } +} + +// Evicts oldest unused entries (Map iterates in insertion order). +const evictIfNeeded = () => { + if (cache.size <= MAX_CACHE_SIZE) return + for (const [key, entry] of cache) { + if (cache.size <= MAX_CACHE_SIZE) break + if (entry.refCount === 0) { + if (entry.blobUrl) URL.revokeObjectURL(entry.blobUrl) + cache.delete(key) + } + } +} + +/** + * Loads an image via fetch() with AbortController so that in-flight requests + * are canceled on unmount (e.g., during pagination). Uses a module-level cache + * so remounting returns the cached blob URL instantly. + */ +export const useImageUrl = (url) => { + const cached = url ? cache.get(url) : null + const [imgUrl, setImgUrl] = useState(cached?.blobUrl || null) + const [loading, setLoading] = useState(!!url && !cached) + const [error, setError] = useState(cached?.error || false) + const abortedRef = useRef(false) + + useEffect(() => { + abortedRef.current = false + + if (!url) { + setImgUrl(null) + setLoading(false) + setError(false) + return + } + + // Re-check: another component's effect may have populated the cache + // between this component's render and effect execution. + const entry = cache.get(url) + if (entry) { + entry.refCount++ + setImgUrl(entry.blobUrl) + setLoading(false) + setError(entry.error || false) + return () => { + entry.refCount-- + } + } + + const controller = new AbortController() + let queued = true + setImgUrl(null) + setLoading(true) + setError(false) + + const doFetch = () => { + queued = false + activeFetches++ + fetch(url, { signal: controller.signal }) + .then((res) => { + if (!res.ok) { + throw new Error(`HTTP ${res.status}`) + } + return res.blob() + }) + .then((blob) => { + activeFetches-- + processQueue() + // Guard against late resolution after abort + if (abortedRef.current) { + return + } + const objectUrl = URL.createObjectURL(blob) + // Handle concurrent fetches: if another component already cached + // this URL, use its entry and discard our blob. + const existing = cache.get(url) + if (existing && existing.blobUrl) { + existing.refCount++ + URL.revokeObjectURL(objectUrl) + setImgUrl(existing.blobUrl) + } else { + cache.set(url, { blobUrl: objectUrl, refCount: 1 }) + evictIfNeeded() + setImgUrl(objectUrl) + } + setLoading(false) + }) + .catch((err) => { + activeFetches-- + processQueue() + if (err.name === 'AbortError') { + return // Expected on unmount or URL change + } + // Cache the error so repeated mounts don't re-fetch broken URLs + cache.set(url, { blobUrl: null, error: true, refCount: 0 }) + setError(true) + setLoading(false) + }) + } + + if (activeFetches < MAX_CONCURRENT) { + queued = false + doFetch() + } else { + pendingQueue.push(doFetch) + } + + return () => { + abortedRef.current = true + if (queued) { + // Remove from queue if not yet started + const idx = pendingQueue.indexOf(doFetch) + if (idx !== -1) pendingQueue.splice(idx, 1) + } else { + controller.abort() + } + const entry = cache.get(url) + if (entry) { + entry.refCount-- + } + } + }, [url]) + + return { imgUrl, loading, error } +} diff --git a/ui/src/common/useImageUrl.test.js b/ui/src/common/useImageUrl.test.js new file mode 100644 index 00000000..97631710 --- /dev/null +++ b/ui/src/common/useImageUrl.test.js @@ -0,0 +1,234 @@ +import { renderHook, act } from '@testing-library/react-hooks' +import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest' + +// Helper to flush all pending promises +const flushPromises = () => new Promise((resolve) => setTimeout(resolve, 0)) + +// We need a fresh module for each test to reset the module-level cache +let useImageUrl + +describe('useImageUrl', () => { + let abortSpy + let OriginalAbortController + let originalCreateObjectURL + let originalRevokeObjectURL + let originalFetch + + beforeEach(async () => { + // Reset module to clear the cache + vi.resetModules() + const mod = await import('./useImageUrl') + useImageUrl = mod.useImageUrl + + abortSpy = vi.fn() + OriginalAbortController = global.AbortController + originalCreateObjectURL = global.URL.createObjectURL + originalRevokeObjectURL = global.URL.revokeObjectURL + originalFetch = global.fetch + + global.AbortController = function () { + this.signal = 'mock-signal' + this.abort = abortSpy + } + global.URL.createObjectURL = vi.fn(() => 'blob:mock-url') + global.URL.revokeObjectURL = vi.fn() + }) + + afterEach(() => { + global.AbortController = OriginalAbortController + global.URL.createObjectURL = originalCreateObjectURL + global.URL.revokeObjectURL = originalRevokeObjectURL + global.fetch = originalFetch + vi.restoreAllMocks() + }) + + it('should return null values when url is null', () => { + const { result } = renderHook(() => useImageUrl(null)) + + expect(result.current.loading).toBe(false) + expect(result.current.imgUrl).toBeNull() + expect(result.current.error).toBe(false) + }) + + it('should return loading state initially', () => { + global.fetch = vi.fn(() => new Promise(() => {})) + const { result } = renderHook(() => + useImageUrl('http://example.com/img.jpg'), + ) + + expect(result.current.loading).toBe(true) + expect(result.current.imgUrl).toBeNull() + expect(result.current.error).toBe(false) + }) + + it('should fetch image and return blob URL on success', async () => { + const mockBlob = new Blob(['image-data'], { type: 'image/png' }) + global.fetch = vi.fn(() => + Promise.resolve({ + ok: true, + blob: () => Promise.resolve(mockBlob), + }), + ) + + const { result } = renderHook(() => + useImageUrl('http://example.com/img.jpg'), + ) + + await act(async () => { + await flushPromises() + }) + + expect(result.current.loading).toBe(false) + expect(result.current.imgUrl).toBe('blob:mock-url') + expect(result.current.error).toBe(false) + expect(global.fetch).toHaveBeenCalledWith('http://example.com/img.jpg', { + signal: 'mock-signal', + }) + }) + + it('should set error on HTTP failure', async () => { + global.fetch = vi.fn(() => Promise.resolve({ ok: false, status: 404 })) + + const { result } = renderHook(() => + useImageUrl('http://example.com/missing.jpg'), + ) + + await act(async () => { + await flushPromises() + }) + + expect(result.current.loading).toBe(false) + expect(result.current.imgUrl).toBeNull() + expect(result.current.error).toBe(true) + }) + + it('should abort fetch on unmount', async () => { + global.fetch = vi.fn(() => new Promise(() => {})) + + const { unmount } = renderHook(() => + useImageUrl('http://example.com/img.jpg'), + ) + + await act(async () => { + await flushPromises() + }) + + unmount() + + expect(abortSpy).toHaveBeenCalled() + }) + + it('should abort previous fetch when URL changes', async () => { + const abortSpies = [] + global.AbortController = function () { + const spy = vi.fn() + abortSpies.push(spy) + this.signal = `signal-${abortSpies.length}` + this.abort = spy + } + + const mockBlob = new Blob(['data'], { type: 'image/png' }) + global.fetch = vi.fn(() => + Promise.resolve({ + ok: true, + blob: () => Promise.resolve(mockBlob), + }), + ) + + const { rerender } = renderHook(({ url }) => useImageUrl(url), { + initialProps: { url: 'http://example.com/img1.jpg' }, + }) + + await act(async () => { + await flushPromises() + }) + + // Change URL - should abort the first controller + rerender({ url: 'http://example.com/img2.jpg' }) + + expect(abortSpies[0]).toHaveBeenCalled() + }) + + it('should not set error on AbortError', async () => { + const abortError = new DOMException('Aborted', 'AbortError') + global.fetch = vi.fn(() => Promise.reject(abortError)) + + const { result } = renderHook(() => + useImageUrl('http://example.com/img.jpg'), + ) + + await act(async () => { + await flushPromises() + }) + + expect(result.current.error).toBe(false) + }) + + it('should use cached blob URL on remount without re-fetching', async () => { + const mockBlob = new Blob(['data'], { type: 'image/png' }) + global.fetch = vi.fn(() => + Promise.resolve({ + ok: true, + blob: () => Promise.resolve(mockBlob), + }), + ) + + // First mount — fetches and caches + const { unmount } = renderHook(() => + useImageUrl('http://example.com/img.jpg'), + ) + + await act(async () => { + await flushPromises() + }) + + expect(global.fetch).toHaveBeenCalledTimes(1) + + // Unmount (simulates React Admin refresh) + unmount() + + // Remount with same URL — should use cache + const { result: result2 } = renderHook(() => + useImageUrl('http://example.com/img.jpg'), + ) + + await act(async () => { + await flushPromises() + }) + + // Should NOT have fetched again + expect(global.fetch).toHaveBeenCalledTimes(1) + expect(result2.current.imgUrl).toBe('blob:mock-url') + expect(result2.current.loading).toBe(false) + }) + + it('should cache errors and not re-fetch broken URLs', async () => { + global.fetch = vi.fn(() => Promise.resolve({ ok: false, status: 404 })) + + // First mount — fetch fails and error is cached + const { unmount } = renderHook(() => + useImageUrl('http://example.com/broken.jpg'), + ) + + await act(async () => { + await flushPromises() + }) + + expect(global.fetch).toHaveBeenCalledTimes(1) + unmount() + + // Remount with same URL — should use cached error, not re-fetch + const { result: result2 } = renderHook(() => + useImageUrl('http://example.com/broken.jpg'), + ) + + await act(async () => { + await flushPromises() + }) + + expect(global.fetch).toHaveBeenCalledTimes(1) + expect(result2.current.error).toBe(true) + expect(result2.current.imgUrl).toBeNull() + expect(result2.current.loading).toBe(false) + }) +}) diff --git a/ui/src/radio/RadioList.jsx b/ui/src/radio/RadioList.jsx index 582fcaff..945bac51 100644 --- a/ui/src/radio/RadioList.jsx +++ b/ui/src/radio/RadioList.jsx @@ -14,8 +14,12 @@ import { UrlField, useTranslate, } from 'react-admin' -import { List } from '../common' -import { ToggleFieldsMenu, useSelectedFields } from '../common' +import { + List, + useImageUrl, + ToggleFieldsMenu, + useSelectedFields, +} from '../common' import subsonic from '../subsonic' import { StreamField } from './StreamField' import { setTrack } from '../actions' @@ -78,10 +82,12 @@ const RadioListActions = ({ const avatarStyle = { width: 40, height: 40 } const CoverArtField = ({ record }) => { - if (!record) return null - const src = record.uploadedImage + const directUrl = record?.uploadedImage ? subsonic.getCoverArtUrl(record, 40, true) - : RADIO_PLACEHOLDER_IMAGE + : null + const { imgUrl } = useImageUrl(directUrl) + if (!record) return null + const src = imgUrl || RADIO_PLACEHOLDER_IMAGE return ( <Avatar src={src} variant="rounded" style={avatarStyle} alt={record.name} /> )