diff --git a/packages/flutter/lib/src/material/dropdown.dart b/packages/flutter/lib/src/material/dropdown.dart index c4473969a7..627f8dcd2a 100644 --- a/packages/flutter/lib/src/material/dropdown.dart +++ b/packages/flutter/lib/src/material/dropdown.dart @@ -328,6 +328,10 @@ class _DropdownRoute extends PopupRoute<_DropdownRouteResult> { child: menu, ); } + + void _dismiss() { + navigator?.removeRoute(this); + } } /// An item in a menu created by a [DropdownButton]. @@ -486,7 +490,7 @@ class _DropdownButtonState extends State> with WidgetsBindi @override void dispose() { WidgetsBinding.instance.removeObserver(this); - //TODO(hansmuller) if _dropDownRoute != null Navigator.remove(context, _dropdownRoute) + _removeDropdownRoute(); super.dispose(); } @@ -494,7 +498,11 @@ class _DropdownButtonState extends State> with WidgetsBindi // Defined by WidgetsBindingObserver @override void didChangeMetrics() { - //TODO(hansmuller) if _dropDownRoute != null Navigator.remove(context, _dropdownRoute) + _removeDropdownRoute(); + } + + void _removeDropdownRoute() { + _dropdownRoute?._dismiss(); _dropdownRoute = null; } diff --git a/packages/flutter/lib/src/widgets/navigator.dart b/packages/flutter/lib/src/widgets/navigator.dart index eba712dfa7..c89d7d3e93 100644 --- a/packages/flutter/lib/src/widgets/navigator.dart +++ b/packages/flutter/lib/src/widgets/navigator.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'package:flutter/foundation.dart'; import 'package:flutter/rendering.dart'; +import 'package:flutter/scheduler.dart'; import 'basic.dart'; import 'binding.dart'; @@ -231,12 +232,15 @@ class NavigatorObserver { NavigatorState get navigator => _navigator; NavigatorState _navigator; - /// The [Navigator] pushed the given route. + /// The [Navigator] pushed `route`. void didPush(Route route, Route previousRoute) { } - /// The [Navigator] popped the given route. + /// The [Navigator] popped `route`. void didPop(Route route, Route previousRoute) { } + /// The [Navigator] removed `route`. + void didRemove(Route route, Route previousRoute) { } + /// The [Navigator] is being controlled by a user gesture. /// /// Used for the iOS back gesture. @@ -673,6 +677,19 @@ class Navigator extends StatefulWidget { return Navigator.of(context).pushReplacement(route, result: result); } + /// Immediately remove `route` and [Route.dispose] it. + /// + /// The route's animation does not run and the future returned from pushing + /// the route will not complete. Ongoing input gestures are cancelled. If + /// the [Navigator] has any [Navigator.observers], they will be notified with + /// [NavigatorObserver.didRemove]. + /// + /// This method is used to dismiss dropdown menus that are up when the screen's + /// orientation changes. + static void removeRoute(BuildContext context, Route route) { + return Navigator.of(context).removeRoute(route); + } + /// The state from the closest instance of this class that encloses the given context. /// /// Typical usage is as follows: @@ -1100,6 +1117,36 @@ class NavigatorState extends State with TickerProviderStateMixin { return true; } + /// Immediately remove `route` and [Route.dispose] it. + /// + /// The route's animation does not run and the future returned from pushing + /// the route will not complete. Ongoing input gestures are cancelled. If + /// the [Navigator] has any [Navigator.observers], they will be notified with + /// [NavigatorObserver.didRemove]. + /// + /// This method is used to dismiss dropdown menus that are up when the screen's + /// orientation changes. + void removeRoute(Route route) { + assert(route != null); + assert(!_debugLocked); + assert(() { _debugLocked = true; return true; }); + assert(route._navigator == this); + final int index = _history.indexOf(route); + assert(index != -1); + final Route previousRoute = index > 0 ? _history[index - 1] : null; + final Route nextRoute = (index + 1 < _history.length) ? _history[index + 1] : null; + setState(() { + _history.removeAt(index); + previousRoute?.didChangeNext(nextRoute); + nextRoute?.didChangePrevious(previousRoute); + for (NavigatorObserver observer in widget.observers) + observer.didRemove(route, previousRoute); + route.dispose(); + }); + assert(() { _debugLocked = false; return true; }); + _cancelActivePointers(); + } + /// Complete the lifecycle for a route that has been popped off the navigator. /// /// When the navigator pops a route, the navigator retains a reference to the @@ -1178,10 +1225,15 @@ class NavigatorState extends State with TickerProviderStateMixin { void _cancelActivePointers() { // TODO(abarth): This mechanism is far from perfect. See https://github.com/flutter/flutter/issues/4770 - final RenderAbsorbPointer absorber = _overlayKey.currentContext?.ancestorRenderObjectOfType(const TypeMatcher()); - setState(() { - absorber?.absorbing = true; - }); + if (SchedulerBinding.instance.schedulerPhase == SchedulerPhase.idle) { + // If we're between frames (SchedulerPhase.idle) then absorb any + // subsequent pointers from this frame. The absorbing flag will be + // reset in the next frame, see build(). + final RenderAbsorbPointer absorber = _overlayKey.currentContext?.ancestorRenderObjectOfType(const TypeMatcher()); + setState(() { + absorber?.absorbing = true; + }); + } for (int pointer in _activePointers.toList()) WidgetsBinding.instance.cancelPointer(pointer); } diff --git a/packages/flutter/test/material/dropdown_test.dart b/packages/flutter/test/material/dropdown_test.dart index 0f3b38219d..459ebbf02a 100644 --- a/packages/flutter/test/material/dropdown_test.dart +++ b/packages/flutter/test/material/dropdown_test.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:math' as math; +import 'dart:ui' show window; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter/material.dart'; @@ -463,4 +464,16 @@ void main() { expect(menuRect.bottomLeft, new Offset(800.0 - menuRect.width, 600.0)); expect(menuRect.bottomRight, const Offset(800.0, 600.0)); }); + + testWidgets('Dropdown menus are dismissed on screen orientation changes', (WidgetTester tester) async { + await tester.pumpWidget(buildFrame()); + await tester.tap(find.byType(dropdownButtonType)); + await tester.pumpAndSettle(); + expect(find.byType(ListView), findsOneWidget); + + window.onMetricsChanged(); + await tester.pump(); + expect(find.byType(ListView, skipOffstage: false), findsNothing); + }); + } diff --git a/packages/flutter/test/widgets/navigator_test.dart b/packages/flutter/test/widgets/navigator_test.dart index 8fbeddb6ab..3aa80bbc3e 100644 --- a/packages/flutter/test/widgets/navigator_test.dart +++ b/packages/flutter/test/widgets/navigator_test.dart @@ -8,7 +8,7 @@ import 'package:flutter/material.dart'; class FirstWidget extends StatelessWidget { @override Widget build(BuildContext context) { - return new GestureDetector( + return new GestureDetector( onTap: () { Navigator.pushNamed(context, '/second'); }, @@ -85,12 +85,12 @@ class OnTapPage extends StatelessWidget { } } -typedef void OnPushed(Route route, Route previousRoute); -typedef void OnPopped(Route route, Route previousRoute); +typedef void OnObservation(Route route, Route previousRoute); class TestObserver extends NavigatorObserver { - OnPushed onPushed; - OnPopped onPopped; + OnObservation onPushed; + OnObservation onPopped; + OnObservation onRemoved; @override void didPush(Route route, Route previousRoute) { @@ -105,6 +105,12 @@ class TestObserver extends NavigatorObserver { onPopped(route, previousRoute); } } + + @override + void didRemove(Route route, Route previousRoute) { + if (onRemoved != null) + onRemoved(route, previousRoute); + } } void main() { @@ -445,4 +451,120 @@ void main() { final String replaceNamedValue = await value; // replaceNamed result was 'B' expect(replaceNamedValue, 'B'); }); + + testWidgets('removeRoute', (WidgetTester tester) async { + final Map pageBuilders = { + '/': (BuildContext context) => new OnTapPage(id: '/', onTap: () { Navigator.pushNamed(context, '/A'); }), + '/A': (BuildContext context) => new OnTapPage(id: 'A', onTap: () { Navigator.pushNamed(context, '/B'); }), + '/B': (BuildContext context) => const OnTapPage(id: 'B'), + }; + final Map> routes = >{}; + + Route removedRoute; + Route previousRoute; + + final TestObserver observer = new TestObserver() + ..onRemoved = (Route route, Route previous) { + removedRoute = route; + previousRoute = previous; + }; + + await tester.pumpWidget(new MaterialApp( + navigatorObservers: [observer], + onGenerateRoute: (RouteSettings settings) { + routes[settings.name] = new PageRouteBuilder( + settings: settings, + pageBuilder: (BuildContext context, Animation _, Animation __) { + return pageBuilders[settings.name](context); + }, + ); + return routes[settings.name]; + } + )); + + expect(find.text('/'), findsOneWidget); + expect(find.text('A'), findsNothing); + expect(find.text('B'), findsNothing); + + await tester.tap(find.text('/')); // pushNamed('/A'), stack becomes /, /A + await tester.pumpAndSettle(); + expect(find.text('/'), findsNothing); + expect(find.text('A'), findsOneWidget); + expect(find.text('B'), findsNothing); + + await tester.tap(find.text('A')); // pushNamed('/B'), stack becomes /, /A, /B + await tester.pumpAndSettle(); + expect(find.text('/'), findsNothing); + expect(find.text('A'), findsNothing); + expect(find.text('B'), findsOneWidget); + + // Verify that the navigator's stack is ordered as expected. + expect(routes['/'].isActive, true); + expect(routes['/A'].isActive, true); + expect(routes['/B'].isActive, true); + expect(routes['/'].isFirst, true); + expect(routes['/B'].isCurrent, true); + + final NavigatorState navigator = tester.state(find.byType(Navigator)); + navigator.removeRoute(routes['/B']); // stack becomes /, /A + await tester.pump(); + expect(find.text('/'), findsNothing); + expect(find.text('A'), findsOneWidget); + expect(find.text('B'), findsNothing); + + // Verify that the navigator's stack no longer includes /B + expect(routes['/'].isActive, true); + expect(routes['/A'].isActive, true); + expect(routes['/B'].isActive, false); + expect(routes['/'].isFirst, true); + expect(routes['/A'].isCurrent, true); + + expect(removedRoute, routes['/B']); + expect(previousRoute, routes['/A']); + + navigator.removeRoute(routes['/A']); // stack becomes just / + await tester.pump(); + expect(find.text('/'), findsOneWidget); + expect(find.text('A'), findsNothing); + expect(find.text('B'), findsNothing); + + // Verify that the navigator's stack no longer includes /A + expect(routes['/'].isActive, true); + expect(routes['/A'].isActive, false); + expect(routes['/B'].isActive, false); + expect(routes['/'].isFirst, true); + expect(routes['/'].isCurrent, true); + expect(removedRoute, routes['/A']); + expect(previousRoute, routes['/']); + }); + + testWidgets('remove a route whose value is awaited', (WidgetTester tester) async { + Future pageValue; + final Map pageBuilders = { + '/': (BuildContext context) => new OnTapPage(id: '/', onTap: () { pageValue = Navigator.pushNamed(context, '/A'); }), + '/A': (BuildContext context) => new OnTapPage(id: 'A', onTap: () { Navigator.pop(context, 'A'); }), + }; + final Map> routes = >{}; + + await tester.pumpWidget(new MaterialApp( + onGenerateRoute: (RouteSettings settings) { + routes[settings.name] = new PageRouteBuilder( + settings: settings, + pageBuilder: (BuildContext context, Animation _, Animation __) { + return pageBuilders[settings.name](context); + }, + ); + return routes[settings.name]; + } + )); + + await tester.tap(find.text('/')); // pushNamed('/A'), stack becomes /, /A + await tester.pumpAndSettle(); + pageValue.then((String value) { assert(false); }); + + final NavigatorState navigator = tester.state(find.byType(Navigator)); + navigator.removeRoute(routes['/A']); // stack becomes /, pageValue will not complete + }); + + }