From f8d2f58b52c9e9f162bfaf6c4a882aa922682719 Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Thu, 12 Nov 2020 11:45:32 -0800 Subject: [PATCH] Ignore DismissIntent when barrier is not dismissible (#70156) --- packages/flutter/lib/src/widgets/routes.dart | 96 +++++++++++-------- .../flutter/test/material/debug_test.dart | 1 + .../flutter/test/widgets/routes_test.dart | 23 +++++ 3 files changed, 78 insertions(+), 42 deletions(-) diff --git a/packages/flutter/lib/src/widgets/routes.dart b/packages/flutter/lib/src/widgets/routes.dart index b9a1591f05..385d188d4e 100644 --- a/packages/flutter/lib/src/widgets/routes.dart +++ b/packages/flutter/lib/src/widgets/routes.dart @@ -650,9 +650,19 @@ mixin LocalHistoryRoute on Route { } class _DismissModalAction extends DismissAction { + _DismissModalAction(this.context); + + final BuildContext context; + + @override + bool isEnabled(DismissIntent intent) { + final ModalRoute route = ModalRoute.of(context)!; + return route.barrierDismissible; + } + @override Object invoke(DismissIntent intent) { - return Navigator.of(primaryFocus!.context!)!.maybePop(); + return Navigator.of(context)!.maybePop(); } } @@ -767,10 +777,6 @@ class _ModalScopeState extends State<_ModalScope> { setState(fn); } - static final Map> _actionMap = >{ - DismissIntent: _DismissModalAction(), - }; - @override Widget build(BuildContext context) { return AnimatedBuilder( @@ -790,51 +796,57 @@ class _ModalScopeState extends State<_ModalScope> { offstage: widget.route.offstage, // _routeSetState is called if this updates child: PageStorage( bucket: widget.route._storageBucket, // immutable - child: Actions( - actions: _actionMap, - child: FocusScope( - node: focusScopeNode, // immutable - child: RepaintBoundary( - child: AnimatedBuilder( - animation: _listenable, // immutable - builder: (BuildContext context, Widget? child) { - return widget.route.buildTransitions( - context, - widget.route.animation!, - widget.route.secondaryAnimation!, - // This additional AnimatedBuilder is include because if the - // value of the userGestureInProgressNotifier changes, it's - // only necessary to rebuild the IgnorePointer widget and set - // the focus node's ability to focus. - AnimatedBuilder( - animation: widget.route.navigator?.userGestureInProgressNotifier ?? ValueNotifier(false), - builder: (BuildContext context, Widget? child) { - final bool ignoreEvents = _shouldIgnoreFocusRequest; - focusScopeNode.canRequestFocus = !ignoreEvents; - return IgnorePointer( - ignoring: ignoreEvents, - child: child, - ); - }, - child: child, - ), - ); - }, - child: _page ??= RepaintBoundary( - key: widget.route._subtreeKey, // immutable - child: Builder( - builder: (BuildContext context) { - return widget.route.buildPage( + child: Builder( + builder: (BuildContext context) { + return Actions( + actions: >{ + DismissIntent: _DismissModalAction(context), + }, + child: FocusScope( + node: focusScopeNode, // immutable + child: RepaintBoundary( + child: AnimatedBuilder( + animation: _listenable, // immutable + builder: (BuildContext context, Widget? child) { + return widget.route.buildTransitions( context, widget.route.animation!, widget.route.secondaryAnimation!, + // This additional AnimatedBuilder is include because if the + // value of the userGestureInProgressNotifier changes, it's + // only necessary to rebuild the IgnorePointer widget and set + // the focus node's ability to focus. + AnimatedBuilder( + animation: widget.route.navigator?.userGestureInProgressNotifier ?? ValueNotifier(false), + builder: (BuildContext context, Widget? child) { + final bool ignoreEvents = _shouldIgnoreFocusRequest; + focusScopeNode.canRequestFocus = !ignoreEvents; + return IgnorePointer( + ignoring: ignoreEvents, + child: child, + ); + }, + child: child, + ), ); }, + child: _page ??= RepaintBoundary( + key: widget.route._subtreeKey, // immutable + child: Builder( + builder: (BuildContext context) { + return widget.route.buildPage( + context, + widget.route.animation!, + widget.route.secondaryAnimation!, + ); + }, + ), + ), ), ), ), - ), - ), + ); + }, ), ), ), diff --git a/packages/flutter/test/material/debug_test.dart b/packages/flutter/test/material/debug_test.dart index 5647ba9f86..d9aabdbac5 100644 --- a/packages/flutter/test/material/debug_test.dart +++ b/packages/flutter/test/material/debug_test.dart @@ -137,6 +137,7 @@ void main() { ' FocusScope\n' ' _ActionsMarker\n' ' Actions\n' + ' Builder\n' ' PageStorage\n' ' Offstage\n' ' _ModalScopeStatus\n' diff --git a/packages/flutter/test/widgets/routes_test.dart b/packages/flutter/test/widgets/routes_test.dart index 58a26492f3..967f0ee608 100644 --- a/packages/flutter/test/widgets/routes_test.dart +++ b/packages/flutter/test/widgets/routes_test.dart @@ -1641,6 +1641,29 @@ void main() { expect(find.text('dialog1'), findsNothing); }); + testWidgets('can not be dismissed with escape keyboard shortcut if barrier not dismissible', (WidgetTester tester) async { + final GlobalKey navigatorKey = GlobalKey(); + await tester.pumpWidget(MaterialApp( + navigatorKey: navigatorKey, + home: const Text('dummy1'), + )); + final Element textOnPageOne = tester.element(find.text('dummy1')); + + // Show a simple dialog + showDialog( + context: textOnPageOne, + barrierDismissible: false, + builder: (BuildContext context) => const Text('dialog1'), + ); + await tester.pumpAndSettle(); + expect(find.text('dialog1'), findsOneWidget); + + // Try to dismiss the dialog with the shortcut key + await tester.sendKeyEvent(LogicalKeyboardKey.escape); + await tester.pumpAndSettle(); + expect(find.text('dialog1'), findsOneWidget); + }); + testWidgets('ModalRoute.of works for void routes', (WidgetTester tester) async { final GlobalKey navigatorKey = GlobalKey(); await tester.pumpWidget(MaterialApp(