Heroes and nested Navigators (#29069)

packages/flutter/lib/src/widgets/heroes.dart

@@ -10,6 +10,7 @@ import 'framework.dart';
 import 'navigator.dart';
 import 'overlay.dart';
 import 'pages.dart';
+import 'routes.dart';
 import 'transitions.dart';
 
 /// Signature for a function that takes two [Rect] instances and returns a
@@ -110,6 +111,14 @@ Rect _globalBoundingBoxFor(BuildContext context) {
 /// B to A, route A's hero's widget is, by default, placed over where route B's
 /// hero's widget was, and then the animation goes the other way.
 ///
+/// ### Nested Navigators
+///
+/// If either or both routes contain nested [Navigator]s, only [Hero]s
+/// contained in the top-most routes (as defined by [Route.isCurrent]) *of those
+/// nested [Navigator]s* are considered for animation. Just like in the
+/// non-nested case the top-most routes containing these [Hero]s in the nested
+/// [Navigator]s have to be [PageRoute]s.
+///
 /// ## Parts of a Hero Transition
 ///
 /// ![Diagrams with parts of the Hero transition.](https://flutter.github.io/assets-for-api-docs/assets/interaction/heroes.png)
@@ -193,27 +202,29 @@ class Hero extends StatefulWidget {
   /// Defaults to false and cannot be null.
   final bool transitionOnUserGestures;
 
-  // Returns a map of all of the heroes in context, indexed by hero tag.
-  static Map<Object, _HeroState> _allHeroesFor(BuildContext context, bool isUserGestureTransition) {
+  // Returns a map of all of the heroes in `context` indexed by hero tag that
+  // should be considered for animation when `navigator` transitions from one
+  // PageRoute to another.
+  static Map<Object, _HeroState> _allHeroesFor(
+      BuildContext context,
+      bool isUserGestureTransition,
+      NavigatorState navigator,
+  ) {
     assert(context != null);
     assert(isUserGestureTransition != null);
+    assert(navigator != null);
     final Map<Object, _HeroState> result = <Object, _HeroState>{};
-    void visitor(Element element) {
-      if (element.widget is Hero) {
-        final StatefulElement hero = element;
-        final Hero heroWidget = element.widget;
-        if (!isUserGestureTransition || heroWidget.transitionOnUserGestures) {
-          final Object tag = heroWidget.tag;
-          assert(tag != null);
+
+    void addHero(StatefulElement hero, Object tag) {
       assert(() {
         if (result.containsKey(tag)) {
           throw FlutterError(
             'There are multiple heroes that share the same tag within a subtree.\n'
-                'Within each subtree for which heroes are to be animated (typically a PageRoute subtree), '
+            'Within each subtree for which heroes are to be animated (i.e. a PageRoute subtree), '
             'each Hero must have a unique non-null tag.\n'
             'In this case, multiple heroes had the following tag: $tag\n'
             'Here is the subtree for one of the offending heroes:\n'
-                '${element.toStringDeep(prefixLineOne: "# ")}'
+            '${hero.toStringDeep(prefixLineOne: "# ")}'
           );
         }
         return true;
@@ -221,9 +232,32 @@ class Hero extends StatefulWidget {
       final _HeroState heroState = hero.state;
       result[tag] = heroState;
     }
+
+    void visitor(Element element) {
+      if (element.widget is Hero) {
+        final StatefulElement hero = element;
+        final Hero heroWidget = element.widget;
+        if (!isUserGestureTransition || heroWidget.transitionOnUserGestures) {
+          final Object tag = heroWidget.tag;
+          assert(tag != null);
+          if (Navigator.of(hero) == navigator) {
+            addHero(hero, tag);
+          } else {
+            // The nearest navigator to the Hero is not the Navigator that is
+            // currently transitioning from one route to another. This means
+            // the Hero is inside a nested Navigator and should only be
+            // considered for animation if it is part of the top-most route in
+            // that nested Navigator and if that route is also a PageRoute.
+            final ModalRoute<dynamic> heroRoute = ModalRoute.of(hero);
+            if (heroRoute != null && heroRoute is PageRoute && heroRoute.isCurrent) {
+              addHero(hero, tag);
+            }
+          }
+        }
       }
       element.visitChildren(visitor);
     }
+
     context.visitChildElements(visitor);
     return result;
   }
@@ -652,8 +686,8 @@ class HeroController extends NavigatorObserver {
     final Rect navigatorRect = _globalBoundingBoxFor(navigator.context);
 
     // At this point the toHeroes may have been built and laid out for the first time.
-    final Map<Object, _HeroState> fromHeroes = Hero._allHeroesFor(from.subtreeContext, isUserGestureTransition);
-    final Map<Object, _HeroState> toHeroes = Hero._allHeroesFor(to.subtreeContext, isUserGestureTransition);
+    final Map<Object, _HeroState> fromHeroes = Hero._allHeroesFor(from.subtreeContext, isUserGestureTransition, navigator);
+    final Map<Object, _HeroState> toHeroes = Hero._allHeroesFor(to.subtreeContext, isUserGestureTransition, navigator);
 
     // If the `to` route was offstage, then we're implicitly restoring its
     // animation value back to what it was before it was "moved" offstage.

packages/flutter/test/widgets/heroes_test.dart

@@ -1383,7 +1383,7 @@ void main() {
 
     await tester.pump();
 
-    // Both Heros exist and seated in their normal parents.
+    // Both Heroes exist and are seated in their normal parents.
     expect(find.byKey(firstKey), isOnstage);
     expect(find.byKey(firstKey), isInCard);
     expect(find.byKey(secondKey), isOnstage);
@@ -1450,4 +1450,164 @@ void main() {
     ));
     expect(find.text('two'), findsOneWidget);
   });
+
+  testWidgets('Can push/pop on outer Navigator if nested Navigator contains Heroes', (WidgetTester tester) async {
+    // Regression test for https://github.com/flutter/flutter/issues/28042.
+
+    const String heroTag = 'You are my hero!';
+    final GlobalKey<NavigatorState> rootNavigator = GlobalKey();
+    final GlobalKey<NavigatorState> nestedNavigator = GlobalKey();
+    final Key nestedRouteHeroBottom = UniqueKey();
+    final Key nestedRouteHeroTop = UniqueKey();
+
+    await tester.pumpWidget(
+      MaterialApp(
+        navigatorKey: rootNavigator,
+        home: Navigator(
+          key: nestedNavigator,
+          onGenerateRoute: (RouteSettings settings) {
+            return MaterialPageRoute<void>(
+              builder: (BuildContext context) {
+                return Hero(
+                  tag: heroTag,
+                  child: Placeholder(
+                    key: nestedRouteHeroBottom,
+                  ),
+                );
+              }
+            );
+          },
+        ),
+      )
+    );
+
+    nestedNavigator.currentState.push(MaterialPageRoute<void>(
+      builder: (BuildContext context) {
+        return Hero(
+          tag: heroTag,
+          child: Placeholder(
+            key: nestedRouteHeroTop,
+          ),
+        );
+      },
+    ));
+    await tester.pumpAndSettle();
+
+    // Both heroes are in the tree, one is offstage
+    expect(find.byKey(nestedRouteHeroTop), findsOneWidget);
+    expect(find.byKey(nestedRouteHeroBottom), findsNothing);
+    expect(find.byKey(nestedRouteHeroBottom, skipOffstage: false), findsOneWidget);
+
+    rootNavigator.currentState.push(MaterialPageRoute<void>(
+      builder: (BuildContext context) {
+        return const Text('Foo');
+      },
+    ));
+    await tester.pumpAndSettle();
+
+    expect(find.text('Foo'), findsOneWidget);
+    // Both heroes are still in the tree, both are offstage.
+    expect(find.byKey(nestedRouteHeroBottom), findsNothing);
+    expect(find.byKey(nestedRouteHeroTop), findsNothing);
+    expect(find.byKey(nestedRouteHeroBottom, skipOffstage: false), findsOneWidget);
+    expect(find.byKey(nestedRouteHeroTop, skipOffstage: false), findsOneWidget);
+
+    // Doesn't crash.
+    expect(tester.takeException(), isNull);
+
+    rootNavigator.currentState.pop();
+    await tester.pumpAndSettle();
+
+    expect(find.text('Foo'), findsNothing);
+    // Both heroes are in the tree, one is offstage
+    expect(find.byKey(nestedRouteHeroTop), findsOneWidget);
+    expect(find.byKey(nestedRouteHeroBottom), findsNothing);
+    expect(find.byKey(nestedRouteHeroBottom, skipOffstage: false), findsOneWidget);
+  });
+
+  testWidgets('Can hero from route in root Navigator to route in nested Navigator', (WidgetTester tester) async {
+    const String heroTag = 'foo';
+    final GlobalKey<NavigatorState> rootNavigator = GlobalKey();
+    final Key smallContainer = UniqueKey();
+    final Key largeContainer = UniqueKey();
+
+    await tester.pumpWidget(
+      MaterialApp(
+        navigatorKey: rootNavigator,
+        home: Center(
+          child: Card(
+            child: Hero(
+              tag: heroTag,
+              child: Container(
+                key: largeContainer,
+                color: Colors.red,
+                height: 200.0,
+                width: 200.0,
+              ),
+            ),
+          ),
+        ),
+      ),
+    );
+
+
+    // The initial setup.
+    expect(find.byKey(largeContainer), isOnstage);
+    expect(find.byKey(largeContainer), isInCard);
+    expect(find.byKey(smallContainer, skipOffstage: false), findsNothing);
+
+    rootNavigator.currentState.push(
+      MaterialPageRoute<void>(
+        builder: (BuildContext context) {
+          return Center(
+            child: Card(
+              child: Hero(
+                tag: heroTag,
+                child: Container(
+                  key: smallContainer,
+                  color: Colors.red,
+                  height: 100.0,
+                  width: 100.0,
+                ),
+              ),
+            ),
+          );
+        }
+      ),
+    );
+    await tester.pump();
+
+    // The second route exists offstage.
+    expect(find.byKey(largeContainer), isOnstage);
+    expect(find.byKey(largeContainer), isInCard);
+    expect(find.byKey(smallContainer, skipOffstage: false), isOffstage);
+    expect(find.byKey(smallContainer, skipOffstage: false), isInCard);
+
+    await tester.pump();
+
+    // The hero started flying.
+    expect(find.byKey(largeContainer), findsNothing);
+    expect(find.byKey(smallContainer), isOnstage);
+    expect(find.byKey(smallContainer), isNotInCard);
+
+    await tester.pump(const Duration(milliseconds: 100));
+
+    // The hero is in-flight.
+    expect(find.byKey(largeContainer), findsNothing);
+    expect(find.byKey(smallContainer), isOnstage);
+    expect(find.byKey(smallContainer), isNotInCard);
+    final Size size = tester.getSize(find.byKey(smallContainer));
+    expect(size.height, greaterThan(100));
+    expect(size.width, greaterThan(100));
+    expect(size.height, lessThan(200));
+    expect(size.width, lessThan(200));
+
+    await tester.pumpAndSettle();
+
+    // The transition has ended.
+    expect(find.byKey(largeContainer), findsNothing);
+    expect(find.byKey(smallContainer), isOnstage);
+    expect(find.byKey(smallContainer), isInCard);
+    expect(tester.getSize(find.byKey(smallContainer)), const Size(100,100));
+  });
 }