From ebde5e253f8172149672a8f4de0f44bef3222a72 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 14 Oct 2019 14:19:18 -0700 Subject: [PATCH] Change modal barrier to dismissing on tap up (#42253) * Change modal barrier to tap up * Verify dismiss happen at release --- .../lib/src/widgets/modal_barrier.dart | 38 ++++++------- .../test/widgets/modal_barrier_test.dart | 54 +++++++------------ 2 files changed, 39 insertions(+), 53 deletions(-) diff --git a/packages/flutter/lib/src/widgets/modal_barrier.dart b/packages/flutter/lib/src/widgets/modal_barrier.dart index cc449ef650..af633cce5c 100644 --- a/packages/flutter/lib/src/widgets/modal_barrier.dart +++ b/packages/flutter/lib/src/widgets/modal_barrier.dart @@ -94,7 +94,7 @@ class ModalBarrier extends StatelessWidget { // modal barriers are not dismissible in accessibility mode. excluding: !semanticsDismissible || !modalBarrierSemanticsDismissible, child: _ModalBarrierGestureDetector( - onAnyTapDown: () { + onDismiss: () { if (dismissible) Navigator.maybePop(context); }, @@ -195,12 +195,12 @@ class _AnyTapGestureRecognizer extends BaseTapGestureRecognizer { _AnyTapGestureRecognizer({ Object debugOwner }) : super(debugOwner: debugOwner); - VoidCallback onAnyTapDown; + VoidCallback onAnyTapUp; @protected @override bool isPointerAllowed(PointerDownEvent event) { - if (onAnyTapDown == null) + if (onAnyTapUp == null) return false; return super.isPointerAllowed(event); } @@ -208,14 +208,14 @@ class _AnyTapGestureRecognizer extends BaseTapGestureRecognizer { @protected @override void handleTapDown({PointerDownEvent down}) { - if (onAnyTapDown != null) - onAnyTapDown(); + // Do nothing. } @protected @override void handleTapUp({PointerDownEvent down, PointerUpEvent up}) { - // Do nothing. + if (onAnyTapUp != null) + onAnyTapUp(); } @protected @@ -229,28 +229,28 @@ class _AnyTapGestureRecognizer extends BaseTapGestureRecognizer { } class _ModalBarrierSemanticsDelegate extends SemanticsGestureDelegate { - const _ModalBarrierSemanticsDelegate({this.onAnyTapDown}); + const _ModalBarrierSemanticsDelegate({this.onDismiss}); - final VoidCallback onAnyTapDown; + final VoidCallback onDismiss; @override void assignSemantics(RenderSemanticsGestureHandler renderObject) { - renderObject.onTap = onAnyTapDown; + renderObject.onTap = onDismiss; } } class _AnyTapGestureRecognizerFactory extends GestureRecognizerFactory<_AnyTapGestureRecognizer> { - const _AnyTapGestureRecognizerFactory({this.onAnyTapDown}); + const _AnyTapGestureRecognizerFactory({this.onAnyTapUp}); - final VoidCallback onAnyTapDown; + final VoidCallback onAnyTapUp; @override _AnyTapGestureRecognizer constructor() => _AnyTapGestureRecognizer(); @override void initializer(_AnyTapGestureRecognizer instance) { - instance.onAnyTapDown = onAnyTapDown; + instance.onAnyTapUp = onAnyTapUp; } } @@ -260,29 +260,29 @@ class _ModalBarrierGestureDetector extends StatelessWidget { const _ModalBarrierGestureDetector({ Key key, @required this.child, - @required this.onAnyTapDown, + @required this.onDismiss, }) : assert(child != null), - assert(onAnyTapDown != null), + assert(onDismiss != null), super(key: key); /// The widget below this widget in the tree. /// See [RawGestureDetector.child]. final Widget child; - /// Immediately called when a pointer causes a tap down. - /// See [_AnyTapGestureRecognizer.onAnyTapDown]. - final VoidCallback onAnyTapDown; + /// Immediately called when an event that should dismiss the modal barrier + /// has happened. + final VoidCallback onDismiss; @override Widget build(BuildContext context) { final Map gestures = { - _AnyTapGestureRecognizer: _AnyTapGestureRecognizerFactory(onAnyTapDown: onAnyTapDown), + _AnyTapGestureRecognizer: _AnyTapGestureRecognizerFactory(onAnyTapUp: onDismiss), }; return RawGestureDetector( gestures: gestures, behavior: HitTestBehavior.opaque, - semantics: _ModalBarrierSemanticsDelegate(onAnyTapDown: onAnyTapDown), + semantics: _ModalBarrierSemanticsDelegate(onDismiss: onDismiss), child: child, ); } diff --git a/packages/flutter/test/widgets/modal_barrier_test.dart b/packages/flutter/test/widgets/modal_barrier_test.dart index 8a447df0eb..6fd27d8a0f 100644 --- a/packages/flutter/test/widgets/modal_barrier_test.dart +++ b/packages/flutter/test/widgets/modal_barrier_test.dart @@ -105,16 +105,21 @@ void main() { await tester.pump(); // begin transition await tester.pump(const Duration(seconds: 1)); // end transition - // Tap on the barrier to dismiss it - await tester.tap(find.byKey(const ValueKey('barrier'))); - await tester.pump(); // begin transition - await tester.pump(const Duration(seconds: 1)); // end transition + // Press the barrier; it shouldn't dismiss yet + final TestGesture gesture = await tester.press( + find.byKey(const ValueKey('barrier')), + ); + await tester.pumpAndSettle(); // begin transition + expect(find.byKey(const ValueKey('barrier')), findsOneWidget); + // Release the pointer; the barrier should be dismissed + await gesture.up(); + await tester.pumpAndSettle(const Duration(seconds: 1)); // end transition expect(find.byKey(const ValueKey('barrier')), findsNothing, reason: 'The route should have been dismissed by tapping the barrier.'); }); - testWidgets('ModalBarrier pops the Navigator when dismissed by primary tap down', (WidgetTester tester) async { + testWidgets('ModalBarrier pops the Navigator when dismissed by non-primary tap', (WidgetTester tester) async { final Map routes = { '/': (BuildContext context) => FirstWidget(), '/modal': (BuildContext context) => SecondWidget(), @@ -130,36 +135,17 @@ void main() { await tester.pump(); // begin transition await tester.pump(const Duration(seconds: 1)); // end transition - // Press the barrier to dismiss it - await tester.press(find.byKey(const ValueKey('barrier'))); - await tester.pump(); // begin transition - await tester.pump(const Duration(seconds: 1)); // end transition - - expect(find.byKey(const ValueKey('barrier')), findsNothing, - reason: 'The route should have been dismissed by tapping the barrier.'); - }); - - testWidgets('ModalBarrier pops the Navigator when dismissed by non-primary tap down', (WidgetTester tester) async { - final Map routes = { - '/': (BuildContext context) => FirstWidget(), - '/modal': (BuildContext context) => SecondWidget(), - }; - - await tester.pumpWidget(MaterialApp(routes: routes)); - - // Initially the barrier is not visible - expect(find.byKey(const ValueKey('barrier')), findsNothing); - - // Tapping on X routes to the barrier - await tester.tap(find.text('X')); - await tester.pump(); // begin transition - await tester.pump(const Duration(seconds: 1)); // end transition - - // Press the barrier to dismiss it - await tester.press(find.byKey(const ValueKey('barrier')), buttons: kSecondaryButton); - await tester.pump(); // begin transition - await tester.pump(const Duration(seconds: 1)); // end transition + // Press the barrier; it shouldn't dismiss yet + final TestGesture gesture = await tester.press( + find.byKey(const ValueKey('barrier')), + buttons: kSecondaryButton, + ); + await tester.pumpAndSettle(); // begin transition + expect(find.byKey(const ValueKey('barrier')), findsOneWidget); + // Release the pointer; the barrier should be dismissed + await gesture.up(); + await tester.pumpAndSettle(const Duration(seconds: 1)); // end transition expect(find.byKey(const ValueKey('barrier')), findsNothing, reason: 'The route should have been dismissed by tapping the barrier.'); });