fix(flutter): full-player kebab nav + restore artistAlbums SWR

Two unrelated issues, batched.

Full-player kebab → "Go to album/artist" still crashed with
_debugCheckDuplicatedPageKeys despite the prior onBeforeNavigate
fix. Cause: pop() and push() ran in the same frame, so go_router's
page-key reservation table briefly contained both /now-playing AND
the destination shell-child, tripping the duplicate-key assert.

Refactor: replace onBeforeNavigate with onNavigate(path), where the
host receives the path AFTER the sheet pops and owns the
navigation entirely. The full player wires:
  onNavigate: (path) async {
    await Navigator.of(context).maybePop();
    if (context.mounted) GoRouter.of(context).push(path);
  }
Awaiting the pop guarantees /now-playing is fully gone from the
navigator's page list before the push starts.

Mini player + track rows + everywhere else use the default (no
onNavigate) path that does context.push(path) inline — they're
already inside the ShellRoute, no race possible.

Restored alwaysRefresh: true on artistAlbumsProvider. Dropping it
in the cache-loop fix had a side effect: if drift's cachedAlbums
held only a subset of an artist's albums (user previously visited
just one album by them), the artist detail page rendered that
partial list forever — provider only fetched on empty drift, never
on partial drift. The metadata prefetcher only mass-warms
artistProvider (single row), so re-enabling SWR on artistAlbums
won't recreate the storm.
This commit is contained in:
2026-05-11 23:54:11 -04:00
parent a09b636e1a
commit 5fc04f14b7
4 changed files with 46 additions and 23 deletions
@@ -101,6 +101,14 @@ final artistAlbumsProvider =
isOnline: () async => (await ref isOnline: () async => (await ref
.read(connectivityProvider.future) .read(connectivityProvider.future)
.timeout(const Duration(seconds: 3), onTimeout: () => true)), .timeout(const Duration(seconds: 3), onTimeout: () => true)),
// SWR: re-fetch the full album list on every visit. Without this,
// a previously-incomplete drift cache (e.g. user had only opened
// one album by this artist, so cachedAlbums had just that row)
// would render forever as a partial list. The prefetcher only
// warms artistProvider (single row), so this provider isn't
// mass-instantiated and the storm risk that motivated dropping
// alwaysRefresh elsewhere doesn't apply here.
alwaysRefresh: true,
tag: 'artistAlbums($artistId)', tag: 'artistAlbums($artistId)',
); );
}); });
@@ -400,9 +400,14 @@ class _SecondaryControls extends StatelessWidget {
// /now-playing lives outside the ShellRoute. Pushing // /now-playing lives outside the ShellRoute. Pushing
// /artists/:id or /albums/:id (both shell-children) on top // /artists/:id or /albums/:id (both shell-children) on top
// of it would make go_router try to mount a duplicate // of it would make go_router try to mount a duplicate
// ShellRoute. Pop ourselves first so the destination route // ShellRoute (the same _debugCheckDuplicatedPageKeys crash
// mounts cleanly inside the existing shell. // we hit before with /queue). Await our own pop fully
onBeforeNavigate: () => Navigator.of(context).maybePop(), // before pushing the destination so go_router never sees
// both pages active in the same frame.
onNavigate: (path) async {
await Navigator.of(context).maybePop();
if (context.mounted) GoRouter.of(context).push(path);
},
), ),
], ],
); );
@@ -11,7 +11,7 @@ class TrackActionsButton extends StatelessWidget {
super.key, super.key,
required this.track, required this.track,
this.hideQueueActions = false, this.hideQueueActions = false,
this.onBeforeNavigate, this.onNavigate,
}); });
final TrackRef track; final TrackRef track;
@@ -20,10 +20,10 @@ class TrackActionsButton extends StatelessWidget {
/// screen where the menu's track IS the currently-playing one. /// screen where the menu's track IS the currently-playing one.
final bool hideQueueActions; final bool hideQueueActions;
/// Forwarded to TrackActionsSheet.onBeforeNavigate. Use to dismiss /// Forwarded to TrackActionsSheet.onNavigate. The host receives the
/// the host route before "Go to album" / "Go to artist" pushes the /// destination path AFTER the sheet has popped and is responsible
/// detail screen. /// for navigating to it (typically: pop self first, then push).
final void Function()? onBeforeNavigate; final Future<void> Function(String path)? onNavigate;
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
@@ -35,7 +35,7 @@ class TrackActionsButton extends StatelessWidget {
context, context,
track, track,
hideQueueActions: hideQueueActions, hideQueueActions: hideQueueActions,
onBeforeNavigate: onBeforeNavigate, onNavigate: onNavigate,
), ),
); );
} }
@@ -21,25 +21,27 @@ class TrackActionsSheet extends ConsumerWidget {
super.key, super.key,
required this.track, required this.track,
required this.hideQueueActions, required this.hideQueueActions,
this.onBeforeNavigate, this.onNavigate,
}); });
final TrackRef track; final TrackRef track;
final bool hideQueueActions; final bool hideQueueActions;
/// Optional hook invoked just before "Go to album" / "Go to artist" /// Optional hook for "Go to album" / "Go to artist". Called with
/// pushes the destination route. Lets callers that live OUTSIDE the /// the destination route path AFTER the sheet has popped. Lets
/// ShellRoute (e.g. the full player at /now-playing) pop themselves /// callers that live OUTSIDE the ShellRoute (e.g. the full player
/// first, so go_router doesn't try to mount a second ShellRoute on /// at /now-playing) pop themselves first then push, so go_router
/// top of the active top-level route — the same duplicate-page-key /// doesn't try to mount a second ShellRoute on top of the active
/// failure mode we hit when /queue lived inside the shell. /// top-level route. When null, the sheet does its own
final void Function()? onBeforeNavigate; /// context.push, which is correct for surfaces already inside the
/// shell (mini player, track rows on detail screens, etc.).
final Future<void> Function(String path)? onNavigate;
static Future<void> show( static Future<void> show(
BuildContext context, BuildContext context,
TrackRef track, { TrackRef track, {
bool hideQueueActions = false, bool hideQueueActions = false,
void Function()? onBeforeNavigate, Future<void> Function(String path)? onNavigate,
}) { }) {
return showModalBottomSheet<void>( return showModalBottomSheet<void>(
context: context, context: context,
@@ -47,7 +49,7 @@ class TrackActionsSheet extends ConsumerWidget {
builder: (_) => TrackActionsSheet( builder: (_) => TrackActionsSheet(
track: track, track: track,
hideQueueActions: hideQueueActions, hideQueueActions: hideQueueActions,
onBeforeNavigate: onBeforeNavigate, onNavigate: onNavigate,
), ),
); );
} }
@@ -152,8 +154,12 @@ class TrackActionsSheet extends ConsumerWidget {
label: 'Go to album', label: 'Go to album',
onTap: () { onTap: () {
Navigator.pop(context); Navigator.pop(context);
onBeforeNavigate?.call(); final path = '/albums/${track.albumId}';
context.push('/albums/${track.albumId}'); if (onNavigate != null) {
onNavigate!(path);
} else {
context.push(path);
}
}, },
), ),
_MenuItem( _MenuItem(
@@ -162,8 +168,12 @@ class TrackActionsSheet extends ConsumerWidget {
label: 'Go to artist', label: 'Go to artist',
onTap: () { onTap: () {
Navigator.pop(context); Navigator.pop(context);
onBeforeNavigate?.call(); final path = '/artists/${track.artistId}';
context.push('/artists/${track.artistId}'); if (onNavigate != null) {
onNavigate!(path);
} else {
context.push(path);
}
}, },
), ),
const _Divider(), const _Divider(),