From 107abda97e1ca13b3f2fefd04f608026f8bcc643 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 08:39:41 -0400 Subject: [PATCH] fix(flutter): album/artist covers in artist detail + tighter grid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drift cache intentionally drops cover_url (server-derived). The adapters comment says "REST cold-cache fallback briefly shows the real values before drift takes over" — but once drift takes over, covers are empty forever. Fix per source: - Album cover: server constructs the URL deterministically as /api/albums//cover (internal/api/convert.go:69). Mirror that in CachedAlbumAdapter.toRef so AlbumRef.coverUrl is non-empty whether the row came from a fresh fetch or a drift hit. Restores cover art on the artist detail album grid (and any other surface reading albums from drift). - Artist cover: server picks a representative album and reuses its cover (convert.go:98). Drift doesn't store the pointer, so derive client-side via the artist's first loaded album. New _ArtistAvatar prefers a non-empty server-emitted coverUrl and falls back to /api/albums//cover, then slate while the album list is still loading. Album grid spacing was off because childAspectRatio: 0.8 inflated each cell taller than the AlbumCard's actual ~160dp footprint, leaving a visible gap below every card. Switch to mainAxisExtent: 168 with explicit 8dp main/cross spacing — cells now match the card and sit on a clean grid. --- flutter_client/lib/cache/adapters.dart | 6 ++ .../lib/library/artist_detail_screen.dart | 60 ++++++++++++++++--- 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/flutter_client/lib/cache/adapters.dart b/flutter_client/lib/cache/adapters.dart index 49bbe7dd..ba19be86 100644 --- a/flutter_client/lib/cache/adapters.dart +++ b/flutter_client/lib/cache/adapters.dart @@ -34,12 +34,18 @@ extension ArtistRefDriftWrite on ArtistRef { extension CachedAlbumAdapter on CachedAlbum { /// `artistName` is supplied by the joined CachedArtists row at query time. + /// `coverUrl` is reconstructed deterministically from the album id — + /// the server emits the same shape (see internal/api/convert.go:69). + /// We don't need to persist it, so AlbumRef.coverUrl is non-empty + /// even when the row was populated from a sync that didn't carry the + /// derived URL. AlbumRef toRef({String artistName = ''}) => AlbumRef( id: id, title: title, sortTitle: sortTitle, artistId: artistId, artistName: artistName, + coverUrl: '/api/albums/$id/cover', ); } diff --git a/flutter_client/lib/library/artist_detail_screen.dart b/flutter_client/lib/library/artist_detail_screen.dart index 27c11554..45178f6d 100644 --- a/flutter_client/lib/library/artist_detail_screen.dart +++ b/flutter_client/lib/library/artist_detail_screen.dart @@ -35,13 +35,16 @@ class ArtistDetailScreen extends ConsumerWidget { child: SizedBox( width: 96, height: 96, - child: a.coverUrl.isEmpty - ? Container(color: fs.slate) - : ServerImage( - url: a.coverUrl, - fit: BoxFit.cover, - fallback: Container(color: fs.slate), - ), + // Server derives artist cover from a representative + // album. Drift cache doesn't persist that pointer, so + // mirror the trick client-side: reuse the first album + // returned by artistAlbumsProvider. Falls back to + // slate while albums are loading or empty. + child: _ArtistAvatar( + serverCoverUrl: a.coverUrl, + albums: albums, + fs: fs, + ), ), ), const SizedBox(width: 16), @@ -82,9 +85,16 @@ class ArtistDetailScreen extends ConsumerWidget { shrinkWrap: true, physics: const NeverScrollableScrollPhysics(), padding: const EdgeInsets.all(8), + // mainAxisExtent locks each cell's height to the actual + // AlbumCard footprint (124 cover + 8 + 14 title + 12 + // artist + ~6 padding), instead of letting + // childAspectRatio inflate it. Removes the visible vertical + // gap below each card. gridDelegate: const SliverGridDelegateWithFixedCrossAxisCount( crossAxisCount: 2, - childAspectRatio: 0.8, + mainAxisExtent: 168, + mainAxisSpacing: 8, + crossAxisSpacing: 8, ), itemCount: list.length, itemBuilder: (_, i) { @@ -98,3 +108,37 @@ class ArtistDetailScreen extends ConsumerWidget { ); } } + +/// Renders the artist's avatar. Server-emitted coverUrl wins when +/// non-empty; otherwise we mirror the server's "use the first album's +/// cover" rule client-side via the loaded album list. +class _ArtistAvatar extends StatelessWidget { + const _ArtistAvatar({ + required this.serverCoverUrl, + required this.albums, + required this.fs, + }); + final String serverCoverUrl; + final AsyncValue> albums; + final FabledSwordTheme fs; + + @override + Widget build(BuildContext context) { + if (serverCoverUrl.isNotEmpty) { + return ServerImage( + url: serverCoverUrl, + fit: BoxFit.cover, + fallback: Container(color: fs.slate), + ); + } + final firstId = albums.value?.isNotEmpty == true ? albums.value!.first.id : null; + if (firstId == null) { + return Container(color: fs.slate); + } + return ServerImage( + url: '/api/albums/$firstId/cover', + fit: BoxFit.cover, + fallback: Container(color: fs.slate), + ); + } +}