fix(ui): address CoverImage review findings

Restructure CoverImage so the size/shape lives on the root and the blurhash + image are absolute fills: the <img> mounts only once its blob is ready, so an unresolved cover never flashes a broken <img>. Add a fit prop (default cover) so album/playlist detail keep their letterbox instead of being cropped by a hardcoded object-fit. Remove the orphaned coverLoading styles and an unused subsonic import; add a CoverImage unit test.

ui/src/album/AlbumDetails.jsx

@@ -87,9 +87,6 @@ const useStyles = makeStyles(
       backgroundColor: 'transparent',
       transition: 'opacity 0.3s ease-in-out',
     },
-    coverLoading: {
-      opacity: 0.5,
-    },
     loveButton: {
       top: theme.spacing(-0.2),
       left: theme.spacing(0.5),
@@ -253,6 +250,7 @@ const AlbumDetails = (props) => {
         <div className={classes.coverParent}>
           <CoverImage
             record={record}
+            fit="contain"
             className={classes.cover}
             title={record.name}
             onClick={() => setLightboxOpen(true)}

ui/src/album/AlbumGridView.jsx

@@ -106,9 +106,6 @@ const useCoverStyles = makeStyles({
     height: (props) => props.height,
     transition: 'opacity 0.3s ease-in-out',
   },
-  coverLoading: {
-    opacity: 0,
-  },
 })
 
 const getColsForWidth = (width) => {

ui/src/artist/DesktopArtistDetails.jsx

@@ -44,9 +44,6 @@ const useStyles = makeStyles(
       transition: 'opacity 0.3s ease-in-out',
       objectFit: 'cover',
     },
-    coverLoading: {
-      opacity: 0.5,
-    },
     artistImage: {
       maxHeight: '12rem',
       minHeight: '12rem',

ui/src/artist/MobileArtistDetails.jsx

@@ -55,9 +55,6 @@ const useStyles = makeStyles(
       transition: 'opacity 0.3s ease-in-out',
       objectFit: 'cover',
     },
-    coverLoading: {
-      opacity: 0.5,
-    },
     artistImage: {
       marginLeft: '1em',
       maxHeight: '7rem',

ui/src/common/CoverArtAvatar.jsx

@@ -17,7 +17,10 @@ const useStyles = makeStyles({
   },
 })
 
-export const CoverArtAvatar = ({ record: recordProp, variant = 'circular' }) => {
+export const CoverArtAvatar = ({
+  record: recordProp,
+  variant = 'circular',
+}) => {
   const classes = useStyles()
   const recordContext = useRecordContext()
   const record = recordProp || recordContext
@@ -28,7 +31,10 @@ export const CoverArtAvatar = ({ record: recordProp, variant = 'circular' }) =>
       record={record}
       size={config.uiCoverArtSize}
       square={square}
-      className={clsx(classes.avatar, square ? classes.square : classes.circular)}
+      className={clsx(
+        classes.avatar,
+        square ? classes.square : classes.circular,
+      )}
       title={record.name}
     />
   )

ui/src/common/CoverImage.jsx

@@ -7,18 +7,31 @@ import { useImageUrl } from './useImageUrl'
 import { BlurHashCanvas } from './BlurHashCanvas'
 
 const useStyles = makeStyles({
-  root: { position: 'relative', display: 'inline-flex' },
-  blur: { position: 'absolute', top: 0, left: 0, zIndex: 0 },
-  img: { position: 'relative', zIndex: 1 },
+  // className supplies the size and shape; overflow:hidden clips the fills to a rounded shape.
+  root: {
+    position: 'relative',
+    display: 'inline-flex',
+    overflow: 'hidden',
+  },
+  fill: {
+    position: 'absolute',
+    top: 0,
+    left: 0,
+    width: '100%',
+    height: '100%',
+  },
+  '@keyframes fadeIn': { from: { opacity: 0 }, to: { opacity: 1 } },
+  img: { animation: '$fadeIn 0.3s ease-in-out' },
 })
 
 // CoverImage renders an entity's cover through the shared useImageUrl blob cache, so it survives
-// React remounts without re-fetching, with the blurhash as the loading placeholder. `className`
-// supplies the size (and any transition); the fade opacity is applied here.
+// React remounts without re-fetching. The blurhash is the loading placeholder; the image is only
+// mounted once its blob is ready, so an unresolved cover never renders as a broken <img>.
 export const CoverImage = ({
   record,
   size = config.uiCoverArtSize,
   square = false,
+  fit = 'cover',
   className,
   title,
   onClick,
@@ -31,25 +44,23 @@ export const CoverImage = ({
   const showBlurHash = loading && record.blurHash
   const handleClick = imgUrl && onClick ? onClick : undefined
   return (
-    <div className={classes.root}>
+    <div
+      className={clsx(classes.root, className)}
+      onClick={handleClick}
+      style={{ cursor: handleClick ? 'pointer' : 'default' }}
+    >
       {showBlurHash && (
-        <BlurHashCanvas
-          hash={record.blurHash}
-          className={clsx(className, classes.blur)}
+        <BlurHashCanvas hash={record.blurHash} className={classes.fill} />
+      )}
+      {imgUrl && (
+        <img
+          src={imgUrl}
+          alt={title}
+          title={title}
+          className={clsx(classes.fill, classes.img)}
+          style={{ objectFit: fit }}
         />
       )}
-      <img
-        src={imgUrl || undefined}
-        alt={title}
-        title={title}
-        onClick={handleClick}
-        className={clsx(className, showBlurHash && classes.img)}
-        style={{
-          objectFit: 'cover',
-          opacity: loading ? 0.5 : 1,
-          cursor: handleClick ? 'pointer' : 'default',
-        }}
-      />
     </div>
   )
 }
@@ -58,6 +69,7 @@ CoverImage.propTypes = {
   record: PropTypes.object,
   size: PropTypes.number,
   square: PropTypes.bool,
+  fit: PropTypes.oneOf(['cover', 'contain']),
   className: PropTypes.string,
   title: PropTypes.string,
   onClick: PropTypes.func,

ui/src/common/CoverImage.test.jsx

@@ -0,0 +1,68 @@
+import { render } from '@testing-library/react'
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+
+vi.mock('./useImageUrl', () => ({ useImageUrl: vi.fn() }))
+vi.mock('../subsonic', () => ({
+  default: { getCoverArtUrl: () => '/rest/getCoverArt?id=al-1' },
+}))
+vi.mock('../config', () => ({ default: { uiCoverArtSize: 300 } }))
+
+import { useImageUrl } from './useImageUrl'
+import { CoverImage } from './CoverImage'
+
+const withArt = {
+  id: 'al-1',
+  name: 'Album',
+  blurHash: 'LEHV6nWB2yk8pyo0adR*.7kCMdnj',
+}
+
+describe('CoverImage', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    // jsdom has no 2D context; stub it so BlurHashCanvas bails cleanly without console noise
+    HTMLCanvasElement.prototype.getContext = vi.fn(() => null)
+  })
+
+  it('renders nothing without a record', () => {
+    useImageUrl.mockReturnValue({ imgUrl: null, loading: false })
+    const { container } = render(<CoverImage record={null} />)
+    expect(container.firstChild).toBeNull()
+  })
+
+  it('shows the blurhash and no <img> while loading', () => {
+    useImageUrl.mockReturnValue({ imgUrl: null, loading: true })
+    const { container } = render(<CoverImage record={withArt} title="Album" />)
+    expect(container.querySelector('canvas')).not.toBeNull()
+    expect(container.querySelector('img')).toBeNull()
+  })
+
+  it('shows neither a broken <img> nor a canvas while loading a record with no blurhash', () => {
+    useImageUrl.mockReturnValue({ imgUrl: null, loading: true })
+    const { container } = render(<CoverImage record={{ id: 'al-2', name: 'X' }} />)
+    expect(container.querySelector('img')).toBeNull()
+    expect(container.querySelector('canvas')).toBeNull()
+  })
+
+  it('mounts the image only once its blob is ready', () => {
+    useImageUrl.mockReturnValue({ imgUrl: 'blob:abc', loading: false })
+    const { container } = render(<CoverImage record={withArt} title="Album" />)
+    const img = container.querySelector('img')
+    expect(img).not.toBeNull()
+    expect(img.getAttribute('src')).toBe('blob:abc')
+  })
+
+  it('fires onClick only when the image is loaded', () => {
+    const onClick = vi.fn()
+    useImageUrl.mockReturnValue({ imgUrl: null, loading: true })
+    const { container, rerender } = render(
+      <CoverImage record={withArt} onClick={onClick} />,
+    )
+    container.firstChild.click()
+    expect(onClick).not.toHaveBeenCalled()
+
+    useImageUrl.mockReturnValue({ imgUrl: 'blob:abc', loading: false })
+    rerender(<CoverImage record={withArt} onClick={onClick} />)
+    container.firstChild.click()
+    expect(onClick).toHaveBeenCalledTimes(1)
+  })
+})

ui/src/playlist/PlaylistDetails.jsx

@@ -1,10 +1,5 @@
 import { useState } from 'react'
-import {
-  Card,
-  CardContent,
-  Typography,
-  useMediaQuery,
-} from '@material-ui/core'
+import { Card, CardContent, Typography, useMediaQuery } from '@material-ui/core'
 import { makeStyles } from '@material-ui/core/styles'
 import { useTranslate } from 'react-admin'
 import Lightbox from 'react-image-lightbox'
@@ -74,9 +69,6 @@ const useStyles = makeStyles(
       backgroundColor: 'transparent',
       transition: 'opacity 0.3s ease-in-out',
     },
-    coverLoading: {
-      opacity: 0.5,
-    },
     title: {
       overflow: 'hidden',
       textOverflow: 'ellipsis',
@@ -117,6 +109,7 @@ const PlaylistDetails = (props) => {
           <CoverImage
             record={record}
             square
+            fit="contain"
             className={classes.cover}
             title={record.name}
             onClick={() => setLightboxOpen(true)}

ui/src/radio/RadioList.jsx

@@ -22,7 +22,6 @@ import {
   useSelectedFields,
 } from '../common'
 import { CoverImage } from '../common/CoverImage'
-import subsonic from '../subsonic'
 import { StreamField } from './StreamField'
 import { setTrack } from '../actions'
 import { songFromRadio } from './helper'
@@ -103,7 +102,12 @@ const CoverArtField = ({ record }) => {
     )
   }
   return (
-    <Avatar src={RADIO_PLACEHOLDER_IMAGE} variant="rounded" style={avatarStyle} alt={record.name} />
+    <Avatar
+      src={RADIO_PLACEHOLDER_IMAGE}
+      variant="rounded"
+      style={avatarStyle}
+      alt={record.name}
+    />
   )
 }
 CoverArtField.defaultProps = { label: '' }