From 49332da76a412e1c5945636e1742f39c78c0a8cc Mon Sep 17 00:00:00 2001 From: nt4f04uNd Date: Mon, 20 Sep 2021 23:50:32 +0300 Subject: [PATCH] Fix Dismissible confirmDismiss errors (#88672) * Fix Dismissible confirmDismiss errors * + * refactor test * fix rebase * remove new line --- .../flutter/lib/src/widgets/dismissible.dart | 38 +++- .../test/widgets/dismissible_test.dart | 166 ++++++++++++++++-- 2 files changed, 183 insertions(+), 21 deletions(-) diff --git a/packages/flutter/lib/src/widgets/dismissible.dart b/packages/flutter/lib/src/widgets/dismissible.dart index eaeb339c1b..dc5748ceac 100644 --- a/packages/flutter/lib/src/widgets/dismissible.dart +++ b/packages/flutter/lib/src/widgets/dismissible.dart @@ -128,6 +128,8 @@ class Dismissible extends StatefulWidget { /// Gives the app an opportunity to confirm or veto a pending dismissal. /// + /// The widget cannot be dragged again until the returned future resolves. + /// /// If the returned Future completes true, then this widget will be /// dismissed, otherwise it will be moved back to its original location. /// @@ -263,6 +265,7 @@ class _DismissibleState extends State with TickerProviderStateMixin Animation? _resizeAnimation; double _dragExtent = 0.0; + bool _confirming = false; bool _dragUnderway = false; Size? _sizePriorToCollapse; @@ -308,6 +311,8 @@ class _DismissibleState extends State with TickerProviderStateMixin } void _handleDragStart(DragStartDetails details) { + if (_confirming) + return; _dragUnderway = true; if (_moveController!.isAnimating) { _dragExtent = _moveController!.value * _overallDragAxisExtent * _dragExtent.sign; @@ -426,12 +431,12 @@ class _DismissibleState extends State with TickerProviderStateMixin return _FlingGestureKind.reverse; } - Future _handleDragEnd(DragEndDetails details) async { + void _handleDragEnd(DragEndDetails details) { if (!_isActive || _moveController!.isAnimating) return; _dragUnderway = false; - if (_moveController!.isCompleted && await _confirmStartResizeAnimation() == true) { - _startResizeAnimation(); + if (_moveController!.isCompleted) { + _handleMoveCompleted(); return; } final double flingVelocity = _directionIsXAxis ? details.velocity.pixelsPerSecond.dx : details.velocity.pixelsPerSecond.dy; @@ -466,24 +471,41 @@ class _DismissibleState extends State with TickerProviderStateMixin Future _handleDismissStatusChanged(AnimationStatus status) async { if (status == AnimationStatus.completed && !_dragUnderway) { - if (await _confirmStartResizeAnimation() == true) + await _handleMoveCompleted(); + } + if (mounted) { + updateKeepAlive(); + } + } + + Future _handleMoveCompleted() async { + if ((widget.dismissThresholds[_dismissDirection] ?? _kDismissThreshold) >= 1.0) { + _moveController!.reverse(); + return; + } + final bool result = await _confirmStartResizeAnimation(); + if (mounted) { + if (result) _startResizeAnimation(); else _moveController!.reverse(); } - updateKeepAlive(); } - Future _confirmStartResizeAnimation() async { + Future _confirmStartResizeAnimation() async { if (widget.confirmDismiss != null) { + _confirming = true; final DismissDirection direction = _dismissDirection; - return widget.confirmDismiss!(direction); + try { + return await widget.confirmDismiss!(direction) ?? false; + } finally { + _confirming = false; + } } return true; } void _startResizeAnimation() { - assert(_moveController != null); assert(_moveController!.isCompleted); assert(_resizeController == null); assert(_sizePriorToCollapse == null); diff --git a/packages/flutter/test/widgets/dismissible_test.dart b/packages/flutter/test/widgets/dismissible_test.dart index 438ceeb7de..b9d7444f30 100644 --- a/packages/flutter/test/widgets/dismissible_test.dart +++ b/packages/flutter/test/widgets/dismissible_test.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; + import 'package:flutter/gestures.dart' show DragStartBehavior; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -19,6 +21,7 @@ Widget buildTest({ TextDirection textDirection = TextDirection.ltr, Future Function(BuildContext context, DismissDirection direction)? confirmDismiss, ScrollController? controller, + ScrollPhysics? scrollPhysics, Widget? background, }) { return Directionality( @@ -59,6 +62,7 @@ Widget buildTest({ return Container( padding: const EdgeInsets.all(10.0), child: ListView( + physics: scrollPhysics, controller: controller, dragStartBehavior: DragStartBehavior.down, scrollDirection: scrollDirection, @@ -83,23 +87,23 @@ Future dismissElement(WidgetTester tester, Finder finder, { required AxisD // getTopRight() returns a point that's just beyond itemWidget's right // edge and outside the Dismissible event listener's bounds. downLocation = tester.getTopRight(finder) + const Offset(-0.1, 0.0); - upLocation = tester.getTopLeft(finder); + upLocation = tester.getTopLeft(finder) + const Offset(-0.1, 0.0); break; case AxisDirection.right: // we do the same thing here to keep the test symmetric downLocation = tester.getTopLeft(finder) + const Offset(0.1, 0.0); - upLocation = tester.getTopRight(finder); + upLocation = tester.getTopRight(finder) + const Offset(0.1, 0.0); break; case AxisDirection.up: // getBottomLeft() returns a point that's just below itemWidget's bottom // edge and outside the Dismissible event listener's bounds. downLocation = tester.getBottomLeft(finder) + const Offset(0.0, -0.1); - upLocation = tester.getTopLeft(finder); + upLocation = tester.getTopLeft(finder) + const Offset(0.0, -0.1); break; case AxisDirection.down: // again with doing the same here for symmetry downLocation = tester.getTopLeft(finder) + const Offset(0.1, 0.0); - upLocation = tester.getBottomLeft(finder); + upLocation = tester.getBottomLeft(finder) + const Offset(0.1, 0.0); break; } @@ -146,12 +150,7 @@ Future dismissItem( expect(itemFinder, findsOneWidget); await mechanism(tester, itemFinder, gestureDirection: gestureDirection); - - await tester.pump(); // start the slide - await tester.pump(const Duration(seconds: 1)); // finish the slide and start shrinking... - await tester.pump(); // first frame of shrinking animation - await tester.pump(const Duration(seconds: 1)); // finish the shrinking and call the callback... - await tester.pump(); // rebuild after the callback removes the entry + await tester.pumpAndSettle(); } Future checkFlingItemBeforeMovementEnd( @@ -779,7 +778,145 @@ void main() { expect(confirmDismissDirection, DismissDirection.endToStart); }); - testWidgets('setState that does not remove the Dismissible from tree should throws Error', (WidgetTester tester) async { + testWidgets('Pending confirmDismiss does not cause errors', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/54990 + + late Completer completer; + Widget buildFrame() { + completer = Completer(); + return buildTest( + confirmDismiss: (BuildContext context, DismissDirection dismissDirection) { + return completer.future; + }, + ); + } + + // false for _handleDragEnd - when dragged to the end and released + + await tester.pumpWidget(buildFrame()); + + await dismissItem(tester, 0, gestureDirection: AxisDirection.right); + expect(find.text('0'), findsOneWidget); + expect(dismissedItems, isEmpty); + + await tester.pumpWidget(const SizedBox()); + completer.complete(false); + await tester.pump(); + + // true for _handleDragEnd - when dragged to the end and released + + await tester.pumpWidget(buildFrame()); + + await dismissItem(tester, 0, gestureDirection: AxisDirection.right); + expect(find.text('0'), findsOneWidget); + expect(dismissedItems, isEmpty); + + await tester.pumpWidget(const SizedBox()); + completer.complete(true); + await tester.pump(); + + // false for _handleDismissStatusChanged - when fling reaches the end + + await tester.pumpWidget(buildFrame()); + + await dismissItem(tester, 0, gestureDirection: AxisDirection.right, mechanism: flingElement); + expect(find.text('0'), findsOneWidget); + expect(dismissedItems, isEmpty); + + await tester.pumpWidget(const SizedBox()); + completer.complete(false); + await tester.pump(); + + // true for _handleDismissStatusChanged - when fling reaches the end + + await tester.pumpWidget(buildFrame()); + + await dismissItem(tester, 0, gestureDirection: AxisDirection.right, mechanism: flingElement); + expect(find.text('0'), findsOneWidget); + expect(dismissedItems, isEmpty); + + await tester.pumpWidget(const SizedBox()); + completer.complete(true); + await tester.pump(); + }); + + testWidgets('Dismissible cannot be dragged with pending confirmDismiss', (WidgetTester tester) async { + final Completer completer = Completer(); + await tester.pumpWidget( + buildTest( + confirmDismiss: (BuildContext context, DismissDirection dismissDirection) { + return completer.future; + }, + ), + ); + + // Trigger confirmDismiss call. + await dismissItem(tester, 0, gestureDirection: AxisDirection.right); + final Offset position = tester.getTopLeft(find.text('0')); + + // Try to move and verify it has not moved. + Offset dragAt = tester.getTopLeft(find.text('0')); + dragAt = Offset(100.0, dragAt.dy); + final TestGesture gesture = await tester.startGesture(dragAt); + await gesture.moveTo(dragAt + const Offset(100.0, 0.0)); + await gesture.up(); + await tester.pump(); + expect(tester.getTopLeft(find.text('0')), position); + }); + + testWidgets('Drag to end and release - items does not get stuck if confirmDismiss returns false', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/87556 + + final Completer completer = Completer(); + await tester.pumpWidget( + buildTest( + confirmDismiss: (BuildContext context, DismissDirection dismissDirection) { + return completer.future; + }, + ), + ); + + final Offset position = tester.getTopLeft(find.text('0')); + await dismissItem(tester, 0, gestureDirection: AxisDirection.right); + completer.complete(false); + await tester.pumpAndSettle(); + expect(tester.getTopLeft(find.text('0')), position); + }); + + testWidgets('Dismissible with null resizeDuration calls onDismissed immediately', (WidgetTester tester) async { + bool resized = false; + bool dismissed = false; + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Dismissible( + dragStartBehavior: DragStartBehavior.down, + key: UniqueKey(), + direction: DismissDirection.horizontal, + resizeDuration: null, + onDismissed: (DismissDirection direction) { + dismissed = true; + }, + onResize: () { + resized = true; + }, + child: const SizedBox( + width: 100.0, + height: 100.0, + child: Text('0'), + ), + ), + ), + ); + + await dismissElement(tester, find.text('0'), gestureDirection: AxisDirection.right); + await tester.pump(); + expect(dismissed, true); + expect(resized, false); + }); + + testWidgets('setState that does not remove the Dismissible from tree should throw Error', (WidgetTester tester) async { const Axis scrollDirection = Axis.vertical; const DismissDirection dismissDirection = DismissDirection.horizontal; @@ -912,7 +1049,10 @@ void main() { }); testWidgets('DismissDirection.none does not trigger dismiss', (WidgetTester tester) async { - await tester.pumpWidget(buildTest(dismissDirection: DismissDirection.none)); + await tester.pumpWidget(buildTest( + dismissDirection: DismissDirection.none, + scrollPhysics: const NeverScrollableScrollPhysics(), + )); expect(dismissedItems, isEmpty); await dismissItem(tester, 0, gestureDirection: AxisDirection.left); @@ -941,7 +1081,7 @@ void main() { await dismissItem(tester, 0, gestureDirection: AxisDirection.down); expect(controller.offset, 0.0); await dismissItem(tester, 0, gestureDirection: AxisDirection.up); - expect(controller.offset, 99.9); + expect(controller.offset, 100.0); controller.dispose(); }); }