[go_router] [shell_route] Add observers parameter (#2664)

* [go_router] [shell_route] Add observers parameter

* [go_router] [shell_route] Add observers parameter test

* Added Licence for shell_route_observers_test.dart

* [go_router] [shell_route] Added type annotation to shell_route_observers_test.dart

* [go_router] [shell_route] Use `HeroControllerScope` for nested Navigator

* Use the correct HeroController based on the App type.

* Cache the HeroController for the nested Navigator.

* Clean up previous cache to prevent memory leak.

* Added better cache-clearing policy for the HeroController cache.

* Fixed Typos

Co-authored-by: chunhtai <[email protected]>

* Fixed Typos

Co-authored-by: chunhtai <[email protected]>

* [go_router] [shell_route] Added a better Hero test

Credits to @flodaniel!

---------

Co-authored-by: chunhtai <[email protected]>

Author: angjelkom

packages/go_router/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 6.0.7
+
+- Add observers parameter to the ShellRoute that will be passed to the nested Navigator.
+- Use `HeroControllerScope` for nested Navigator that fixes Hero Widgets not animating in Nested Navigator.
+
 ## 6.0.6
 
 - Adds `reverseTransitionDuration` to `CustomTransitionPage`

packages/go_router/lib/src/builder.dart

@@ -62,6 +62,10 @@ class RouteBuilder {
       _routeMatchLookUp[page];
 
   // final Map<>
+  /// Caches a HeroController for the nested Navigator, which solves cases where the
+  /// Hero Widget animation stops working when navigating.
+  final Map<GlobalKey<NavigatorState>, HeroController> _goHeroCache =
+      <GlobalKey<NavigatorState>, HeroController>{};
 
   /// Builds the top-level Navigator for the given [RouteMatchList].
   Widget build(
@@ -132,10 +136,10 @@ class RouteBuilder {
       bool routerNeglect,
       GlobalKey<NavigatorState> navigatorKey,
       Map<Page<Object?>, GoRouterState> registry) {
+    final Map<GlobalKey<NavigatorState>, List<Page<Object?>>> keyToPage =
+        <GlobalKey<NavigatorState>, List<Page<Object?>>>{};
     try {
       assert(_routeMatchLookUp.isEmpty);
-      final Map<GlobalKey<NavigatorState>, List<Page<Object?>>> keyToPage =
-          <GlobalKey<NavigatorState>, List<Page<Object?>>>{};
       _buildRecursive(context, matchList, 0, onPopPage, routerNeglect,
           keyToPage, navigatorKey, registry);
 
@@ -147,6 +151,10 @@ class RouteBuilder {
       return <Page<Object?>>[
         _buildErrorPage(context, e, matchList.uri),
       ];
+    } finally {
+      /// Clean up previous cache to prevent memory leak.
+      _goHeroCache.removeWhere(
+          (GlobalKey<NavigatorState> key, _) => !keyToPage.keys.contains(key));
     }
   }
 
@@ -191,6 +199,10 @@ class RouteBuilder {
       // The key to provide to the ShellRoute's Navigator.
       final GlobalKey<NavigatorState> shellNavigatorKey = route.navigatorKey;
 
+      // The observers list for the ShellRoute's Navigator.
+      final List<NavigatorObserver> observers =
+          route.observers ?? <NavigatorObserver>[];
+
       // Add an entry for the parent navigator if none exists.
       keyToPages.putIfAbsent(parentNavigatorKey, () => <Page<Object?>>[]);
 
@@ -206,9 +218,15 @@ class RouteBuilder {
       _buildRecursive(context, matchList, startIndex + 1, onPopPage,
           routerNeglect, keyToPages, shellNavigatorKey, registry);
 
+      final HeroController heroController = _goHeroCache.putIfAbsent(
+          shellNavigatorKey, () => _getHeroController(context));
       // Build the Navigator
-      final Widget child = _buildNavigator(
-          onPopPage, keyToPages[shellNavigatorKey]!, shellNavigatorKey);
+      final Widget child = HeroControllerScope(
+        controller: heroController,
+        child: _buildNavigator(
+            onPopPage, keyToPages[shellNavigatorKey]!, shellNavigatorKey,
+            observers: observers),
+      );
 
       // Build the Page for this route
       final Page<Object?> page =
@@ -451,6 +469,18 @@ class RouteBuilder {
                 : _errorBuilderForAppType!(context, state),
           );
   }
+
+  /// Return a HeroController based on the app type.
+  HeroController _getHeroController(BuildContext context) {
+    if (context is Element) {
+      if (isMaterialApp(context)) {
+        return createMaterialHeroController();
+      } else if (isCupertinoApp(context)) {
+        return createCupertinoHeroController();
+      }
+    }
+    return HeroController();
+  }
 }
 
 typedef _PageBuilderForAppType = Page<void> Function({

packages/go_router/lib/src/pages/cupertino.dart

@@ -11,6 +11,10 @@ import '../misc/extensions.dart';
 bool isCupertinoApp(Element elem) =>
     elem.findAncestorWidgetOfExactType<CupertinoApp>() != null;
 
+/// Creates a Cupertino HeroController.
+HeroController createCupertinoHeroController() =>
+    CupertinoApp.createCupertinoHeroController();
+
 /// Builds a Cupertino page.
 CupertinoPage<void> pageBuilderForCupertinoApp({
   required LocalKey key,

packages/go_router/lib/src/pages/material.dart

@@ -12,6 +12,10 @@ import '../misc/extensions.dart';
 bool isMaterialApp(Element elem) =>
     elem.findAncestorWidgetOfExactType<MaterialApp>() != null;
 
+/// Creates a Material HeroController.
+HeroController createMaterialHeroController() =>
+    MaterialApp.createMaterialHeroController();
+
 /// Builds a Material page.
 MaterialPage<void> pageBuilderForMaterialApp({
   required LocalKey key,

packages/go_router/lib/src/route.dart

@@ -420,6 +420,7 @@ class ShellRoute extends RouteBase {
   ShellRoute({
     this.builder,
     this.pageBuilder,
+    this.observers,
     super.routes,
     GlobalKey<NavigatorState>? navigatorKey,
   })  : assert(routes.isNotEmpty),
@@ -447,6 +448,12 @@ class ShellRoute extends RouteBase {
   /// sub-route's builder.
   final ShellRoutePageBuilder? pageBuilder;
 
+  /// The observers for a shell route.
+  ///
+  /// The observers parameter is used by the [Navigator] built for this route.
+  /// sub-route's observers.
+  final List<NavigatorObserver>? observers;
+
   /// The [GlobalKey] to be used by the [Navigator] built for this route.
   /// All ShellRoutes build a Navigator by default. Child GoRoutes
   /// are placed onto this Navigator instead of the root Navigator.

packages/go_router/pubspec.yaml

@@ -1,7 +1,7 @@
 name: go_router
 description: A declarative router for Flutter based on Navigation 2 supporting
   deep linking, data-driven routes and more
-version: 6.0.6
+version: 6.0.7
 repository: https://github.com/flutter/packages/tree/main/packages/go_router
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22
 

packages/go_router/test/go_router_test.dart

@@ -3218,6 +3218,56 @@ void main() {
         final bool? result = await resultFuture;
         expect(result, isTrue);
       });
+
+      testWidgets('Triggers a Hero inside a ShellRoute',
+          (WidgetTester tester) async {
+        final UniqueKey heroKey = UniqueKey();
+        const String kHeroTag = 'hero';
+
+        final List<RouteBase> routes = <RouteBase>[
+          ShellRoute(
+            builder: (BuildContext context, GoRouterState state, Widget child) {
+              return child;
+            },
+            routes: <GoRoute>[
+              GoRoute(
+                  path: '/a',
+                  builder: (BuildContext context, _) {
+                    return Hero(
+                      tag: kHeroTag,
+                      child: Container(),
+                      flightShuttleBuilder: (_, __, ___, ____, _____) {
+                        return Container(key: heroKey);
+                      },
+                    );
+                  }),
+              GoRoute(
+                  path: '/b',
+                  builder: (BuildContext context, _) {
+                    return Hero(
+                      tag: kHeroTag,
+                      child: Container(),
+                    );
+                  }),
+            ],
+          )
+        ];
+        final GoRouter router =
+            await createRouter(routes, tester, initialLocation: '/a');
+
+        // check that flightShuttleBuilder widget is not yet present
+        expect(find.byKey(heroKey), findsNothing);
+
+        // start navigation
+        router.go('/b');
+        await tester.pump();
+        await tester.pump(const Duration(milliseconds: 10));
+        // check that flightShuttleBuilder widget is visible
+        expect(find.byKey(heroKey), isOnstage);
+        // // Waits for the animation finishes.
+        await tester.pumpAndSettle();
+        expect(find.byKey(heroKey), findsNothing);
+      });
     });
   });
 }

packages/go_router/test/shell_route_observers_test.dart

@@ -0,0 +1,28 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter/material.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:go_router/go_router.dart';
+
+void main() {
+  test('ShellRoute observers test', () {
+    final ShellRoute shell = ShellRoute(
+      observers: <NavigatorObserver>[HeroController()],
+      builder: (BuildContext context, GoRouterState state, Widget child) {
+        return SafeArea(child: child);
+      },
+      routes: <RouteBase>[
+        GoRoute(
+          path: '/home',
+          builder: (BuildContext context, GoRouterState state) {
+            return Container();
+          },
+        ),
+      ],
+    );
+
+    expect(shell.observers!.length, 1);
+  });
+}