From 8966de6d0a99da630cb0c7d2f96646fa8ce3d753 Mon Sep 17 00:00:00 2001 From: zarzet Date: Tue, 14 Jul 2026 08:01:21 +0700 Subject: [PATCH] revert(ui): drop hero cover flights from home/search to detail screens The list-to-header cover flights read as a gimmick and add per-frame cost during navigation. Removed the heroTag plumbing end to end (recents, search rows, discography cards, Album/Artist/Downloaded headers, motion banners). Kept: the mini player expand hero, the library-to-track-metadata hero, real loading headers, and the loading crossfade. --- lib/screens/album_screen.dart | 17 +-- lib/screens/artist_screen.dart | 162 +++++++++-------------- lib/screens/downloaded_album_screen.dart | 5 - lib/screens/home_tab.dart | 38 ++---- lib/screens/home_tab_widgets.dart | 47 ++----- lib/widgets/album_detail_header.dart | 10 +- 6 files changed, 82 insertions(+), 197 deletions(-) diff --git a/lib/screens/album_screen.dart b/lib/screens/album_screen.dart index 64fbac66..30a48a25 100644 --- a/lib/screens/album_screen.dart +++ b/lib/screens/album_screen.dart @@ -48,9 +48,6 @@ class AlbumScreen extends ConsumerStatefulWidget { final String? artistId; final String? artistName; - /// Shared-element tag for the header cover (see [AlbumDetailHeader.heroTag]). - final Object? heroTag; - const AlbumScreen({ super.key, required this.albumId, @@ -63,7 +60,6 @@ class AlbumScreen extends ConsumerStatefulWidget { this.extensionId, this.artistId, this.artistName, - this.heroTag, }); @override @@ -398,26 +394,15 @@ class _AlbumScreenState extends ConsumerState ), ); - Widget background = hasMotion + final Widget background = hasMotion ? MotionHeaderBanner(videoUrl: motionUrl, fallback: headerBgImage()) : headerBgImage(); - // With a motion banner there is no square cover, so the Hero rides the - // full-bleed background instead; the shuttle flies the still image (a - // shuttle-built banner would restart the video mid-flight). - if (hasMotion && widget.heroTag != null) { - background = Hero( - tag: widget.heroTag!, - flightShuttleBuilder: (_, _, _, _, _) => headerBgImage(), - child: background, - ); - } return AlbumDetailHeader( title: widget.albumName, expandedHeight: expandedHeight, showTitleInAppBar: showTitleInAppBar, backgroundColor: pageBackgroundColor, - heroTag: widget.heroTag, background: background, blurAndScrimBackground: showSquareCover, coverBuilder: showSquareCover diff --git a/lib/screens/artist_screen.dart b/lib/screens/artist_screen.dart index 092cd211..4166acb0 100644 --- a/lib/screens/artist_screen.dart +++ b/lib/screens/artist_screen.dart @@ -88,10 +88,6 @@ class ArtistScreen extends ConsumerStatefulWidget { final List? topTracks; final String? extensionId; - /// Shared-element tag: when set, the header image flies from the list item - /// that pushed this screen (which wraps its thumbnail in a matching [Hero]). - final Object? heroTag; - const ArtistScreen({ super.key, required this.artistId, @@ -103,7 +99,6 @@ class ArtistScreen extends ConsumerStatefulWidget { this.albums, this.topTracks, this.extensionId, - this.heroTag, }); @override @@ -121,19 +116,6 @@ class _ArtistScreenState extends ConsumerState int? _monthlyListeners; String? _error; - /// [shuttle], when given, is flown instead of re-instantiating [child] - /// (needed for motion banners, which would restart their video mid-flight). - Widget _heroWrap(Widget child, {Widget Function()? shuttle}) => - widget.heroTag != null - ? Hero( - tag: widget.heroTag!, - flightShuttleBuilder: shuttle == null - ? null - : (_, _, _, _, _) => shuttle(), - child: child, - ) - : child; - bool _showTitleInAppBar = false; final ScrollController _scrollController = ScrollController(); final PageController _popularPageController = PageController(); @@ -1180,59 +1162,49 @@ class _ArtistScreenState extends ConsumerState fit: StackFit.expand, children: [ if (hasMotionBanner) - Builder( - builder: (context) { - Widget fallbackImage() => hasValidImage - ? CachedCoverImage( - imageUrl: imageUrl!, - fit: BoxFit.cover, - alignment: Alignment.topCenter, - memCacheWidth: 800, - placeholder: (context, url) => Container( - color: colorScheme.surfaceContainerHighest, - ), - errorWidget: (context, url, error) => Container( - color: colorScheme.surfaceContainerHighest, - child: Icon( - Icons.person, - size: 80, - color: colorScheme.onSurfaceVariant, - ), - ), - ) - : Container( + MotionHeaderBanner( + videoUrl: headerVideoUrl, + fallback: hasValidImage + ? CachedCoverImage( + imageUrl: imageUrl, + fit: BoxFit.cover, + alignment: Alignment.topCenter, + memCacheWidth: 800, + placeholder: (context, url) => Container( + color: colorScheme.surfaceContainerHighest, + ), + errorWidget: (context, url, error) => Container( color: colorScheme.surfaceContainerHighest, child: Icon( Icons.person, size: 80, color: colorScheme.onSurfaceVariant, ), - ); - return _heroWrap( - MotionHeaderBanner( - videoUrl: headerVideoUrl!, - fallback: fallbackImage(), - ), - shuttle: fallbackImage, - ); - }, + ), + ) + : Container( + color: colorScheme.surfaceContainerHighest, + child: Icon( + Icons.person, + size: 80, + color: colorScheme.onSurfaceVariant, + ), + ), ) else if (hasValidImage) - _heroWrap( - CachedCoverImage( - imageUrl: imageUrl, - fit: BoxFit.cover, - alignment: Alignment.topCenter, - memCacheWidth: 800, - placeholder: (context, url) => - Container(color: colorScheme.surfaceContainerHighest), - errorWidget: (context, url, error) => Container( - color: colorScheme.surfaceContainerHighest, - child: Icon( - Icons.person, - size: 80, - color: colorScheme.onSurfaceVariant, - ), + CachedCoverImage( + imageUrl: imageUrl, + fit: BoxFit.cover, + alignment: Alignment.topCenter, + memCacheWidth: 800, + placeholder: (context, url) => + Container(color: colorScheme.surfaceContainerHighest), + errorWidget: (context, url, error) => Container( + color: colorScheme.surfaceContainerHighest, + child: Icon( + Icons.person, + size: 80, + color: colorScheme.onSurfaceVariant, ), ), ) @@ -1733,7 +1705,6 @@ class _ArtistScreenState extends ConsumerState tileSize: tileSize, sectionHeight: sectionHeight, showTypeBadge: showTypeBadge, - sectionKey: title, ), ); }, @@ -1749,12 +1720,8 @@ class _ArtistScreenState extends ConsumerState required double tileSize, required double sectionHeight, bool showTypeBadge = false, - String sectionKey = '', }) { final isSelected = selectedIds.contains(album.id); - // The same album can appear in several sections (releases + its type - // bucket), so the section is part of the tag to keep it unique. - final albumHeroTag = 'discography-$sectionKey-${album.id}'; return Semantics( button: true, @@ -1767,7 +1734,7 @@ class _ArtistScreenState extends ConsumerState if (isSelectionMode) { toggleSelection(album.id); } else { - _navigateToAlbum(album, heroTag: albumHeroTag); + _navigateToAlbum(album); } }, onLongPress: () { @@ -1787,35 +1754,22 @@ class _ArtistScreenState extends ConsumerState child: Stack( fit: StackFit.expand, children: [ - Hero( - tag: albumHeroTag, - child: ClipRRect( - borderRadius: BorderRadius.circular(8), - child: album.coverUrl != null - ? CachedCoverImage( - imageUrl: album.coverUrl!, - width: tileSize, - height: tileSize, - fit: BoxFit.cover, - memCacheWidth: (tileSize * 2).round(), - memCacheHeight: (tileSize * 2).round(), - placeholder: (context, url) => ShimmerLoading( - child: Container( - color: colorScheme.surfaceContainerHighest, - ), + ClipRRect( + borderRadius: BorderRadius.circular(8), + child: album.coverUrl != null + ? CachedCoverImage( + imageUrl: album.coverUrl!, + width: tileSize, + height: tileSize, + fit: BoxFit.cover, + memCacheWidth: (tileSize * 2).round(), + memCacheHeight: (tileSize * 2).round(), + placeholder: (context, url) => ShimmerLoading( + child: Container( + color: colorScheme.surfaceContainerHighest, ), - errorWidget: (context, url, error) => - Container( - color: - colorScheme.surfaceContainerHighest, - child: Icon( - Icons.album, - color: colorScheme.onSurfaceVariant, - size: 40, - ), - ), - ) - : Container( + ), + errorWidget: (context, url, error) => Container( color: colorScheme.surfaceContainerHighest, child: Icon( Icons.album, @@ -1823,7 +1777,15 @@ class _ArtistScreenState extends ConsumerState size: 40, ), ), - ), + ) + : Container( + color: colorScheme.surfaceContainerHighest, + child: Icon( + Icons.album, + color: colorScheme.onSurfaceVariant, + size: 40, + ), + ), ), if (isSelectionMode) Positioned.fill( @@ -1924,7 +1886,7 @@ class _ArtistScreenState extends ConsumerState ); } - void _navigateToAlbum(ArtistAlbum album, {Object? heroTag}) { + void _navigateToAlbum(ArtistAlbum album) { ref.read(settingsProvider.notifier).setHasSearchedBefore(); if (album.providerId != null && album.providerId!.isNotEmpty) { @@ -1938,7 +1900,6 @@ class _ArtistScreenState extends ConsumerState coverUrl: album.coverUrl, initialAlbumType: album.albumType, initialTotalTracks: album.totalTracks, - heroTag: heroTag, ), ), ); @@ -1950,7 +1911,6 @@ class _ArtistScreenState extends ConsumerState albumId: album.id, albumName: album.name, coverUrl: album.coverUrl, - heroTag: heroTag, ), ), ); diff --git a/lib/screens/downloaded_album_screen.dart b/lib/screens/downloaded_album_screen.dart index bfc8e1e3..e837d3c5 100644 --- a/lib/screens/downloaded_album_screen.dart +++ b/lib/screens/downloaded_album_screen.dart @@ -35,15 +35,11 @@ class DownloadedAlbumScreen extends ConsumerStatefulWidget { final String artistName; final String? coverUrl; - /// Shared-element tag for the header cover (see [AlbumDetailHeader.heroTag]). - final Object? heroTag; - const DownloadedAlbumScreen({ super.key, required this.albumName, required this.artistName, this.coverUrl, - this.heroTag, }); @override @@ -369,7 +365,6 @@ class _DownloadedAlbumScreenState extends ConsumerState title: widget.albumName, expandedHeight: expandedHeight, showTitleInAppBar: showTitleInAppBar, - heroTag: widget.heroTag, background: background, blurAndScrimBackground: embeddedCoverPath != null || widget.coverUrl != null, diff --git a/lib/screens/home_tab.dart b/lib/screens/home_tab.dart index 362a715f..386e5041 100644 --- a/lib/screens/home_tab.dart +++ b/lib/screens/home_tab.dart @@ -2325,21 +2325,18 @@ class _HomeTabState extends ConsumerState padding: const EdgeInsets.symmetric(horizontal: 4, vertical: 8), child: Row( children: [ - Hero( - tag: _recentHeroTag(item), - child: _DownloadedOrRemoteCover( - downloadedFilePath: isDownloaded - ? downloadFilePathByRecentKey['${item.type.name}:${item.id}'] - : null, - imageUrl: item.imageUrl, - width: 56, - height: 56, - borderRadius: BorderRadius.circular( - item.type == RecentAccessType.artist ? 28 : 4, - ), - fallbackIcon: typeIcon, - colorScheme: colorScheme, + _DownloadedOrRemoteCover( + downloadedFilePath: isDownloaded + ? downloadFilePathByRecentKey['${item.type.name}:${item.id}'] + : null, + imageUrl: item.imageUrl, + width: 56, + height: 56, + borderRadius: BorderRadius.circular( + item.type == RecentAccessType.artist ? 28 : 4, ), + fallbackIcon: typeIcon, + colorScheme: colorScheme, ), const SizedBox(width: 12), Expanded( @@ -2411,13 +2408,9 @@ class _HomeTabState extends ConsumerState ); } - static String _recentHeroTag(RecentAccessItem item) => - 'recent-${item.type.name}-${item.id}'; - Future _navigateToRecentItem(RecentAccessItem item) async { _searchFocusNode.unfocus(); - final heroTag = _recentHeroTag(item); switch (item.type) { case RecentAccessType.artist: if (_isEnabledMetadataExtension(item.providerId)) { @@ -2429,7 +2422,6 @@ class _HomeTabState extends ConsumerState artistId: item.id, artistName: item.name, coverUrl: item.imageUrl, - heroTag: heroTag, ), ), ); @@ -2441,7 +2433,6 @@ class _HomeTabState extends ConsumerState artistId: item.id, artistName: item.name, coverUrl: item.imageUrl, - heroTag: heroTag, ), ), ); @@ -2456,7 +2447,6 @@ class _HomeTabState extends ConsumerState albumName: item.name, artistName: item.subtitle ?? '', coverUrl: item.imageUrl, - heroTag: heroTag, ), ), ); @@ -2469,7 +2459,6 @@ class _HomeTabState extends ConsumerState albumId: item.id, albumName: item.name, coverUrl: item.imageUrl, - heroTag: heroTag, ), ), ); @@ -2481,7 +2470,6 @@ class _HomeTabState extends ConsumerState albumId: item.id, albumName: item.name, coverUrl: item.imageUrl, - heroTag: heroTag, ), ), ); @@ -3256,7 +3244,6 @@ class _HomeTabState extends ConsumerState artistId: artistId, artistName: artistName, coverUrl: imageUrl, - heroTag: 'search-artist-$artistId', ), ), ); @@ -3284,7 +3271,6 @@ class _HomeTabState extends ConsumerState albumName: album.name, coverUrl: album.imageUrl, tracks: const [], - heroTag: 'search-album-${album.id}', ), ), ); @@ -3350,7 +3336,6 @@ class _HomeTabState extends ConsumerState coverUrl: albumItem.coverUrl, initialAlbumType: albumItem.albumType, initialTotalTracks: albumItem.totalTracks, - heroTag: 'search-album-${albumItem.id}', ), ), ); @@ -3422,7 +3407,6 @@ class _HomeTabState extends ConsumerState artistId: artistItem.id, artistName: artistItem.name, coverUrl: artistItem.coverUrl, - heroTag: 'search-artist-${artistItem.id}', ), ), ); diff --git a/lib/screens/home_tab_widgets.dart b/lib/screens/home_tab_widgets.dart index c8732cea..d232bc2e 100644 --- a/lib/screens/home_tab_widgets.dart +++ b/lib/screens/home_tab_widgets.dart @@ -450,12 +450,6 @@ class _CollectionItemWidget extends StatelessWidget { child: Icon(placeholderIcon, color: colorScheme.onSurfaceVariant), ), ); - // Matches the heroTag passed to the pushed detail screen; playlists have - // no hero destination. - final heroTag = isPlaylist - ? null - : (isArtist ? 'search-artist-${item.id}' : 'search-album-${item.id}'); - return Column( mainAxisSize: MainAxisSize.min, children: [ @@ -467,7 +461,7 @@ class _CollectionItemWidget extends StatelessWidget { padding: const EdgeInsets.symmetric(horizontal: 12, vertical: 10), child: Row( children: [ - heroTag != null ? Hero(tag: heroTag, child: cover) : cover, + cover, const SizedBox(width: 12), Expanded( child: Column( @@ -532,9 +526,6 @@ class _SearchResultRowItem extends StatelessWidget { final bool showDivider; final VoidCallback onTap; - /// Matches the heroTag passed to the pushed detail screen. - final Object? heroTag; - const _SearchResultRowItem({ required this.imageUrl, required this.coverBorderRadius, @@ -543,7 +534,6 @@ class _SearchResultRowItem extends StatelessWidget { required this.subtitle, required this.showDivider, required this.onTap, - this.heroTag, }); @override @@ -582,7 +572,7 @@ class _SearchResultRowItem extends StatelessWidget { padding: const EdgeInsets.symmetric(horizontal: 12, vertical: 10), child: Row( children: [ - heroTag != null ? Hero(tag: heroTag!, child: cover) : cover, + cover, const SizedBox(width: 12), Expanded( child: Column( @@ -643,7 +633,6 @@ class _SearchArtistItemWidget extends StatelessWidget { imageUrl: artist.imageUrl, coverBorderRadius: 28, fallbackIcon: Icons.person, - heroTag: 'search-artist-${artist.id}', title: artist.name, subtitle: Text( context.l10n.recentTypeArtist, @@ -679,7 +668,6 @@ class _SearchAlbumItemWidget extends StatelessWidget { imageUrl: album.imageUrl, coverBorderRadius: 10, fallbackIcon: Icons.album, - heroTag: 'search-album-${album.id}', title: album.name, subtitle: ClickableArtistName( artistName: album.artists.isNotEmpty @@ -960,19 +948,14 @@ mixin _RouteSettled on State { } } -/// Loading state for [ExtensionAlbumScreen]: the real collection header with -/// the Hero cover already in its final slot (so the flight from the tapped -/// list item lands correctly) above a shimmering track list. +/// Loading state for [ExtensionAlbumScreen]: the real collection header +/// (title + cover already in their final slots) above a shimmering track +/// list, so arriving content barely shifts the page. class _AlbumLoadingScaffold extends StatefulWidget { final String title; final String? coverUrl; - final Object? heroTag; - const _AlbumLoadingScaffold({ - required this.title, - required this.coverUrl, - required this.heroTag, - }); + const _AlbumLoadingScaffold({required this.title, required this.coverUrl}); @override State<_AlbumLoadingScaffold> createState() => _AlbumLoadingScaffoldState(); @@ -1006,7 +989,6 @@ class _AlbumLoadingScaffoldState extends State<_AlbumLoadingScaffold> title: widget.title, expandedHeight: calculateExpandedHeight(context), showTitleInAppBar: showTitleInAppBar, - heroTag: widget.heroTag, background: cover(), coverBuilder: (context, coverSize) => cover(), // Placeholder rows sized like the loaded header's subtitle @@ -1030,16 +1012,14 @@ class _AlbumLoadingScaffoldState extends State<_AlbumLoadingScaffold> } /// Loading state for [ExtensionArtistScreen]: the real full-bleed header with -/// the Hero cover and artist name above the discography skeleton. +/// the cover and artist name above the discography skeleton. class _ArtistLoadingScaffold extends StatelessWidget { final String artistName; final String? coverUrl; - final Object? heroTag; const _ArtistLoadingScaffold({ required this.artistName, required this.coverUrl, - required this.heroTag, }); @override @@ -1052,7 +1032,7 @@ class _ArtistLoadingScaffold extends StatelessWidget { url.isNotEmpty && Uri.tryParse(url)?.hasAuthority == true; - Widget image = hasImage + final Widget image = hasImage ? CachedCoverImage( imageUrl: url, fit: BoxFit.cover, @@ -1067,9 +1047,6 @@ class _ArtistLoadingScaffold extends StatelessWidget { color: colorScheme.onSurfaceVariant, ), ); - if (heroTag != null) { - image = Hero(tag: heroTag!, child: image); - } return Scaffold( body: CustomScrollView( @@ -1156,7 +1133,6 @@ class ExtensionAlbumScreen extends ConsumerStatefulWidget { final String? coverUrl; final String? initialAlbumType; final int? initialTotalTracks; - final Object? heroTag; const ExtensionAlbumScreen({ super.key, @@ -1166,7 +1142,6 @@ class ExtensionAlbumScreen extends ConsumerStatefulWidget { this.coverUrl, this.initialAlbumType, this.initialTotalTracks, - this.heroTag, }); @override @@ -1325,7 +1300,6 @@ class _ExtensionAlbumScreenState extends ConsumerState child: _AlbumLoadingScaffold( title: widget.albumName, coverUrl: widget.coverUrl, - heroTag: widget.heroTag, ), ); } @@ -1342,7 +1316,6 @@ class _ExtensionAlbumScreenState extends ConsumerState extensionId: widget.extensionId, artistId: _artistId, artistName: _artistName, - heroTag: widget.heroTag, ), ); } @@ -1492,7 +1465,6 @@ class ExtensionArtistScreen extends ConsumerStatefulWidget { final String artistId; final String artistName; final String? coverUrl; - final Object? heroTag; const ExtensionArtistScreen({ super.key, @@ -1500,7 +1472,6 @@ class ExtensionArtistScreen extends ConsumerStatefulWidget { required this.artistId, required this.artistName, this.coverUrl, - this.heroTag, }); @override @@ -1639,7 +1610,6 @@ class _ExtensionArtistScreenState extends ConsumerState child: _ArtistLoadingScaffold( artistName: widget.artistName, coverUrl: widget.coverUrl, - heroTag: widget.heroTag, ), ); } @@ -1655,7 +1625,6 @@ class _ExtensionArtistScreenState extends ConsumerState albums: _albums, topTracks: _topTracks, extensionId: widget.extensionId, // Skip Spotify/Deezer fetch - heroTag: widget.heroTag, ), ); } diff --git a/lib/widgets/album_detail_header.dart b/lib/widgets/album_detail_header.dart index 1ef764bc..31c64e69 100644 --- a/lib/widgets/album_detail_header.dart +++ b/lib/widgets/album_detail_header.dart @@ -23,7 +23,6 @@ class AlbumDetailHeader extends StatelessWidget { this.appBarTitle, this.leading, this.backgroundColor, - this.heroTag, }); final String title; @@ -62,10 +61,6 @@ class AlbumDetailHeader extends StatelessWidget { final Color? backgroundColor; - /// Shared-element tag: when set, the cover flies from the list item that - /// pushed this screen (which wraps its thumbnail in a matching [Hero]). - final Object? heroTag; - /// Shrinks long titles so up to three lines fit the header. double _titleFontSize() { final length = title.trim().length; @@ -154,7 +149,7 @@ class AlbumDetailHeader extends StatelessWidget { final coverSize = (constraints.maxWidth * 0.5) .clamp(150.0, 210.0) .toDouble(); - final cover = Container( + return Container( width: coverSize, height: coverSize, decoration: BoxDecoration( @@ -174,9 +169,6 @@ class AlbumDetailHeader extends StatelessWidget { child: coverBuilder!(context, coverSize), ), ); - return heroTag != null - ? Hero(tag: heroTag!, child: cover) - : cover; }, ), const SizedBox(height: 20),