diff --git a/packages/flutter/lib/src/widgets/overlay.dart b/packages/flutter/lib/src/widgets/overlay.dart index af2f19f49c..3edcbfd562 100644 --- a/packages/flutter/lib/src/widgets/overlay.dart +++ b/packages/flutter/lib/src/widgets/overlay.dart @@ -86,8 +86,7 @@ class OverlayEntry { if (_opaque == value) return; _opaque = value; - assert(_overlay != null); - _overlay._didChangeEntryOpacity(); + _overlay?._didChangeEntryOpacity(); } /// Whether this entry must be included in the tree even if there is a fully diff --git a/packages/flutter/lib/src/widgets/routes.dart b/packages/flutter/lib/src/widgets/routes.dart index 1edccba4b6..2fbf9758c7 100644 --- a/packages/flutter/lib/src/widgets/routes.dart +++ b/packages/flutter/lib/src/widgets/routes.dart @@ -184,7 +184,7 @@ abstract class TransitionRoute extends OverlayRoute { TickerFuture didPush() { assert(_controller != null, '$runtimeType.didPush called before calling install() or after calling dispose().'); assert(!_transitionCompleter.isCompleted, 'Cannot reuse a $runtimeType after disposing it.'); - _animation.addStatusListener(_handleStatusChanged); + _didPushOrReplace(); super.didPush(); return _controller.forward(); } @@ -195,10 +195,19 @@ abstract class TransitionRoute extends OverlayRoute { assert(!_transitionCompleter.isCompleted, 'Cannot reuse a $runtimeType after disposing it.'); if (oldRoute is TransitionRoute) _controller.value = oldRoute._controller.value; - _animation.addStatusListener(_handleStatusChanged); + _didPushOrReplace(); super.didReplace(oldRoute); } + void _didPushOrReplace() { + _animation.addStatusListener(_handleStatusChanged); + // If the animation is already completed, _handleStatusChanged will not get + // a chance to set opaqueness of OverlayEntry. + if (_animation.isCompleted && overlayEntries.isNotEmpty) { + overlayEntries.first.opaque = opaque; + } + } + @override bool didPop(T result) { assert(_controller != null, '$runtimeType.didPop called before calling install() or after calling dispose().'); diff --git a/packages/flutter/test/material/app_test.dart b/packages/flutter/test/material/app_test.dart index 82afb60424..6c06b42fc2 100644 --- a/packages/flutter/test/material/app_test.dart +++ b/packages/flutter/test/material/app_test.dart @@ -241,10 +241,10 @@ void main() { ), ); - expect(find.text('route "/"'), findsOneWidget); + expect(find.text('route "/"', skipOffstage: false), findsOneWidget); expect(find.text('route "/a"'), findsOneWidget); - expect(find.text('route "/a/b"'), findsNothing); - expect(find.text('route "/b"'), findsNothing); + expect(find.text('route "/a/b"', skipOffstage: false), findsNothing); + expect(find.text('route "/b"', skipOffstage: false), findsNothing); }); testWidgets('Return value from pop is correct', (WidgetTester tester) async { @@ -301,10 +301,10 @@ void main() { routes: routes, ), ); - expect(find.text('route "/"'), findsOneWidget); - expect(find.text('route "/a"'), findsOneWidget); + expect(find.text('route "/"', skipOffstage: false), findsOneWidget); + expect(find.text('route "/a"', skipOffstage: false), findsOneWidget); expect(find.text('route "/a/b"'), findsOneWidget); - expect(find.text('route "/b"'), findsNothing); + expect(find.text('route "/b"', skipOffstage: false), findsNothing); }); testWidgets('Initial route with missing step', (WidgetTester tester) async { @@ -343,9 +343,9 @@ void main() { routes: routes, ), ); - expect(find.text('route "/"'), findsOneWidget); + expect(find.text('route "/"', skipOffstage: false), findsOneWidget); expect(find.text('route "/a"'), findsOneWidget); - expect(find.text('route "/b"'), findsNothing); + expect(find.text('route "/b"', skipOffstage: false), findsNothing); // changing initialRoute has no effect await tester.pumpWidget( @@ -354,15 +354,15 @@ void main() { routes: routes, ), ); - expect(find.text('route "/"'), findsOneWidget); + expect(find.text('route "/"', skipOffstage: false), findsOneWidget); expect(find.text('route "/a"'), findsOneWidget); - expect(find.text('route "/b"'), findsNothing); + expect(find.text('route "/b"', skipOffstage: false), findsNothing); // removing it has no effect await tester.pumpWidget(MaterialApp(routes: routes)); - expect(find.text('route "/"'), findsOneWidget); + expect(find.text('route "/"', skipOffstage: false), findsOneWidget); expect(find.text('route "/a"'), findsOneWidget); - expect(find.text('route "/b"'), findsNothing); + expect(find.text('route "/b"', skipOffstage: false), findsNothing); }); testWidgets('onGenerateRoute / onUnknownRoute', (WidgetTester tester) async { diff --git a/packages/flutter/test/widgets/heroes_test.dart b/packages/flutter/test/widgets/heroes_test.dart index 6c7d90e547..9737ccfa2b 100644 --- a/packages/flutter/test/widgets/heroes_test.dart +++ b/packages/flutter/test/widgets/heroes_test.dart @@ -1754,7 +1754,9 @@ Future main() async { routes: routes, initialRoute: '/two', )); - expect(find.text('two'), findsOneWidget); + expect(tester.takeException(), isNull); + expect(find.text('two'), findsNothing); + expect(find.text('three'), findsOneWidget); }); testWidgets('Can push/pop on outer Navigator if nested Navigator contains Heroes', (WidgetTester tester) async { diff --git a/packages/flutter/test/widgets/navigator_test.dart b/packages/flutter/test/widgets/navigator_test.dart index 27a63ccfe4..cd44140e83 100644 --- a/packages/flutter/test/widgets/navigator_test.dart +++ b/packages/flutter/test/widgets/navigator_test.dart @@ -994,15 +994,15 @@ void main() { ); // The initial route /A/B/C should've been pushed successfully. - expect(find.byKey(keyRoot), findsOneWidget); - expect(find.byKey(keyA), findsOneWidget); + expect(find.byKey(keyRoot, skipOffstage: false), findsOneWidget); + expect(find.byKey(keyA, skipOffstage: false), findsOneWidget); expect(find.byKey(keyABC), findsOneWidget); keyNav.currentState.pop(); await tester.pumpAndSettle(); - expect(find.byKey(keyRoot), findsOneWidget); + expect(find.byKey(keyRoot, skipOffstage: false), findsOneWidget); expect(find.byKey(keyA), findsOneWidget); - expect(find.byKey(keyABC), findsNothing); + expect(find.byKey(keyABC, skipOffstage: false), findsNothing); }); testWidgets('The full initial route has to be matched', (WidgetTester tester) async { @@ -1089,4 +1089,111 @@ void main() { }); }); + testWidgets('OverlayEntry of topmost initial route is marked as opaque', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/38038. + + final Key root = UniqueKey(); + final Key intermediate = UniqueKey(); + final GlobalKey topmost = GlobalKey(); + await tester.pumpWidget( + MaterialApp( + initialRoute: '/A/B', + routes: { + '/': (BuildContext context) => Container(key: root), + '/A': (BuildContext context) => Container(key: intermediate), + '/A/B': (BuildContext context) => Container(key: topmost), + }, + ), + ); + + expect(ModalRoute.of(topmost.currentContext).overlayEntries.first.opaque, isTrue); + + expect(find.byKey(root), findsNothing); // hidden by opaque Route + expect(find.byKey(intermediate), findsNothing); // hidden by opaque Route + expect(find.byKey(topmost), findsOneWidget); + }); + + testWidgets('OverlayEntry of topmost route is set to opaque after Push', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/38038. + + final GlobalKey navigator = GlobalKey(); + await tester.pumpWidget( + MaterialApp( + navigatorKey: navigator, + initialRoute: '/', + onGenerateRoute: (RouteSettings settings) { + return NoAnimationPageRoute( + pageBuilder: (_) => Container(key: ValueKey(settings.name)), + ); + }, + ), + ); + expect(find.byKey(const ValueKey('/')), findsOneWidget); + + navigator.currentState.pushNamed('/A'); + await tester.pump(); + + final BuildContext topMostContext = tester.element(find.byKey(const ValueKey('/A'))); + expect(ModalRoute.of(topMostContext).overlayEntries.first.opaque, isTrue); + + expect(find.byKey(const ValueKey('/')), findsNothing); // hidden by /A + expect(find.byKey(const ValueKey('/A')), findsOneWidget); + }); + + testWidgets('OverlayEntry of topmost route is set to opaque after Replace', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/38038. + + final GlobalKey navigator = GlobalKey(); + await tester.pumpWidget( + MaterialApp( + navigatorKey: navigator, + initialRoute: '/A/B', + onGenerateRoute: (RouteSettings settings) { + return NoAnimationPageRoute( + pageBuilder: (_) => Container(key: ValueKey(settings.name)), + ); + }, + ), + ); + expect(find.byKey(const ValueKey('/')), findsNothing); + expect(find.byKey(const ValueKey('/A')), findsNothing); + expect(find.byKey(const ValueKey('/A/B')), findsOneWidget); + + final Route oldRoute = ModalRoute.of( + tester.element(find.byKey(const ValueKey('/A'), skipOffstage: false)), + ); + final Route newRoute = NoAnimationPageRoute( + pageBuilder: (_) => Container(key: const ValueKey('/C')), + ); + + navigator.currentState.replace(oldRoute: oldRoute, newRoute: newRoute); + await tester.pump(); + + expect(newRoute.overlayEntries.first.opaque, isTrue); + + expect(find.byKey(const ValueKey('/')), findsNothing); // hidden by /A/B + expect(find.byKey(const ValueKey('/A')), findsNothing); // replaced + expect(find.byKey(const ValueKey('/C')), findsNothing); // hidden by /A/B + expect(find.byKey(const ValueKey('/A/B')), findsOneWidget); + + navigator.currentState.pop(); + await tester.pumpAndSettle(); + + expect(find.byKey(const ValueKey('/')), findsNothing); // hidden by /C + expect(find.byKey(const ValueKey('/A')), findsNothing); // replaced + expect(find.byKey(const ValueKey('/A/B')), findsNothing); // popped + expect(find.byKey(const ValueKey('/C')), findsOneWidget); + }); +} + +class NoAnimationPageRoute extends PageRouteBuilder { + NoAnimationPageRoute({WidgetBuilder pageBuilder}) + : super(pageBuilder: (BuildContext context, __, ___) { + return pageBuilder(context); + }); + + @override + AnimationController createAnimationController() { + return super.createAnimationController()..value = 1.0; + } } diff --git a/packages/flutter/test/widgets/overlay_test.dart b/packages/flutter/test/widgets/overlay_test.dart index eed52a2ab3..484871cd5b 100644 --- a/packages/flutter/test/widgets/overlay_test.dart +++ b/packages/flutter/test/widgets/overlay_test.dart @@ -657,4 +657,45 @@ void main() { ), ); }); + + testWidgets('OverlayEntry.opaque can be changed when OverlayEntry is not part of an Overlay (yet)', (WidgetTester tester) async { + final GlobalKey overlayKey = GlobalKey(); + final Key root = UniqueKey(); + final Key top = UniqueKey(); + final OverlayEntry rootEntry = OverlayEntry( + builder: (BuildContext context) { + return Container(key: root); + }, + ); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Overlay( + key: overlayKey, + initialEntries: [ + rootEntry, + ], + ), + ), + ); + + expect(find.byKey(root), findsOneWidget); + + final OverlayEntry newEntry = OverlayEntry( + builder: (BuildContext context) { + return Container(key: top); + }, + ); + expect(newEntry.opaque, isFalse); + newEntry.opaque = true; // Does neither trigger an assert nor throw. + expect(newEntry.opaque, isTrue); + + // The new opaqueness is honored when inserted into an overlay. + overlayKey.currentState.insert(newEntry); + await tester.pumpAndSettle(); + + expect(find.byKey(root), findsNothing); + expect(find.byKey(top), findsOneWidget); + }); }