diff --git a/packages/flutter/lib/src/material/bottom_sheet.dart b/packages/flutter/lib/src/material/bottom_sheet.dart index 530dc5fea26..f88e627bac2 100644 --- a/packages/flutter/lib/src/material/bottom_sheet.dart +++ b/packages/flutter/lib/src/material/bottom_sheet.dart @@ -223,7 +223,6 @@ Future showBottomSheet({ BuildContext context, GlobalKey place return completer.future.then((_) { // If our overlay has been obscured by an opaque OverlayEntry then currentState // will have been cleared already. - if (placeholderKey.currentState != null) - placeholderKey.currentState.child = null; + placeholderKey.currentState?.child = null; }); } diff --git a/packages/flutter/lib/src/widgets/hero_controller.dart b/packages/flutter/lib/src/widgets/hero_controller.dart index 3d3bdc63a1d..fb2eb082fd7 100644 --- a/packages/flutter/lib/src/widgets/hero_controller.dart +++ b/packages/flutter/lib/src/widgets/hero_controller.dart @@ -24,28 +24,26 @@ class HeroController extends NavigatorObserver { final List _overlayEntries = new List(); - void didPushModal(Route route) { + void didPushModal(Route route, Route previousRoute) { assert(navigator != null); assert(route != null); if (route is ModalRoute) { // as opposed to StateRoute, say assert(route.performance != null); - Route from = navigator.currentRoute; - if (from is ModalRoute) // as opposed to the many other types of routes, or null - _from = from; + if (previousRoute is ModalRoute) // as opposed to the many other types of routes, or null + _from = previousRoute; _to = route; _performance = route.performance; _checkForHeroQuest(); } } - void didPopModal(Route route) { + void didPopModal(Route route, Route previousRoute) { assert(navigator != null); assert(route != null); if (route is ModalRoute) { // as opposed to StateRoute, say assert(route.performance != null); - Route to = navigator.currentRoute; - if (to is ModalRoute) { // as opposed to the many other types of routes - _to = to; + if (previousRoute is ModalRoute) { // as opposed to the many other types of routes + _to = previousRoute; _from = route; _performance = route.performance; _checkForHeroQuest(); diff --git a/packages/flutter/lib/src/widgets/navigator.dart b/packages/flutter/lib/src/widgets/navigator.dart index 56a98c55274..a6a0ba119b2 100644 --- a/packages/flutter/lib/src/widgets/navigator.dart +++ b/packages/flutter/lib/src/widgets/navigator.dart @@ -7,14 +7,24 @@ import 'overlay.dart'; abstract class Route { List get overlayEntries; + void didPush(OverlayState overlay, OverlayEntry insertionPoint) { } + void didPop(dynamic result) { } - void didPush(OverlayState overlay, OverlayEntry insertionPoint); - void didPop(dynamic result); + /// The given route has been pushed onto the navigator after this route. + /// Return true if the route before this one should be notified also. The + /// first route to return false will be the one passed to the + /// NavigatorObserver's didPushModal() as the previousRoute. + bool willPushNext(Route nextRoute) => false; + + /// The given route, which came after this one, has been popped off the + /// navigator. Return true if the route before this one should be notified + /// also. The first route to return false will be the one passed to the + /// NavigatorObserver's didPushModal() as the previousRoute. + bool didPopNext(Route nextRoute) => false; } class NamedRouteSettings { const NamedRouteSettings({ this.name, this.mostValuableKeys }); - final String name; final Set mostValuableKeys; } @@ -24,8 +34,8 @@ typedef Route RouteFactory(NamedRouteSettings settings); class NavigatorObserver { NavigatorState _navigator; NavigatorState get navigator => _navigator; - void didPopModal(Route route) { } - void didPushModal(Route route) { } + void didPushModal(Route route, Route previousRoute) { } + void didPopModal(Route route, Route previousRoute) { } } class Navigator extends StatefulComponent { @@ -99,8 +109,6 @@ class NavigatorState extends State { return null; } - Route get currentRoute => _ephemeral.isNotEmpty ? _ephemeral.last : _modal.isNotEmpty ? _modal.last : null; - void pushNamed(String name, { Set mostValuableKeys }) { assert(name != null); NamedRouteSettings settings = new NamedRouteSettings( @@ -112,8 +120,11 @@ class NavigatorState extends State { void push(Route route, { Set mostValuableKeys }) { _popAllEphemeralRoutes(); + int index = _modal.length-1; + while (index >= 0 && _modal[index].willPushNext(route)) + index -= 1; route.didPush(overlay, _currentOverlay); - config.observer?.didPushModal(route); + config.observer?.didPushModal(route, index >= 0 ? _modal[index] : null); _modal.add(route); } @@ -134,9 +145,13 @@ class NavigatorState extends State { if (_ephemeral.isNotEmpty) { _ephemeral.removeLast().didPop(result); } else { + assert(_modal.length > 1); Route route = _modal.removeLast(); route.didPop(result); - config.observer?.didPopModal(route); + int index = _modal.length-1; + while (index >= 0 && _modal[index].didPopNext(route)) + index -= 1; + config.observer?.didPopModal(route, index >= 0 ? _modal[index] : null); } } diff --git a/packages/flutter/lib/src/widgets/routes.dart b/packages/flutter/lib/src/widgets/routes.dart index d43d1210847..06fe3bfb60f 100644 --- a/packages/flutter/lib/src/widgets/routes.dart +++ b/packages/flutter/lib/src/widgets/routes.dart @@ -22,11 +22,13 @@ class StateRoute extends Route { List get overlayEntries => const []; - void didPush(OverlayState overlay, OverlayEntry insertionPoint) { } void didPop(dynamic result) { if (onPop != null) onPop(); } + + bool willPushNext(Route nextRoute) => true; + bool didPopNext(Route nextRoute) => true; } class OverlayRoute extends Route { diff --git a/packages/unit/test/widget/heroes_test.dart b/packages/unit/test/widget/heroes_test.dart index 25eb1e86474..66d386b2405 100644 --- a/packages/unit/test/widget/heroes_test.dart +++ b/packages/unit/test/widget/heroes_test.dart @@ -5,117 +5,153 @@ import 'package:flutter/material.dart'; import 'package:test/test.dart'; +import 'test_matchers.dart'; import 'widget_tester.dart'; -class TestOverlayRoute extends OverlayRoute { - List get builders => [ _build ]; - Widget _build(BuildContext context) => new Text('Overlay'); -} - -bool _isOnStage(Element element) { - expect(element, isNotNull); - bool result = true; - element.visitAncestorElements((Element ancestor) { - if (ancestor.widget is OffStage) { - result = false; - return false; - } - return true; - }); - return result; -} - -class _IsOnStage extends Matcher { - const _IsOnStage(); - bool matches(item, Map matchState) => _isOnStage(item); - Description describe(Description description) => description.add('onstage'); -} - -class _IsOffStage extends Matcher { - const _IsOffStage(); - bool matches(item, Map matchState) => !_isOnStage(item); - Description describe(Description description) => description.add('offstage'); -} - -const Matcher isOnStage = const _IsOnStage(); -const Matcher isOffStage = const _IsOffStage(); +Key firstKey = new Key('first'); +Key secondKey = new Key('second'); +final Map routes = { + '/': (RouteArguments args) => 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('state route please'), onPressed: () => Navigator.of(args.context).push(new StateRoute())), + new FlatButton(child: new Text('button'), onPressed: () => Navigator.of(args.context).pushNamed('/two')), + ]), + '/two': (RouteArguments args) => new Block([ + 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.of(args.context).pop()), + ]), +}; void main() { - test('Can pop ephemeral route without black flash', () { + test('Heroes animate', () { testWidgets((WidgetTester tester) { - GlobalKey containerKey = new GlobalKey(); - final Map routes = { - '/': (_) => new Container(key: containerKey, child: new Text('Home')), - '/settings': (_) => new Container(child: new Text('Settings')), - }; tester.pumpWidget(new MaterialApp(routes: routes)); - expect(tester.findText('Home'), isOnStage); - expect(tester.findText('Settings'), isNull); - expect(tester.findText('Overlay'), isNull); + // the initial setup. - NavigatorState navigator = Navigator.of(containerKey.currentContext); + expect(tester.findElementByKey(firstKey), isOnStage); + expect(tester.findElementByKey(firstKey), isInCard); + expect(tester.findElementByKey(secondKey), isNull); - navigator.pushNamed('/settings'); + tester.tap(tester.findText('button')); + tester.pump(); // begin navigation + + // at this stage, the second route is off-stage, so that we can form the + // hero party. + + expect(tester.findElementByKey(firstKey), isOnStage); + expect(tester.findElementByKey(firstKey), isInCard); + expect(tester.findElementByKey(secondKey), isOffStage); + expect(tester.findElementByKey(secondKey), isInCard); tester.pump(); - expect(tester.findText('Home'), isOnStage); - expect(tester.findText('Settings'), isOffStage); - expect(tester.findText('Overlay'), isNull); + // 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. - tester.pump(const Duration(milliseconds: 16)); - - expect(tester.findText('Home'), isOnStage); - expect(tester.findText('Settings'), isOnStage); - expect(tester.findText('Overlay'), isNull); - - tester.pump(const Duration(seconds: 1)); - - expect(tester.findText('Home'), isNull); - expect(tester.findText('Settings'), isOnStage); - expect(tester.findText('Overlay'), isNull); - - navigator.push(new TestOverlayRoute()); + expect(tester.findElementByKey(firstKey), isNull); + expect(tester.findElementByKey(secondKey), isOnStage); + expect(tester.findElementByKey(secondKey), isNotInCard); tester.pump(); - expect(tester.findText('Home'), isNull); - expect(tester.findText('Settings'), isOnStage); - expect(tester.findText('Overlay'), isOnStage); + // t=32ms for the journey. Surely they are still at it. - tester.pump(const Duration(seconds: 1)); + expect(tester.findElementByKey(firstKey), isNull); + expect(tester.findElementByKey(secondKey), isOnStage); + expect(tester.findElementByKey(secondKey), isNotInCard); - expect(tester.findText('Home'), isNull); - expect(tester.findText('Settings'), isOnStage); - expect(tester.findText('Overlay'), isOnStage); + 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(firstKey), isNull); + expect(tester.findElementByKey(secondKey), isOnStage); + expect(tester.findElementByKey(secondKey), isInCard); - navigator.pop(); tester.pump(); - expect(tester.findText('Home'), isNull); - expect(tester.findText('Settings'), isOnStage); - expect(tester.findText('Overlay'), isNull); + // Should not change anything. - tester.pump(const Duration(seconds: 1)); + expect(tester.findElementByKey(firstKey), isNull); + expect(tester.findElementByKey(secondKey), isOnStage); + expect(tester.findElementByKey(secondKey), isInCard); - expect(tester.findText('Home'), isNull); - expect(tester.findText('Settings'), isOnStage); - expect(tester.findText('Overlay'), isNull); + }); + }); - navigator.pop(); + test('Heroes animate even with intervening state routes', () { + testWidgets((WidgetTester tester) { + + tester.pumpWidget(new Container()); // clear our memory + + tester.pumpWidget(new MaterialApp(routes: routes)); + + // the initial setup. + + expect(tester.findElementByKey(firstKey), isOnStage); + expect(tester.findElementByKey(firstKey), isInCard); + expect(tester.findElementByKey(secondKey), isNull); + + // insert a state route + + tester.tap(tester.findText('state route please')); tester.pump(); - expect(tester.findText('Home'), isOnStage); - expect(tester.findText('Settings'), isOnStage); - expect(tester.findText('Overlay'), isNull); + expect(tester.findElementByKey(firstKey), isOnStage); + expect(tester.findElementByKey(firstKey), isInCard); + expect(tester.findElementByKey(secondKey), isNull); - tester.pump(const Duration(seconds: 1)); + tester.tap(tester.findText('button')); + tester.pump(); // begin navigation - expect(tester.findText('Home'), isOnStage); - expect(tester.findText('Settings'), isNull); - expect(tester.findText('Overlay'), isNull); + // at this stage, the second route is off-stage, so that we can form the + // hero party. + + expect(tester.findElementByKey(firstKey), isOnStage); + expect(tester.findElementByKey(firstKey), isInCard); + expect(tester.findElementByKey(secondKey), isOffStage); + expect(tester.findElementByKey(secondKey), 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(firstKey), isNull); + expect(tester.findElementByKey(secondKey), isOnStage); + expect(tester.findElementByKey(secondKey), isNotInCard); + + tester.pump(); + + // t=32ms for the journey. Surely they are still at it. + + expect(tester.findElementByKey(firstKey), isNull); + expect(tester.findElementByKey(secondKey), isOnStage); + expect(tester.findElementByKey(secondKey), 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(firstKey), isNull); + expect(tester.findElementByKey(secondKey), isOnStage); + expect(tester.findElementByKey(secondKey), isInCard); + + tester.pump(); + + // Should not change anything. + + expect(tester.findElementByKey(firstKey), isNull); + expect(tester.findElementByKey(secondKey), isOnStage); + expect(tester.findElementByKey(secondKey), isInCard); }); }); diff --git a/packages/unit/test/widget/page_transitions_test.dart b/packages/unit/test/widget/page_transitions_test.dart new file mode 100644 index 00000000000..99fee935633 --- /dev/null +++ b/packages/unit/test/widget/page_transitions_test.dart @@ -0,0 +1,95 @@ +// Copyright 2015 The Chromium 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:test/test.dart'; + +import 'test_matchers.dart'; +import 'widget_tester.dart'; + +class TestOverlayRoute extends OverlayRoute { + List get builders => [ _build ]; + Widget _build(BuildContext context) => new Text('Overlay'); +} + +void main() { + test('Check onstage/offstage handling around transitions', () { + testWidgets((WidgetTester tester) { + GlobalKey containerKey = new GlobalKey(); + final Map routes = { + '/': (_) => new Container(key: containerKey, child: new Text('Home')), + '/settings': (_) => new Container(child: new Text('Settings')), + }; + + tester.pumpWidget(new MaterialApp(routes: routes)); + + expect(tester.findText('Home'), isOnStage); + expect(tester.findText('Settings'), isNull); + expect(tester.findText('Overlay'), isNull); + + NavigatorState navigator = Navigator.of(containerKey.currentContext); + + navigator.pushNamed('/settings'); + + tester.pump(); + + expect(tester.findText('Home'), isOnStage); + expect(tester.findText('Settings'), isOffStage); + expect(tester.findText('Overlay'), isNull); + + tester.pump(const Duration(milliseconds: 16)); + + expect(tester.findText('Home'), isOnStage); + expect(tester.findText('Settings'), isOnStage); + expect(tester.findText('Overlay'), isNull); + + tester.pump(const Duration(seconds: 1)); + + expect(tester.findText('Home'), isNull); + expect(tester.findText('Settings'), isOnStage); + expect(tester.findText('Overlay'), isNull); + + navigator.push(new TestOverlayRoute()); + + tester.pump(); + + expect(tester.findText('Home'), isNull); + expect(tester.findText('Settings'), isOnStage); + expect(tester.findText('Overlay'), isOnStage); + + tester.pump(const Duration(seconds: 1)); + + expect(tester.findText('Home'), isNull); + expect(tester.findText('Settings'), isOnStage); + expect(tester.findText('Overlay'), isOnStage); + + navigator.pop(); + tester.pump(); + + expect(tester.findText('Home'), isNull); + expect(tester.findText('Settings'), isOnStage); + expect(tester.findText('Overlay'), isNull); + + tester.pump(const Duration(seconds: 1)); + + expect(tester.findText('Home'), isNull); + expect(tester.findText('Settings'), isOnStage); + expect(tester.findText('Overlay'), isNull); + + navigator.pop(); + tester.pump(); + + expect(tester.findText('Home'), isOnStage); + expect(tester.findText('Settings'), isOnStage); + expect(tester.findText('Overlay'), isNull); + + tester.pump(const Duration(seconds: 1)); + + expect(tester.findText('Home'), isOnStage); + expect(tester.findText('Settings'), isNull); + expect(tester.findText('Overlay'), isNull); + + }); + }); +} diff --git a/packages/unit/test/widget/test_matchers.dart b/packages/unit/test/widget/test_matchers.dart new file mode 100644 index 00000000000..5797347ec0a --- /dev/null +++ b/packages/unit/test/widget/test_matchers.dart @@ -0,0 +1,48 @@ +// Copyright 2015 The Chromium 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:test/test.dart'; + +bool _hasAncestorOfType(Element element, Type targetType) { + expect(element, isNotNull); + bool result = false; + element.visitAncestorElements((Element ancestor) { + if (ancestor.widget.runtimeType == targetType) { + result = true; + return false; + } + return true; + }); + return result; +} + +class _IsOnStage extends Matcher { + const _IsOnStage(); + bool matches(item, Map matchState) => !_hasAncestorOfType(item, OffStage); + Description describe(Description description) => description.add('onstage'); +} + +class _IsOffStage extends Matcher { + const _IsOffStage(); + bool matches(item, Map matchState) => _hasAncestorOfType(item, OffStage); + Description describe(Description description) => description.add('offstage'); +} + +class _IsInCard extends Matcher { + const _IsInCard(); + bool matches(item, Map matchState) => _hasAncestorOfType(item, Card); + Description describe(Description description) => description.add('in card'); +} + +class _IsNotInCard extends Matcher { + const _IsNotInCard(); + bool matches(item, Map matchState) => !_hasAncestorOfType(item, Card); + Description describe(Description description) => description.add('not in card'); +} + +const Matcher isOnStage = const _IsOnStage(); +const Matcher isOffStage = const _IsOffStage(); +const Matcher isInCard = const _IsInCard(); +const Matcher isNotInCard = const _IsNotInCard();