From 2f9ba0b187b03e0652dbcd8ab9808b3579810e50 Mon Sep 17 00:00:00 2001 From: Phan An Date: Sun, 12 Jul 2026 18:48:46 +0200 Subject: [PATCH 1/4] Show the new overview sections on the Home screen koel's overview API now also returns latest/random/recently-added albums and artists, plus new/least-played/random/similar songs. Parse every section (missing keys stay empty so older servers degrade gracefully) and render each non-empty block as a card scroller, ordered to match koel web's default home layout. --- lib/providers/overview_provider.dart | 92 +++++++++------- lib/ui/screens/home.dart | 116 ++++++++++++-------- test/providers/overview_provider_test.dart | 121 +++++++++++++++++++++ 3 files changed, 247 insertions(+), 82 deletions(-) create mode 100644 test/providers/overview_provider_test.dart diff --git a/lib/providers/overview_provider.dart b/lib/providers/overview_provider.dart index a12b0b5e..c8333de3 100644 --- a/lib/providers/overview_provider.dart +++ b/lib/providers/overview_provider.dart @@ -13,9 +13,30 @@ class OverviewProvider with ChangeNotifier, StreamSubscriber { final mostPlayedSongs = []; final recentlyAddedSongs = []; final recentlyPlayedSongs = []; - final recentlyAddedAlbums = []; + final leastPlayedSongs = []; + final randomSongs = []; + final similarSongs = []; final mostPlayedAlbums = []; + final recentlyAddedAlbums = []; + final randomAlbums = []; final mostPlayedArtists = []; + final recentlyAddedArtists = []; + final randomArtists = []; + + late final List> _allSections = [ + mostPlayedSongs, + recentlyAddedSongs, + recentlyPlayedSongs, + leastPlayedSongs, + randomSongs, + similarSongs, + mostPlayedAlbums, + recentlyAddedAlbums, + randomAlbums, + mostPlayedArtists, + recentlyAddedArtists, + randomArtists, + ]; OverviewProvider({ required playableProvider, @@ -27,58 +48,55 @@ class OverviewProvider with ChangeNotifier, StreamSubscriber { _artistProvider = artistProvider, _recentlyPlayedProvider = recentlyPlayedProvider { subscribe(AuthProvider.userLoggedOutStream.listen((_) { - mostPlayedSongs.clear(); - recentlyAddedSongs.clear(); - recentlyPlayedSongs.clear(); - mostPlayedAlbums.clear(); - mostPlayedArtists.clear(); - + for (final section in _allSections) section.clear(); notifyListeners(); })); } - bool get isEmpty => - mostPlayedSongs.isEmpty && - recentlyAddedSongs.isEmpty && - recentlyPlayedSongs.isEmpty && - mostPlayedAlbums.isEmpty && - mostPlayedArtists.isEmpty; + bool get isEmpty => _allSections.every((section) => section.isEmpty); Future refresh() async { final Map response = await get('overview'); - mostPlayedSongs - ..clear() - ..addAll(_playableProvider.parseFromJson(response['most_played_songs'])); + _fill(mostPlayedSongs, _parseSongs(response['most_played_songs'])); + _fill(recentlyAddedSongs, _parseSongs(response['recently_added_songs'])); + _fill(recentlyPlayedSongs, _parseSongs(response['recently_played_songs'])); + _fill(leastPlayedSongs, _parseSongs(response['least_played_songs'])); + _fill(randomSongs, _parseSongs(response['random_songs'])); + _fill(similarSongs, _parseSongs(response['similar_songs'])); - recentlyAddedSongs - ..clear() - ..addAll( - _playableProvider.parseFromJson(response['recently_added_songs'])); + _fill(mostPlayedAlbums, _parseAlbums(response['most_played_albums'])); + _fill(recentlyAddedAlbums, _parseAlbums(response['recently_added_albums'])); + _fill(randomAlbums, _parseAlbums(response['random_albums'])); - recentlyPlayedSongs - ..clear() - ..addAll( - _playableProvider.parseFromJson(response['recently_played_songs'])); + _fill(mostPlayedArtists, _parseArtists(response['most_played_artists'])); + _fill(recentlyAddedArtists, + _parseArtists(response['recently_added_artists'])); + _fill(randomArtists, _parseArtists(response['random_artists'])); _recentlyPlayedProvider.seed(recentlyPlayedSongs); - final _mostPlayedAlbums = response['most_played_albums'] - .map((j) => Album.fromJson(j)) - .toList(); + notifyListeners(); + } - mostPlayedAlbums - ..clear() - ..addAll(_albumProvider.syncWithVault(_mostPlayedAlbums)); + List _parseSongs(dynamic json) => + _playableProvider.parseFromJson(json ?? const []); - final _mostPlayedArtist = response['most_played_artists'] - .map((j) => Artist.fromJson(j)) - .toList(); + List _parseAlbums(dynamic json) => _albumProvider.syncWithVault( + ((json ?? const []) as List) + .map((j) => Album.fromJson(j)) + .toList(), + ); - mostPlayedArtists - ..clear() - ..addAll(_artistProvider.syncWithVault(_mostPlayedArtist)); + List _parseArtists(dynamic json) => _artistProvider.syncWithVault( + ((json ?? const []) as List) + .map((j) => Artist.fromJson(j)) + .toList(), + ); - notifyListeners(); + void _fill(List target, List source) { + target + ..clear() + ..addAll(source); } } diff --git a/lib/ui/screens/home.dart b/lib/ui/screens/home.dart index e5d28974..31b50493 100644 --- a/lib/ui/screens/home.dart +++ b/lib/ui/screens/home.dart @@ -46,6 +46,52 @@ class _HomeScreenState extends State { } } + Widget _songBlock(String heading, List songs) { + return HorizontalCardScroller( + headingText: heading, + cards: [ + ...songs.map((playable) => SongCard(playable: playable)), + PlaceholderCard( + icon: CupertinoIcons.music_note, + onPressed: () => Navigator.of(context).push( + CupertinoPageRoute(builder: (_) => SongsScreen()), + ), + ), + ], + ); + } + + Widget _albumBlock(String heading, List albums) { + return HorizontalCardScroller( + headingText: heading, + cards: [ + ...albums.map((album) => AlbumCard(album: album)), + PlaceholderCard( + icon: CupertinoIcons.music_albums, + onPressed: () => Navigator.of(context).push( + CupertinoPageRoute(builder: (_) => AlbumsScreen()), + ), + ), + ], + ); + } + + Widget _artistBlock(String heading, List artists) { + return HorizontalCardScroller( + headingText: heading, + cards: [ + ...artists.map((artist) => ArtistCard(artist: artist)), + PlaceholderCard( + icon: CupertinoIcons.music_mic, + circular: true, + onPressed: () => Navigator.of(context).push( + CupertinoPageRoute(builder: (_) => const ArtistsScreen()), + ), + ), + ], + ); + } + @override Widget build(BuildContext context) { return Consumer( @@ -53,55 +99,35 @@ class _HomeScreenState extends State { if (_loading) return const HomeScreenPlaceholder(); if (_errored) return OopsBox(onRetry: fetchData); + final op = overviewProvider; final blocks = [ - if (overviewProvider.mostPlayedSongs.isNotEmpty) - HorizontalCardScroller( - headingText: 'Most played', - cards: [ - ...overviewProvider.mostPlayedSongs - .map((playable) => SongCard(playable: playable)), - PlaceholderCard( - icon: CupertinoIcons.music_note, - onPressed: () => Navigator.of(context).push( - CupertinoPageRoute(builder: (_) => SongsScreen()), - ), - ), - ], - ), - if (overviewProvider.mostPlayedAlbums.isNotEmpty) - HorizontalCardScroller( - headingText: 'Top albums', - cards: [ - ...overviewProvider.mostPlayedAlbums - .map((album) => AlbumCard(album: album)), - PlaceholderCard( - icon: CupertinoIcons.music_albums, - onPressed: () => Navigator.of(context).push( - CupertinoPageRoute(builder: (_) => AlbumsScreen()), - ), - ), - ], - ), - if (overviewProvider.mostPlayedArtists.isNotEmpty) - HorizontalCardScroller( - headingText: 'Top artists', - cards: [ - ...overviewProvider.mostPlayedArtists - .map((artist) => ArtistCard(artist: artist)), - PlaceholderCard( - icon: CupertinoIcons.music_mic, - circular: true, - onPressed: () => Navigator.of(context).push( - CupertinoPageRoute(builder: (_) => const ArtistsScreen()), - ), - ), - ], - ), + if (op.recentlyAddedAlbums.isNotEmpty) + _albumBlock('Latest Albums', op.recentlyAddedAlbums), + if (op.similarSongs.isNotEmpty) + _songBlock('You Might Also Like', op.similarSongs), + if (op.mostPlayedAlbums.isNotEmpty) + _albumBlock('Top Albums', op.mostPlayedAlbums), + if (op.mostPlayedSongs.isNotEmpty) + _songBlock('Most Played', op.mostPlayedSongs), + if (op.mostPlayedArtists.isNotEmpty) + _artistBlock('Top Artists', op.mostPlayedArtists), + if (op.recentlyAddedSongs.isNotEmpty) + _songBlock('New Songs', op.recentlyAddedSongs), + if (op.recentlyAddedArtists.isNotEmpty) + _artistBlock('New Artists', op.recentlyAddedArtists), + if (op.leastPlayedSongs.isNotEmpty) + _songBlock('Hidden Gems', op.leastPlayedSongs), + if (op.randomSongs.isNotEmpty) + _songBlock('Random Songs', op.randomSongs), + if (op.randomAlbums.isNotEmpty) + _albumBlock('Random Albums', op.randomAlbums), + if (op.randomArtists.isNotEmpty) + _artistBlock('Random Artists', op.randomArtists), ] .map( - (widget) => Padding( + (block) => Padding( padding: const EdgeInsets.symmetric(vertical: 16), - child: widget, + child: block, ), ) .toList(); diff --git a/test/providers/overview_provider_test.dart b/test/providers/overview_provider_test.dart new file mode 100644 index 00000000..4b650bb2 --- /dev/null +++ b/test/providers/overview_provider_test.dart @@ -0,0 +1,121 @@ +import 'package:app/models/song.dart'; +import 'package:app/providers/album_provider.dart'; +import 'package:app/providers/artist_provider.dart'; +import 'package:app/providers/overview_provider.dart'; +import 'package:app/providers/playable_provider.dart'; +import 'package:app/providers/recently_played_provider.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../helpers/api_test_setup.dart'; + +void main() { + late OverviewProvider overview; + late CapturingClient client; + var _seq = 0; + + List> songs(int count) => + List.generate(count, (_) => Song.fake().toJson()); + + List> albums(int count) => List.generate(count, (_) { + final id = 'album-${_seq++}'; + return { + 'id': id, + 'name': 'Album $id', + 'cover': null, + 'artist_id': 'artist-$id', + 'artist_name': 'Artist $id', + 'year': null, + }; + }); + + List> artists(int count) => List.generate(count, (_) { + final id = 'artist-${_seq++}'; + return {'id': id, 'name': 'Artist $id', 'image': null}; + }); + + setUpAll(() async => await initApiTestEnvironment()); + + setUp(() { + _seq = 0; + final playableProvider = PlayableProvider(); + overview = OverviewProvider( + playableProvider: playableProvider, + albumProvider: AlbumProvider(), + artistProvider: ArtistProvider(), + recentlyPlayedProvider: + RecentlyPlayedProvider(playableProvider: playableProvider), + ); + client = CapturingClient(); + client.install(); + setUpApiTest(); + }); + + tearDown(tearDownApiTest); + + test('refresh populates every overview section', () async { + client.willReturn(json: { + 'most_played_songs': songs(3), + 'recently_added_songs': songs(2), + 'recently_played_songs': songs(4), + 'least_played_songs': songs(1), + 'random_songs': songs(5), + 'similar_songs': songs(6), + 'most_played_albums': albums(3), + 'recently_added_albums': albums(2), + 'random_albums': albums(4), + 'most_played_artists': artists(3), + 'recently_added_artists': artists(2), + 'random_artists': artists(4), + }); + + await overview.refresh(); + + expect(overview.mostPlayedSongs, hasLength(3)); + expect(overview.recentlyAddedSongs, hasLength(2)); + expect(overview.recentlyPlayedSongs, hasLength(4)); + expect(overview.leastPlayedSongs, hasLength(1)); + expect(overview.randomSongs, hasLength(5)); + expect(overview.similarSongs, hasLength(6)); + expect(overview.mostPlayedAlbums, hasLength(3)); + expect(overview.recentlyAddedAlbums, hasLength(2)); + expect(overview.randomAlbums, hasLength(4)); + expect(overview.mostPlayedArtists, hasLength(3)); + expect(overview.recentlyAddedArtists, hasLength(2)); + expect(overview.randomArtists, hasLength(4)); + }); + + test('refresh leaves sections empty when an older API omits them', () async { + client.willReturn(json: { + 'most_played_songs': songs(2), + 'recently_played_songs': songs(1), + 'most_played_albums': albums(2), + 'most_played_artists': artists(2), + }); + + await overview.refresh(); + + expect(overview.mostPlayedSongs, hasLength(2)); + expect(overview.recentlyPlayedSongs, hasLength(1)); + expect(overview.mostPlayedAlbums, hasLength(2)); + expect(overview.mostPlayedArtists, hasLength(2)); + + expect(overview.recentlyAddedSongs, isEmpty); + expect(overview.leastPlayedSongs, isEmpty); + expect(overview.randomSongs, isEmpty); + expect(overview.similarSongs, isEmpty); + expect(overview.recentlyAddedAlbums, isEmpty); + expect(overview.randomAlbums, isEmpty); + expect(overview.recentlyAddedArtists, isEmpty); + expect(overview.randomArtists, isEmpty); + + expect(overview.isEmpty, isFalse); + }); + + test('isEmpty is true when the API returns nothing', () async { + client.willReturn(json: {}); + + await overview.refresh(); + + expect(overview.isEmpty, isTrue); + }); +} From b5ec94e40e92c468f61d8e3e7cb75ae9e99397ac Mon Sep 17 00:00:00 2001 From: Phan An Date: Sun, 12 Jul 2026 19:03:18 +0200 Subject: [PATCH 2/4] Honour the user's saved home-block order koel web lets users reorder their home blocks and persists it as the home_blocks_order preference (returned via GET me). Parse it into the User model and order the Home screen blocks accordingly, falling back to koel web's default order when no preference is saved or the server (older API) doesn't expose one. --- lib/models/user.dart | 11 +++ lib/providers/auth_provider.dart | 6 +- lib/ui/screens/home.dart | 82 ++++++++++++++++++---- test/models/user_test.dart | 36 ++++++++++ test/ui/screens/home_block_order_test.dart | 47 +++++++++++++ 5 files changed, 166 insertions(+), 16 deletions(-) create mode 100644 test/models/user_test.dart create mode 100644 test/ui/screens/home_block_order_test.dart diff --git a/lib/models/user.dart b/lib/models/user.dart index 0375216b..08e5d980 100644 --- a/lib/models/user.dart +++ b/lib/models/user.dart @@ -6,10 +6,16 @@ class User { String name; String email; + /// The user's preferred order of Home screen blocks, by block id. Empty + /// when the server doesn't expose the preference (older API) or the user + /// never reordered the blocks. + final List homeBlocksOrder; + User({ required this.id, required this.name, required this.email, + this.homeBlocksOrder = const [], }); CachedNetworkImageProvider get avatar { @@ -21,10 +27,15 @@ class User { } factory User.fromJson(Map json) { + final preferences = json['preferences']; + final order = preferences is Map ? preferences['home_blocks_order'] : null; + return User( id: json['id'], name: json['name'], email: json['email'], + homeBlocksOrder: + order is List ? order.whereType().toList() : const [], ); } } diff --git a/lib/providers/auth_provider.dart b/lib/providers/auth_provider.dart index 03daa383..423d444f 100644 --- a/lib/providers/auth_provider.dart +++ b/lib/providers/auth_provider.dart @@ -7,9 +7,11 @@ import 'package:app/utils/api_request.dart'; import 'package:app/utils/preferences.dart' as preferences; class AuthProvider with StreamSubscriber { - late User _authUser; + User? _authUser; - User get authUser => _authUser; + User get authUser => _authUser!; + + User? get maybeAuthUser => _authUser; static final _userLoggedIn = StreamController.broadcast(); static final userLoggedInStream = _userLoggedIn.stream; diff --git a/lib/ui/screens/home.dart b/lib/ui/screens/home.dart index 31b50493..e5ca2b48 100644 --- a/lib/ui/screens/home.dart +++ b/lib/ui/screens/home.dart @@ -19,6 +19,36 @@ class HomeScreen extends StatefulWidget { State createState() => _HomeScreenState(); } +class _HomeBlock { + final String id; + final Widget widget; + + const _HomeBlock(this.id, this.widget); +} + +/// Reorders [blocks] to honour the user's saved [savedOrder] (a list of block +/// ids). Blocks named in [savedOrder] come first in that order; the rest keep +/// their default relative order. Ids in [savedOrder] with no matching block are +/// ignored. Returns [blocks] unchanged when no preference is saved. +List orderByHomeBlocksPreference( + List blocks, + String Function(T) idOf, + List savedOrder, +) { + if (savedOrder.isEmpty) return blocks; + + final indexed = blocks.asMap().entries.toList(); + + int sortKey(MapEntry entry) { + final saved = savedOrder.indexOf(idOf(entry.value)); + return saved == -1 ? savedOrder.length + entry.key : saved; + } + + indexed.sort((a, b) => sortKey(a).compareTo(sortKey(b))); + + return indexed.map((entry) => entry.value).toList(); +} + class _HomeScreenState extends State { var _loading = false; var _errored = false; @@ -100,34 +130,58 @@ class _HomeScreenState extends State { if (_errored) return OopsBox(onRetry: fetchData); final op = overviewProvider; - final blocks = [ + + // Default order mirrors koel web's default home layout (minus the + // recently-played section, which the mobile app pins to the top). + final defaultBlocks = <_HomeBlock>[ if (op.recentlyAddedAlbums.isNotEmpty) - _albumBlock('Latest Albums', op.recentlyAddedAlbums), + _HomeBlock('recently-added-albums', + _albumBlock('Latest Albums', op.recentlyAddedAlbums)), if (op.similarSongs.isNotEmpty) - _songBlock('You Might Also Like', op.similarSongs), + _HomeBlock('similar-songs', + _songBlock('You Might Also Like', op.similarSongs)), if (op.mostPlayedAlbums.isNotEmpty) - _albumBlock('Top Albums', op.mostPlayedAlbums), + _HomeBlock('most-played-albums', + _albumBlock('Top Albums', op.mostPlayedAlbums)), if (op.mostPlayedSongs.isNotEmpty) - _songBlock('Most Played', op.mostPlayedSongs), + _HomeBlock('most-played-songs', + _songBlock('Most Played', op.mostPlayedSongs)), if (op.mostPlayedArtists.isNotEmpty) - _artistBlock('Top Artists', op.mostPlayedArtists), + _HomeBlock('most-played-artists', + _artistBlock('Top Artists', op.mostPlayedArtists)), if (op.recentlyAddedSongs.isNotEmpty) - _songBlock('New Songs', op.recentlyAddedSongs), + _HomeBlock('recently-added-songs', + _songBlock('New Songs', op.recentlyAddedSongs)), if (op.recentlyAddedArtists.isNotEmpty) - _artistBlock('New Artists', op.recentlyAddedArtists), + _HomeBlock('recently-added-artists', + _artistBlock('New Artists', op.recentlyAddedArtists)), if (op.leastPlayedSongs.isNotEmpty) - _songBlock('Hidden Gems', op.leastPlayedSongs), + _HomeBlock('least-played-songs', + _songBlock('Hidden Gems', op.leastPlayedSongs)), if (op.randomSongs.isNotEmpty) - _songBlock('Random Songs', op.randomSongs), + _HomeBlock( + 'random-songs', _songBlock('Random Songs', op.randomSongs)), if (op.randomAlbums.isNotEmpty) - _albumBlock('Random Albums', op.randomAlbums), + _HomeBlock('random-albums', + _albumBlock('Random Albums', op.randomAlbums)), if (op.randomArtists.isNotEmpty) - _artistBlock('Random Artists', op.randomArtists), - ] + _HomeBlock('random-artists', + _artistBlock('Random Artists', op.randomArtists)), + ]; + + final savedOrder = + context.read().maybeAuthUser?.homeBlocksOrder ?? + const []; + + final blocks = orderByHomeBlocksPreference<_HomeBlock>( + defaultBlocks, + (block) => block.id, + savedOrder, + ) .map( (block) => Padding( padding: const EdgeInsets.symmetric(vertical: 16), - child: block, + child: block.widget, ), ) .toList(); diff --git a/test/models/user_test.dart b/test/models/user_test.dart new file mode 100644 index 00000000..a0741fae --- /dev/null +++ b/test/models/user_test.dart @@ -0,0 +1,36 @@ +import 'package:app/models/user.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + Map baseJson({dynamic preferences}) => { + 'id': 'user-1', + 'name': 'Jane', + 'email': 'jane@koel.test', + if (preferences != null) 'preferences': preferences, + }; + + test('parses home_blocks_order from preferences', () { + final user = User.fromJson(baseJson(preferences: { + 'home_blocks_order': ['random-songs', 'most-played-albums'], + })); + + expect(user.homeBlocksOrder, ['random-songs', 'most-played-albums']); + }); + + test('defaults home_blocks_order to empty when absent (older API)', () { + expect(User.fromJson(baseJson()).homeBlocksOrder, isEmpty); + expect( + User.fromJson(baseJson(preferences: {'something_else': true})) + .homeBlocksOrder, + isEmpty, + ); + }); + + test('keeps only string entries in home_blocks_order', () { + final user = User.fromJson(baseJson(preferences: { + 'home_blocks_order': ['random-songs', 42, null, 'top-albums'], + })); + + expect(user.homeBlocksOrder, ['random-songs', 'top-albums']); + }); +} diff --git a/test/ui/screens/home_block_order_test.dart b/test/ui/screens/home_block_order_test.dart new file mode 100644 index 00000000..bebdb652 --- /dev/null +++ b/test/ui/screens/home_block_order_test.dart @@ -0,0 +1,47 @@ +import 'package:app/ui/screens/home.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + String idOf(String id) => id; + + test('keeps the default order when no preference is saved', () { + final ordered = orderByHomeBlocksPreference( + ['a', 'b', 'c'], + idOf, + const [], + ); + + expect(ordered, ['a', 'b', 'c']); + }); + + test('moves saved blocks to the front in the saved order', () { + final ordered = orderByHomeBlocksPreference( + ['a', 'b', 'c', 'd'], + idOf, + ['c', 'a'], + ); + + // c, a first (saved order), then the rest keep their default order. + expect(ordered, ['c', 'a', 'b', 'd']); + }); + + test('ignores saved ids that have no matching block', () { + final ordered = orderByHomeBlocksPreference( + ['a', 'b'], + idOf, + ['x', 'b', 'y'], + ); + + expect(ordered, ['b', 'a']); + }); + + test('preserves the default order of blocks absent from the preference', () { + final ordered = orderByHomeBlocksPreference( + ['a', 'b', 'c', 'd', 'e'], + idOf, + ['e'], + ); + + expect(ordered, ['e', 'a', 'b', 'c', 'd']); + }); +} From c538359778472eedfe9d8f8b25e4db0611e541d6 Mon Sep 17 00:00:00 2001 From: Phan An Date: Sun, 12 Jul 2026 19:24:58 +0200 Subject: [PATCH 3/4] Avoid duplicate hero tags when an album/artist repeats on Home The same album or artist can now appear in several Home blocks, which produced multiple heroes sharing one tag and crashed the details transition. Give AlbumCard/ArtistCard an asHero flag and let the Home screen grant the hero to only the first card per entity id; later duplicates render without a hero but still navigate. --- lib/ui/screens/home.dart | 32 ++++++++---- lib/ui/widgets/album_card.dart | 9 +++- lib/ui/widgets/artist_card.dart | 9 +++- test/ui/widgets/card_hero_test.dart | 79 +++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 13 deletions(-) create mode 100644 test/ui/widgets/card_hero_test.dart diff --git a/lib/ui/screens/home.dart b/lib/ui/screens/home.dart index e5ca2b48..f3d5adac 100644 --- a/lib/ui/screens/home.dart +++ b/lib/ui/screens/home.dart @@ -91,11 +91,12 @@ class _HomeScreenState extends State { ); } - Widget _albumBlock(String heading, List albums) { + Widget _albumBlock(String heading, List albums, Set seenAlbumIds) { return HorizontalCardScroller( headingText: heading, cards: [ - ...albums.map((album) => AlbumCard(album: album)), + ...albums.map((album) => + AlbumCard(album: album, asHero: seenAlbumIds.add(album.id))), PlaceholderCard( icon: CupertinoIcons.music_albums, onPressed: () => Navigator.of(context).push( @@ -106,11 +107,12 @@ class _HomeScreenState extends State { ); } - Widget _artistBlock(String heading, List artists) { + Widget _artistBlock(String heading, List artists, Set seenArtistIds) { return HorizontalCardScroller( headingText: heading, cards: [ - ...artists.map((artist) => ArtistCard(artist: artist)), + ...artists.map((artist) => + ArtistCard(artist: artist, asHero: seenArtistIds.add(artist.id))), PlaceholderCard( icon: CupertinoIcons.music_mic, circular: true, @@ -131,30 +133,38 @@ class _HomeScreenState extends State { final op = overviewProvider; + // Grant the hero animation to only the first card per entity, so the + // same album/artist appearing in several blocks doesn't produce + // duplicate hero tags within the route. + final seenAlbumIds = {}; + final seenArtistIds = {}; + // Default order mirrors koel web's default home layout (minus the // recently-played section, which the mobile app pins to the top). final defaultBlocks = <_HomeBlock>[ if (op.recentlyAddedAlbums.isNotEmpty) _HomeBlock('recently-added-albums', - _albumBlock('Latest Albums', op.recentlyAddedAlbums)), + _albumBlock('Latest Albums', op.recentlyAddedAlbums, seenAlbumIds)), if (op.similarSongs.isNotEmpty) _HomeBlock('similar-songs', _songBlock('You Might Also Like', op.similarSongs)), if (op.mostPlayedAlbums.isNotEmpty) _HomeBlock('most-played-albums', - _albumBlock('Top Albums', op.mostPlayedAlbums)), + _albumBlock('Top Albums', op.mostPlayedAlbums, seenAlbumIds)), if (op.mostPlayedSongs.isNotEmpty) _HomeBlock('most-played-songs', _songBlock('Most Played', op.mostPlayedSongs)), if (op.mostPlayedArtists.isNotEmpty) _HomeBlock('most-played-artists', - _artistBlock('Top Artists', op.mostPlayedArtists)), + _artistBlock('Top Artists', op.mostPlayedArtists, seenArtistIds)), if (op.recentlyAddedSongs.isNotEmpty) _HomeBlock('recently-added-songs', _songBlock('New Songs', op.recentlyAddedSongs)), if (op.recentlyAddedArtists.isNotEmpty) - _HomeBlock('recently-added-artists', - _artistBlock('New Artists', op.recentlyAddedArtists)), + _HomeBlock( + 'recently-added-artists', + _artistBlock( + 'New Artists', op.recentlyAddedArtists, seenArtistIds)), if (op.leastPlayedSongs.isNotEmpty) _HomeBlock('least-played-songs', _songBlock('Hidden Gems', op.leastPlayedSongs)), @@ -163,10 +173,10 @@ class _HomeScreenState extends State { 'random-songs', _songBlock('Random Songs', op.randomSongs)), if (op.randomAlbums.isNotEmpty) _HomeBlock('random-albums', - _albumBlock('Random Albums', op.randomAlbums)), + _albumBlock('Random Albums', op.randomAlbums, seenAlbumIds)), if (op.randomArtists.isNotEmpty) _HomeBlock('random-artists', - _artistBlock('Random Artists', op.randomArtists)), + _artistBlock('Random Artists', op.randomArtists, seenArtistIds)), ]; final savedOrder = diff --git a/lib/ui/widgets/album_card.dart b/lib/ui/widgets/album_card.dart index e8707b52..2203680e 100644 --- a/lib/ui/widgets/album_card.dart +++ b/lib/ui/widgets/album_card.dart @@ -10,10 +10,16 @@ class AlbumCard extends StatefulWidget { final Album album; final AppRouter router; + /// Whether the thumbnail animates to the details screen as a Hero. Set to + /// false when the same album may appear more than once in a route (e.g. + /// across Home blocks), so hero tags stay unique. + final bool asHero; + AlbumCard({ Key? key, required this.album, this.router = const AppRouter(), + this.asHero = true, }) : super(key: key); @override @@ -42,7 +48,8 @@ class _AlbumCardState extends State { opacity: _opacity, child: Column( children: [ - AlbumArtistThumbnail.md(entity: widget.album, asHero: true), + AlbumArtistThumbnail.md( + entity: widget.album, asHero: widget.asHero), const SizedBox(height: 12), SizedBox( width: _cardWidth, diff --git a/lib/ui/widgets/artist_card.dart b/lib/ui/widgets/artist_card.dart index 63e6448b..4e87a245 100644 --- a/lib/ui/widgets/artist_card.dart +++ b/lib/ui/widgets/artist_card.dart @@ -10,10 +10,16 @@ class ArtistCard extends StatefulWidget { final Artist artist; final AppRouter router; + /// Whether the thumbnail animates to the details screen as a Hero. Set to + /// false when the same artist may appear more than once in a route (e.g. + /// across Home blocks), so hero tags stay unique. + final bool asHero; + ArtistCard({ Key? key, required this.artist, this.router = const AppRouter(), + this.asHero = true, }) : super(key: key); @override @@ -43,7 +49,8 @@ class _ArtistCardState extends State { opacity: _opacity, child: Column( children: [ - AlbumArtistThumbnail.md(entity: widget.artist, asHero: true), + AlbumArtistThumbnail.md( + entity: widget.artist, asHero: widget.asHero), const SizedBox(height: 12), SizedBox( width: _cardWidth, diff --git a/test/ui/widgets/card_hero_test.dart b/test/ui/widgets/card_hero_test.dart new file mode 100644 index 00000000..2953035e --- /dev/null +++ b/test/ui/widgets/card_hero_test.dart @@ -0,0 +1,79 @@ +import 'package:app/models/album.dart'; +import 'package:app/models/artist.dart'; +import 'package:app/providers/album_provider.dart'; +import 'package:app/providers/artist_provider.dart'; +import 'package:app/ui/widgets/album_artist_thumbnail.dart'; +import 'package:app/ui/widgets/album_card.dart'; +import 'package:app/ui/widgets/artist_card.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:provider/provider.dart'; + +import '../../extensions/widget_tester_extension.dart'; + +void main() { + Finder heroWithTag(Object tag) => + find.byWidgetPredicate((widget) => widget is Hero && widget.tag == tag); + + testWidgets('an album card omits the hero when asHero is false', + (tester) async { + final album = Album.fake(); + + await tester.pumpAppWidget( + ChangeNotifierProvider( + create: (_) => AlbumProvider(), + child: AlbumCard(album: album, asHero: false), + ), + ); + + expect(heroWithTag(AlbumArtistThumbnail.tagForEntity(album)), findsNothing); + }); + + testWidgets('a repeated album yields a single hero tag across cards', + (tester) async { + final album = Album.fake(); + + await tester.pumpAppWidget( + ChangeNotifierProvider( + create: (_) => AlbumProvider(), + child: SingleChildScrollView( + child: Column( + children: [ + AlbumCard(album: album, asHero: true), + AlbumCard(album: album, asHero: false), + ], + ), + ), + ), + ); + + expect( + heroWithTag(AlbumArtistThumbnail.tagForEntity(album)), + findsOneWidget, + ); + }); + + testWidgets('a repeated artist yields a single hero tag across cards', + (tester) async { + final artist = Artist.fake(); + + await tester.pumpAppWidget( + ChangeNotifierProvider( + create: (_) => ArtistProvider(), + child: SingleChildScrollView( + child: Column( + children: [ + ArtistCard(artist: artist, asHero: true), + ArtistCard(artist: artist, asHero: false), + ], + ), + ), + ), + ); + + expect( + heroWithTag(AlbumArtistThumbnail.tagForEntity(artist)), + findsOneWidget, + ); + }); +} From 6a43867c4f5216afa17862a2bc7787bce5b0bcdf Mon Sep 17 00:00:00 2001 From: Phan An Date: Sun, 12 Jul 2026 19:27:44 +0200 Subject: [PATCH 4/4] Rename single-letter lambda params Address review: use descriptive names for the JSON-entry map lambdas and the sort comparator params, per the project's no-single-letter-vars rule. --- lib/providers/overview_provider.dart | 4 ++-- lib/ui/screens/home.dart | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/providers/overview_provider.dart b/lib/providers/overview_provider.dart index c8333de3..39b0debc 100644 --- a/lib/providers/overview_provider.dart +++ b/lib/providers/overview_provider.dart @@ -84,13 +84,13 @@ class OverviewProvider with ChangeNotifier, StreamSubscriber { List _parseAlbums(dynamic json) => _albumProvider.syncWithVault( ((json ?? const []) as List) - .map((j) => Album.fromJson(j)) + .map((entry) => Album.fromJson(entry)) .toList(), ); List _parseArtists(dynamic json) => _artistProvider.syncWithVault( ((json ?? const []) as List) - .map((j) => Artist.fromJson(j)) + .map((entry) => Artist.fromJson(entry)) .toList(), ); diff --git a/lib/ui/screens/home.dart b/lib/ui/screens/home.dart index f3d5adac..e0c22da6 100644 --- a/lib/ui/screens/home.dart +++ b/lib/ui/screens/home.dart @@ -44,7 +44,7 @@ List orderByHomeBlocksPreference( return saved == -1 ? savedOrder.length + entry.key : saved; } - indexed.sort((a, b) => sortKey(a).compareTo(sortKey(b))); + indexed.sort((left, right) => sortKey(left).compareTo(sortKey(right))); return indexed.map((entry) => entry.value).toList(); }