From a09b636e1af42c6ea3d64126ee961772fe8e7da3 Mon Sep 17 00:00:00 2001
From: Bryan Van Deusen <bryan.vandeusen@fabledsword.com>
Date: Mon, 11 May 2026 23:50:42 -0400
Subject: [PATCH] fix(flutter): full player kebab Go to album/artist navigates
 cleanly

Same root cause as the /queue duplicate-page-key crash: /now-playing
is a top-level route (lives outside the ShellRoute), but
/artists/:id and /albums/:id are shell-children. Pushing a shell-
child from a top-level route makes go_router attempt to mount a
second ShellRoute on top of the active one, leaving navigation in a
broken state. The mini player works because it's already inside the
shell.

Add an optional onBeforeNavigate callback to TrackActionsSheet
(forwarded through TrackActionsButton). When set, fires after
sheet.pop() and before context.push() of the destination route.

Wire the full player's TrackActionsButton with onBeforeNavigate:
() => Navigator.of(context).maybePop() so /now-playing dismisses
itself before the detail route is pushed. Result: clean navigation
into the destination, mini player visible underneath as expected.

Mini player keeps the default (no callback) since it's already in
the shell.
---
 flutter_client/lib/player/now_playing_screen.dart   |  6 ++++++
 .../widgets/track_actions/track_actions_button.dart |  7 +++++++
 .../widgets/track_actions/track_actions_sheet.dart  | 13 +++++++++++++
 3 files changed, 26 insertions(+)

diff --git a/flutter_client/lib/player/now_playing_screen.dart b/flutter_client/lib/player/now_playing_screen.dart
index 804c4ef3..e7547b7e 100644
--- a/flutter_client/lib/player/now_playing_screen.dart
+++ b/flutter_client/lib/player/now_playing_screen.dart
@@ -397,6 +397,12 @@ class _SecondaryControls extends StatelessWidget {
             streamUrl: '',
           ),
           hideQueueActions: true,
+          // /now-playing lives outside the ShellRoute. Pushing
+          // /artists/:id or /albums/:id (both shell-children) on top
+          // of it would make go_router try to mount a duplicate
+          // ShellRoute. Pop ourselves first so the destination route
+          // mounts cleanly inside the existing shell.
+          onBeforeNavigate: () => Navigator.of(context).maybePop(),
         ),
       ],
     );
diff --git a/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart b/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart
index cd293209..61b67575 100644
--- a/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart
+++ b/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart
@@ -11,6 +11,7 @@ class TrackActionsButton extends StatelessWidget {
     super.key,
     required this.track,
     this.hideQueueActions = false,
+    this.onBeforeNavigate,
   });
 
   final TrackRef track;
@@ -19,6 +20,11 @@ class TrackActionsButton extends StatelessWidget {
   /// screen where the menu's track IS the currently-playing one.
   final bool hideQueueActions;
 
+  /// Forwarded to TrackActionsSheet.onBeforeNavigate. Use to dismiss
+  /// the host route before "Go to album" / "Go to artist" pushes the
+  /// detail screen.
+  final void Function()? onBeforeNavigate;
+
   @override
   Widget build(BuildContext context) {
     final fs = Theme.of(context).extension<FabledSwordTheme>()!;
@@ -29,6 +35,7 @@ class TrackActionsButton extends StatelessWidget {
         context,
         track,
         hideQueueActions: hideQueueActions,
+        onBeforeNavigate: onBeforeNavigate,
       ),
     );
   }
diff --git a/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart b/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart
index c3adaf7e..b689f70d 100644
--- a/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart
+++ b/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart
@@ -21,15 +21,25 @@ class TrackActionsSheet extends ConsumerWidget {
     super.key,
     required this.track,
     required this.hideQueueActions,
+    this.onBeforeNavigate,
   });
 
   final TrackRef track;
   final bool hideQueueActions;
 
+  /// Optional hook invoked just before "Go to album" / "Go to artist"
+  /// pushes the destination route. Lets callers that live OUTSIDE the
+  /// ShellRoute (e.g. the full player at /now-playing) pop themselves
+  /// first, so go_router doesn't try to mount a second ShellRoute on
+  /// top of the active top-level route — the same duplicate-page-key
+  /// failure mode we hit when /queue lived inside the shell.
+  final void Function()? onBeforeNavigate;
+
   static Future<void> show(
     BuildContext context,
     TrackRef track, {
     bool hideQueueActions = false,
+    void Function()? onBeforeNavigate,
   }) {
     return showModalBottomSheet<void>(
       context: context,
@@ -37,6 +47,7 @@ class TrackActionsSheet extends ConsumerWidget {
       builder: (_) => TrackActionsSheet(
         track: track,
         hideQueueActions: hideQueueActions,
+        onBeforeNavigate: onBeforeNavigate,
       ),
     );
   }
@@ -141,6 +152,7 @@ class TrackActionsSheet extends ConsumerWidget {
               label: 'Go to album',
               onTap: () {
                 Navigator.pop(context);
+                onBeforeNavigate?.call();
                 context.push('/albums/${track.albumId}');
               },
             ),
@@ -150,6 +162,7 @@ class TrackActionsSheet extends ConsumerWidget {
               label: 'Go to artist',
               onTap: () {
                 Navigator.pop(context);
+                onBeforeNavigate?.call();
                 context.push('/artists/${track.artistId}');
               },
             ),