From 35532e09f0080dd0bb3fc351dd2d40d3057c7cc5 Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Fri, 9 Aug 2019 09:51:22 -0700 Subject: [PATCH] hiding original hero after hero transition (#37341) --- packages/flutter/lib/src/widgets/heroes.dart | 77 +++++++++++-------- .../flutter/test/widgets/heroes_test.dart | 62 +++++++++++++++ 2 files changed, 108 insertions(+), 31 deletions(-) diff --git a/packages/flutter/lib/src/widgets/heroes.dart b/packages/flutter/lib/src/widgets/heroes.dart index 66592a08c1..a4299dfe2f 100644 --- a/packages/flutter/lib/src/widgets/heroes.dart +++ b/packages/flutter/lib/src/widgets/heroes.dart @@ -259,7 +259,7 @@ class Hero extends StatefulWidget { assert(navigator != null); final Map result = {}; - 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; - 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 heroRoute = ModalRoute.of(hero); - if (heroRoute != null && heroRoute is PageRoute && heroRoute.isCurrent) { - addHero(hero, tag); - } + final Object tag = heroWidget.tag; + assert(tag != null); + if (Navigator.of(hero) == navigator) { + inviteHero(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 heroRoute = ModalRoute.of(hero); + if (heroRoute != null && heroRoute is PageRoute && heroRoute.isCurrent) { + inviteHero(hero, tag); } } } @@ -345,7 +350,7 @@ class _HeroState extends State { }); } - void endFlight() { + void ensurePlaceholderIsHidden() { if (mounted) { setState(() { _placeholderSize = null; @@ -353,6 +358,14 @@ class _HeroState extends State { } } + // 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 { '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 { 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; - - manifest.fromHero.endFlight(); - manifest.toHero.endFlight(); + // We want to keep the hero underneath the current page hidden. If + // [AnimationStatus.completed], toHero will be the one on top and we keep + // 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.toHero.endFlight(); + manifest.fromHero.endFlight(keepPlaceholder: true); + 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); diff --git a/packages/flutter/test/widgets/heroes_test.dart b/packages/flutter/test/widgets/heroes_test.dart index 4730bc3f73..42390ed8b4 100644 --- a/packages/flutter/test/widgets/heroes_test.dart +++ b/packages/flutter/test/widgets/heroes_test.dart @@ -281,6 +281,29 @@ Future 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 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,