hiding original hero after hero transition (#37341)

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

@@ -259,7 +259,7 @@ class Hero extends StatefulWidget {
     assert(navigator != null);
     final Map<Object, _HeroState> result = <Object, _HeroState>{};
 
-    void addHero(StatefulElement hero, Object tag) {
+    void inviteHero(StatefulElement hero, Object tag) {
       assert(() {
         if (result.containsKey(tag)) {
           throw FlutterError(
@@ -273,29 +273,34 @@ class Hero extends StatefulWidget {
         }
         return true;
       }());
+      final Hero heroWidget = hero.widget;
       final _HeroState heroState = hero.state;
-      result[tag] = heroState;
+      if (!isUserGestureTransition || heroWidget.transitionOnUserGestures) {
+        result[tag] = heroState;
+      } else {
+        // If transition is not allowed, we need to make sure hero is not hidden.
+        // A hero can be hidden previously due to hero transition.
+        heroState.ensurePlaceholderIsHidden();
+      }
     }
 
     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;
-          final Object tag = heroWidget.tag;
+        assert(tag != null);
-          assert(tag != null);
+        if (Navigator.of(hero) == navigator) {
-          if (Navigator.of(hero) == navigator) {
+          inviteHero(hero, tag);
-            addHero(hero, tag);
+        } else {
-          } else {
+          // The nearest navigator to the Hero is not the Navigator that is
-            // The nearest navigator to the Hero is not the Navigator that is
+          // currently transitioning from one route to another. This means
-            // currently transitioning from one route to another. This means
+          // the Hero is inside a nested Navigator and should only be
-            // the Hero is inside a nested Navigator and should only be
+          // considered for animation if it is part of the top-most route in
-            // considered for animation if it is part of the top-most route in
+          // that nested Navigator and if that route is also a PageRoute.
-            // that nested Navigator and if that route is also a PageRoute.
+          final ModalRoute<dynamic> heroRoute = ModalRoute.of(hero);
-            final ModalRoute<dynamic> heroRoute = ModalRoute.of(hero);
+          if (heroRoute != null && heroRoute is PageRoute && heroRoute.isCurrent) {
-            if (heroRoute != null && heroRoute is PageRoute && heroRoute.isCurrent) {
+            inviteHero(hero, tag);
-              addHero(hero, tag);
-            }
           }
         }
       }
@@ -345,7 +350,7 @@ class _HeroState extends State<Hero> {
     });
   }
 
-  void endFlight() {
+  void ensurePlaceholderIsHidden() {
     if (mounted) {
       setState(() {
         _placeholderSize = null;
@@ -353,6 +358,14 @@ class _HeroState extends State<Hero> {
     }
   }
 
+  // When `keepPlaceholder` is true, the placeholder will continue to be shown
+  // after the flight ends.
+  void endFlight({ bool keepPlaceholder = false }) {
+    if (!keepPlaceholder) {
+      ensurePlaceholderIsHidden();
+    }
+  }
+
   @override
   Widget build(BuildContext context) {
     assert(
@@ -360,13 +373,13 @@ class _HeroState extends State<Hero> {
       'A Hero widget cannot be the descendant of another Hero widget.'
     );
 
-    final bool isHeroInFlight = _placeholderSize != null;
+    final bool showPlaceholder = _placeholderSize != null;
 
-    if (isHeroInFlight && widget.placeholderBuilder != null) {
+    if (showPlaceholder && widget.placeholderBuilder != null) {
       return widget.placeholderBuilder(context, _placeholderSize, widget.child);
     }
 
-    if (isHeroInFlight && !_shouldIncludeChild) {
+    if (showPlaceholder && !_shouldIncludeChild) {
       return SizedBox(
         width: _placeholderSize.width,
         height: _placeholderSize.height,
@@ -377,9 +390,9 @@ class _HeroState extends State<Hero> {
       width: _placeholderSize?.width,
       height: _placeholderSize?.height,
       child: Offstage(
-        offstage: isHeroInFlight,
+        offstage: showPlaceholder,
         child: TickerMode(
-          enabled: !isHeroInFlight,
+          enabled: !showPlaceholder,
           child: KeyedSubtree(key: _key, child: widget.child),
         )
       ),
@@ -520,9 +533,13 @@ class _HeroFlight {
       assert(overlayEntry != null);
       overlayEntry.remove();
       overlayEntry = null;
-
+      // We want to keep the hero underneath the current page hidden. If
-      manifest.fromHero.endFlight();
+      // [AnimationStatus.completed], toHero will be the one on top and we keep
-      manifest.toHero.endFlight();
+      // fromHero hidden. If [AnimationStatus.dismissed], the animation is
+      // triggered but canceled before it finishes. In this case, we keep toHero
+      // hidden instead.
+      manifest.fromHero.endFlight(keepPlaceholder: status == AnimationStatus.completed);
+      manifest.toHero.endFlight(keepPlaceholder: status == AnimationStatus.dismissed);
       onFlightEnded(this);
     }
   }
@@ -572,7 +589,6 @@ class _HeroFlight {
   // routes with the same hero. Redirect the in-flight hero to the new toRoute.
   void divert(_HeroFlightManifest newManifest) {
     assert(manifest.tag == newManifest.tag);
-
     if (manifest.type == HeroFlightDirection.push && newManifest.type == HeroFlightDirection.pop) {
       // A push flight was interrupted by a pop.
       assert(newManifest.animation.status == AnimationStatus.reverse);
@@ -600,9 +616,8 @@ class _HeroFlight {
           end: 1.0,
         ),
       );
-
       if (manifest.fromHero != newManifest.toHero) {
-        manifest.fromHero.endFlight();
+        manifest.fromHero.endFlight(keepPlaceholder: true);
         newManifest.toHero.startFlight();
         heroRectTween = _doCreateRectTween(
             heroRectTween.end,
@@ -630,8 +645,8 @@ class _HeroFlight {
       else
         _proxyAnimation.parent = newManifest.animation;
 
-      manifest.fromHero.endFlight();
+      manifest.fromHero.endFlight(keepPlaceholder: true);
-      manifest.toHero.endFlight();
+      manifest.toHero.endFlight(keepPlaceholder: true);
 
       // Let the heroes in each of the routes rebuild with their placeholders.
       newManifest.fromHero.startFlight(shouldIncludedChildInPlaceholder: newManifest.type == HeroFlightDirection.push);

packages/flutter/test/widgets/heroes_test.dart

@@ -281,6 +281,29 @@ Future<void> main() async {
     expect(find.byKey(thirdKey), isInCard);
   });
 
+  testWidgets('Heroes animate should hide original hero', (WidgetTester tester) async {
+    await tester.pumpWidget(MaterialApp(routes: routes));
+    // Checks initial state.
+    expect(find.byKey(firstKey), isOnstage);
+    expect(find.byKey(firstKey), isInCard);
+    expect(find.byKey(secondKey), findsNothing);
+
+    await tester.tap(find.text('two'));
+    await tester.pumpAndSettle(); // Waits for transition finishes.
+
+    expect(find.byKey(firstKey), findsNothing);
+    final Offstage first = tester.widget(
+      find.ancestor(
+        of: find.byKey(firstKey, skipOffstage: false),
+        matching: find.byType(Offstage, skipOffstage: false),
+      ).first
+    );
+    // Original hero should stay hidden.
+    expect(first.offstage, isTrue);
+    expect(find.byKey(secondKey), isOnstage);
+    expect(find.byKey(secondKey), isInCard);
+  });
+
   testWidgets('Destination hero is rebuilt midflight', (WidgetTester tester) async {
     final MutatingRoute route = MutatingRoute();
 
@@ -1642,6 +1665,45 @@ Future<void> main() async {
     expect(find.byKey(secondKey), findsNothing);
   });
 
+  testWidgets('Heroes animate should hide destination hero and display original hero in case of dismissed', (WidgetTester tester) async {
+    transitionFromUserGestures = true;
+    await tester.pumpWidget(MaterialApp(
+      theme: ThemeData(
+        platform: TargetPlatform.iOS,
+      ),
+      routes: routes,
+    ));
+
+    await tester.tap(find.text('two'));
+    await tester.pumpAndSettle();
+
+    expect(find.byKey(firstKey), findsNothing);
+    expect(find.byKey(secondKey), isOnstage);
+    expect(find.byKey(secondKey), isInCard);
+
+    final TestGesture gesture = await tester.startGesture(const Offset(5.0, 200.0));
+    await gesture.moveBy(const Offset(50.0, 0.0));
+    await tester.pump();
+    // It will only register the drag if we move a second time.
+    await gesture.moveBy(const Offset(50.0, 0.0));
+    await tester.pump();
+
+    // We're going to page 1 so page 1's Hero is lifted into flight.
+    expect(find.byKey(firstKey), isOnstage);
+    expect(find.byKey(firstKey), isNotInCard);
+    expect(find.byKey(secondKey), findsNothing);
+
+    // Dismisses hero transition.
+    await gesture.up();
+    await tester.pump();
+    await tester.pumpAndSettle();
+
+    // We goes back to second page.
+    expect(find.byKey(firstKey), findsNothing);
+    expect(find.byKey(secondKey), isOnstage);
+    expect(find.byKey(secondKey), isInCard);
+  });
+
   testWidgets('Handles transitions when a non-default initial route is set', (WidgetTester tester) async {
     await tester.pumpWidget(MaterialApp(
       routes: routes,