diff --git a/ui/src/album/AlbumDetails.jsx b/ui/src/album/AlbumDetails.jsx index f796f3b9..8213eb9d 100644 --- a/ui/src/album/AlbumDetails.jsx +++ b/ui/src/album/AlbumDetails.jsx @@ -72,6 +72,10 @@ const useStyles = makeStyles( width: '15em', minWidth: '15em', }, + backgroundColor: 'transparent', + display: 'flex', + alignItems: 'center', + justifyContent: 'center', }, cover: { objectFit: 'contain', @@ -79,6 +83,11 @@ const useStyles = makeStyles( display: 'block', width: '100%', height: '100%', + backgroundColor: 'transparent', + transition: 'opacity 0.3s ease-in-out', + }, + coverLoading: { + opacity: 0.5, }, loveButton: { top: theme.spacing(-0.2), @@ -213,6 +222,8 @@ const AlbumDetails = (props) => { const [isLightboxOpen, setLightboxOpen] = useState(false) const [expanded, setExpanded] = useState(false) const [albumInfo, setAlbumInfo] = useState() + const [imageLoading, setImageLoading] = useState(false) + const [imageError, setImageError] = useState(false) let notes = albumInfo?.notes?.replace(new RegExp('<.*>', 'g'), '') || record.notes @@ -236,23 +247,51 @@ const AlbumDetails = (props) => { }) }, [record]) + // Reset image state when album changes + useEffect(() => { + setImageLoading(true) + setImageError(false) + }, [record.id]) + const imageUrl = subsonic.getCoverArtUrl(record, 300) const fullImageUrl = subsonic.getCoverArtUrl(record) - const handleOpenLightbox = useCallback(() => setLightboxOpen(true), []) + const handleImageLoad = useCallback(() => { + setImageLoading(false) + setImageError(false) + }, []) + + const handleImageError = useCallback(() => { + setImageLoading(false) + setImageError(true) + }, []) + + const handleOpenLightbox = useCallback(() => { + if (!imageError) { + setLightboxOpen(true) + } + }, [imageError]) + const handleCloseLightbox = useCallback(() => setLightboxOpen(false), []) + return (
@@ -337,7 +376,7 @@ const AlbumDetails = (props) => {
)} - {isLightboxOpen && ( + {isLightboxOpen && !imageError && ( props.height, + transition: 'opacity 0.3s ease-in-out', + }, + coverLoading: { + opacity: 0.5, }, }) @@ -113,6 +117,8 @@ 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, @@ -121,13 +127,33 @@ 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) + }, []) + return (
{record.name}
diff --git a/ui/src/artist/DesktopArtistDetails.jsx b/ui/src/artist/DesktopArtistDetails.jsx index 5d56bdef..b21005fc 100644 --- a/ui/src/artist/DesktopArtistDetails.jsx +++ b/ui/src/artist/DesktopArtistDetails.jsx @@ -38,11 +38,22 @@ const useStyles = makeStyles( height: '12rem', borderRadius: '6em', cursor: 'pointer', + backgroundColor: 'transparent', + transition: 'opacity 0.3s ease-in-out', + objectFit: 'cover', + }, + coverLoading: { + opacity: 0.5, }, artistImage: { maxHeight: '12rem', + minHeight: '12rem', + width: '12rem', + minWidth: '12rem', backgroundColor: 'inherit', display: 'flex', + alignItems: 'center', + justifyContent: 'center', boxShadow: 'none', }, artistDetail: { @@ -73,8 +84,31 @@ const DesktopArtistDetails = ({ artistInfo, record, biography }) => { const classes = useStyles() const title = record.name const [isLightboxOpen, setLightboxOpen] = React.useState(false) + const [imageLoading, setImageLoading] = React.useState(false) + const [imageError, setImageError] = React.useState(false) + + // Reset image state when artist 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 handleOpenLightbox = React.useCallback(() => { + if (!imageError) { + setLightboxOpen(true) + } + }, [imageError]) - const handleOpenLightbox = React.useCallback(() => setLightboxOpen(true), []) const handleCloseLightbox = React.useCallback( () => setLightboxOpen(false), [], @@ -86,10 +120,17 @@ const DesktopArtistDetails = ({ artistInfo, record, biography }) => { {artistInfo && ( )} @@ -140,7 +181,7 @@ const DesktopArtistDetails = ({ artistInfo, record, biography }) => { )}
- {isLightboxOpen && ( + {isLightboxOpen && !imageError && ( { const classes = useStyles({ img, expanded }) const title = record.name const [isLightboxOpen, setLightboxOpen] = React.useState(false) + const [imageLoading, setImageLoading] = React.useState(false) + const [imageError, setImageError] = React.useState(false) + + // Reset image state when artist 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 handleOpenLightbox = React.useCallback(() => { + if (!imageError) { + setLightboxOpen(true) + } + }, [imageError]) - const handleOpenLightbox = React.useCallback(() => setLightboxOpen(true), []) const handleCloseLightbox = React.useCallback( () => setLightboxOpen(false), [], @@ -95,10 +124,17 @@ const MobileArtistDetails = ({ artistInfo, biography, record }) => { {artistInfo && ( )} @@ -136,7 +172,7 @@ const MobileArtistDetails = ({ artistInfo, biography, record }) => { - {isLightboxOpen && ( + {isLightboxOpen && !imageError && ( { ...(square && { square }), } - // For playlists, add a timestamp to prevent caching issues when switching between playlists - if (record.songCount !== undefined) { - // Add current timestamp to ensure fresh requests for playlists - options._ = record.updatedAt || new Date().getTime() - } - - // TODO Move this logic to server. `song` and `album` should have a CoverArtID + // TODO Move this logic to server if (record.album) { return baseUrl(url('getCoverArt', 'mf-' + record.id, options)) } else if (record.albumArtist) { return baseUrl(url('getCoverArt', 'al-' + record.id, options)) - } else if (record.songCount !== undefined) { + } else if (record.sync !== undefined) { // This is a playlist return baseUrl(url('getCoverArt', 'pl-' + record.id, options)) } else { diff --git a/ui/src/subsonic/index.test.js b/ui/src/subsonic/index.test.js index fac8d6dc..6b902dfb 100644 --- a/ui/src/subsonic/index.test.js +++ b/ui/src/subsonic/index.test.js @@ -23,10 +23,10 @@ describe('getCoverArtUrl', () => { Object.defineProperty(window, 'localStorage', { value: localStorageMock }) }) - it('should return playlist cover art URL for records with songCount', () => { + it('should return playlist cover art URL for records with sync property', () => { const playlistRecord = { id: 'playlist-123', - songCount: 10, + sync: true, updatedAt: '2023-01-01T00:00:00Z', } @@ -41,13 +41,15 @@ describe('getCoverArtUrl', () => { it('should add timestamp for playlists without updatedAt', () => { const playlistRecord = { id: 'playlist-123', - songCount: 5, + sync: true, } - const url = subsonic.getCoverArtUrl(playlistRecord) + const url = subsonic.getCoverArtUrl(playlistRecord, 300, true) expect(url).toContain('pl-playlist-123') - expect(url).toMatch(/_=\d+/) + expect(url).toContain('size=300') + expect(url).toContain('square=true') + expect(url).not.toContain('_=') }) it('should return album cover art URL for records with albumArtist', () => { @@ -57,23 +59,25 @@ describe('getCoverArtUrl', () => { updatedAt: '2023-01-01T00:00:00Z', } - const url = subsonic.getCoverArtUrl(albumRecord, 300) + const url = subsonic.getCoverArtUrl(albumRecord, 300, true) expect(url).toContain('al-album-123') expect(url).toContain('size=300') + expect(url).toContain('square=true') }) it('should return media file cover art URL for records with album', () => { - const mediaFileRecord = { - id: 'mf-123', + const songRecord = { + id: 'song-123', album: 'Test Album', updatedAt: '2023-01-01T00:00:00Z', } - const url = subsonic.getCoverArtUrl(mediaFileRecord, 200) + const url = subsonic.getCoverArtUrl(songRecord, 300, true) - expect(url).toContain('mf-mf-123') - expect(url).toContain('size=200') + expect(url).toContain('mf-song-123') + expect(url).toContain('size=300') + expect(url).toContain('square=true') }) it('should return artist cover art URL for other records', () => { @@ -82,10 +86,11 @@ describe('getCoverArtUrl', () => { updatedAt: '2023-01-01T00:00:00Z', } - const url = subsonic.getCoverArtUrl(artistRecord, 150) + const url = subsonic.getCoverArtUrl(artistRecord, 300, true) expect(url).toContain('ar-artist-123') - expect(url).toContain('size=150') + expect(url).toContain('size=300') + expect(url).toContain('square=true') }) it('should handle records without updatedAt', () => {