Merge pull request #801 from collinjackson/fix_dynamic_hero

Fix hero transition when using dynamic routes

examples/stocks/lib/main.dart

@@ -77,7 +77,7 @@ class StocksAppState extends State<StocksApp> {
     }
   }
 
-  Route _getRoute(NamedRouteSettings settings) {
+  Route _getRoute(RouteSettings settings) {
     List<String> path = settings.name.split('/');
     if (path[0] != '')
       return null;

packages/flutter/lib/src/material/material_app.dart

@@ -106,7 +106,7 @@ class _MaterialAppState extends State<MaterialApp> implements BindingObserver {
 
   final HeroController _heroController = new HeroController();
 
-  Route _generateRoute(NamedRouteSettings settings) {
+  Route _generateRoute(RouteSettings settings) {
     RouteBuilder builder = config.routes[settings.name];
     if (builder != null) {
       return new MaterialPageRoute(

packages/flutter/lib/src/material/page.dart

@@ -44,7 +44,7 @@ class MaterialPageRoute<T> extends PageRoute<T> {
   MaterialPageRoute({
     this.builder,
     Completer<T> completer,
-    NamedRouteSettings settings: const NamedRouteSettings()
+    RouteSettings settings: const RouteSettings()
   }) : super(completer: completer, settings: settings) {
     assert(builder != null);
     assert(opaque);

packages/flutter/lib/src/rendering/object.dart

@@ -1038,7 +1038,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
   /// If this render object applies a transform before painting, apply that
   /// transform to the given matrix
   ///
-  /// Used by coordinate conversion functions to translate coordiantes local to
+  /// Used by coordinate conversion functions to translate coordinates local to
   /// one render object into coordinates local to another render object.
   void applyPaintTransform(Matrix4 transform) { }
 

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

@@ -66,8 +66,8 @@ abstract class Route<T> {
   }
 }
 
-class NamedRouteSettings {
-  const NamedRouteSettings({
+class RouteSettings {
+  const RouteSettings({
     this.name,
     this.mostValuableKeys,
     this.isInitialRoute: false
@@ -88,7 +88,7 @@ class NamedRouteSettings {
   }
 }
 
-typedef Route RouteFactory(NamedRouteSettings settings);
+typedef Route RouteFactory(RouteSettings settings);
 typedef void NavigatorTransactionCallback(NavigatorTransaction transaction);
 
 class NavigatorObserver {
@@ -123,9 +123,9 @@ class Navigator extends StatefulComponent {
     });
   }
 
-  static void push(BuildContext context, Route route, { Set<Key> mostValuableKeys }) {
+  static void push(BuildContext context, Route route) {
     openTransaction(context, (NavigatorTransaction transaction) {
-      transaction.push(route, mostValuableKeys: mostValuableKeys);
+      transaction.push(route);
     });
   }
 
@@ -171,7 +171,7 @@ class NavigatorState extends State<Navigator> {
     super.initState();
     assert(config.observer == null || config.observer.navigator == null);
     config.observer?._navigator = this;
-    _push(config.onGenerateRoute(new NamedRouteSettings(
+    _push(config.onGenerateRoute(new RouteSettings(
       name: config.initialRoute ?? Navigator.defaultRouteName,
       isInitialRoute: true
     )));
@@ -213,7 +213,7 @@ class NavigatorState extends State<Navigator> {
   void _pushNamed(String name, { Set<Key> mostValuableKeys }) {
     assert(!_debugLocked);
     assert(name != null);
-    NamedRouteSettings settings = new NamedRouteSettings(
+    RouteSettings settings = new RouteSettings(
       name: name,
       mostValuableKeys: mostValuableKeys
     );
@@ -226,7 +226,7 @@ class NavigatorState extends State<Navigator> {
     _push(route);
   }
 
-  void _push(Route route, { Set<Key> mostValuableKeys }) {
+  void _push(Route route) {
     assert(!_debugLocked);
     assert(() { _debugLocked = true; return true; });
     assert(route != null);
@@ -388,9 +388,9 @@ class NavigatorTransaction {
   /// The route will have didPush() and didChangeNext() called on it; the
   /// previous route, if any, will have didChangeNext() called on it; and the
   /// Navigator observer, if any, will have didPush() called on it.
-  void push(Route route, { Set<Key> mostValuableKeys }) {
+  void push(Route route) {
     assert(_debugOpen);
-    _navigator._push(route, mostValuableKeys: mostValuableKeys);
+    _navigator._push(route);
   }
 
   /// Replaces one given route with another. Calls install(), didReplace(), and

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

@@ -14,7 +14,7 @@ import 'routes.dart';
 abstract class PageRoute<T> extends ModalRoute<T> {
   PageRoute({
     Completer<T> completer,
-    NamedRouteSettings settings: const NamedRouteSettings()
+    RouteSettings settings: const RouteSettings()
   }) : super(completer: completer, settings: settings);
   bool get opaque => true;
   bool get barrierDismissable => false;

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

@@ -366,12 +366,12 @@ class ModalPosition {
 abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T> {
   ModalRoute({
     Completer<T> completer,
-    this.settings: const NamedRouteSettings()
+    this.settings: const RouteSettings()
   }) : super.explicit(completer, null);
 
   // The API for general users of this class
 
-  final NamedRouteSettings settings;
+  final RouteSettings settings;
 
   static ModalRoute of(BuildContext context) {
     _ModalScopeStatus widget = context.inheritFromWidgetOfType(_ModalScopeStatus);

packages/unit/test/widget/heroes_test.dart

@@ -10,13 +10,15 @@ import 'test_matchers.dart';
 
 Key firstKey = new Key('first');
 Key secondKey = new Key('second');
+Key thirdKey = new Key('third');
+
 final Map<String, RouteBuilder> routes = <String, RouteBuilder>{
   '/': (RouteArguments args) => new Material(
     child: new Block([
       new Container(height: 100.0, width: 100.0),
       new Card(child: new Hero(tag: 'a', child: new Container(height: 100.0, width: 100.0, key: firstKey))),
       new Container(height: 100.0, width: 100.0),
-      new FlatButton(child: new Text('button'), onPressed: () => Navigator.pushNamed(args.context, '/two')),
+      new FlatButton(child: new Text('two'), onPressed: () => Navigator.pushNamed(args.context, '/two')),
     ])
   ),
   '/two': (RouteArguments args) => new Material(
@@ -24,11 +26,23 @@ final Map<String, RouteBuilder> routes = <String, RouteBuilder>{
       new Container(height: 150.0, width: 150.0),
       new Card(child: new Hero(tag: 'a', child: new Container(height: 150.0, width: 150.0, key: secondKey))),
       new Container(height: 150.0, width: 150.0),
-      new FlatButton(child: new Text('button'), onPressed: () => Navigator.pop(args.context)),
+      new FlatButton(child: new Text('three'), onPressed: () => Navigator.push(args.context, new ThreeRoute())),
     ])
   ),
 };
 
+class ThreeRoute extends MaterialPageRoute {
+  ThreeRoute() : super(builder: (BuildContext context) {
+    return new Material(
+      child: new Block([
+        new Container(height: 200.0, width: 200.0),
+        new Card(child: new Hero(tag: 'a', child: new Container(height: 200.0, width: 200.0, key: thirdKey))),
+        new Container(height: 200.0, width: 200.0),
+      ])
+    );
+  });
+}
+
 void main() {
   test('Heroes animate', () {
     testWidgets((WidgetTester tester) {
@@ -41,7 +55,7 @@ void main() {
       expect(tester.findElementByKey(firstKey), isInCard);
       expect(tester.findElementByKey(secondKey), isNull);
 
-      tester.tap(tester.findText('button'));
+      tester.tap(tester.findText('two'));
       tester.pump(); // begin navigation
 
       // at this stage, the second route is off-stage, so that we can form the
@@ -86,6 +100,52 @@ void main() {
       expect(tester.findElementByKey(secondKey), isOnStage);
       expect(tester.findElementByKey(secondKey), isInCard);
 
+      // Now move on to view 3
+
+      tester.tap(tester.findText('three'));
+      tester.pump(); // begin navigation
+
+      // at this stage, the second route is off-stage, so that we can form the
+      // hero party.
+
+      expect(tester.findElementByKey(secondKey), isOnStage);
+      expect(tester.findElementByKey(secondKey), isInCard);
+      expect(tester.findElementByKey(thirdKey), isOffStage);
+      expect(tester.findElementByKey(thirdKey), isInCard);
+
+      tester.pump();
+
+      // at this stage, the heroes have just gone on their journey, we are
+      // seeing them at t=16ms. The original page no longer contains the hero.
+
+      expect(tester.findElementByKey(secondKey), isNull);
+      expect(tester.findElementByKey(thirdKey), isOnStage);
+      expect(tester.findElementByKey(thirdKey), isNotInCard);
+
+      tester.pump();
+
+      // t=32ms for the journey. Surely they are still at it.
+
+      expect(tester.findElementByKey(secondKey), isNull);
+      expect(tester.findElementByKey(thirdKey), isOnStage);
+      expect(tester.findElementByKey(thirdKey), isNotInCard);
+
+      tester.pump(new Duration(seconds: 1));
+
+      // t=1.032s for the journey. The journey has ended (it ends this frame, in
+      // fact). The hero should now be in the new page, on-stage.
+
+      expect(tester.findElementByKey(secondKey), isNull);
+      expect(tester.findElementByKey(thirdKey), isOnStage);
+      expect(tester.findElementByKey(thirdKey), isInCard);
+
+      tester.pump();
+
+      // Should not change anything.
+
+      expect(tester.findElementByKey(secondKey), isNull);
+      expect(tester.findElementByKey(thirdKey), isOnStage);
+      expect(tester.findElementByKey(thirdKey), isInCard);
     });
   });
 }

packages/unit/test/widget/page_forward_transitions_test.dart

@@ -28,7 +28,7 @@ class TestTransition extends TransitionComponent {
 }
 
 class TestRoute<T> extends PageRoute<T> {
-  TestRoute({ this.child, NamedRouteSettings settings}) : super(settings: settings);
+  TestRoute({ this.child, RouteSettings settings}) : super(settings: settings);
   final Widget child;
   Duration get transitionDuration => kMaterialPageRouteTransitionDuration;
   Color get barrierColor => null;
@@ -67,7 +67,7 @@ void main() {
 
       tester.pumpWidget(
         new MaterialApp(
-          onGenerateRoute: (NamedRouteSettings settings) {
+          onGenerateRoute: (RouteSettings settings) {
             switch (settings.name) {
               case '/':
                 return new TestRoute(

packages/unit/test/widget/remember_scroll_position_test.dart

@@ -33,7 +33,7 @@ void main() {
       GlobalKey<NavigatorState> navigatorKey = new GlobalKey<NavigatorState>();
       tester.pumpWidget(new Navigator(
         key: navigatorKey,
-        onGenerateRoute: (NamedRouteSettings settings) {
+        onGenerateRoute: (RouteSettings settings) {
           if (settings.name == '/')
             return new MaterialPageRoute(builder: (_) => new Container(child: new ThePositiveNumbers()));
           else if (settings.name == '/second')