From 492bfddd9fdb5143c3fadbd5b1dea049253d17cd Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Mon, 10 Oct 2022 12:57:02 -0700 Subject: [PATCH] Fix selection area causes small scrollables to bounce (#112816) --- .../flutter/lib/src/widgets/scrollable.dart | 48 +++++++++------ .../widgets/scrollable_selection_test.dart | 58 +++++++++++++++++-- 2 files changed, 82 insertions(+), 24 deletions(-) diff --git a/packages/flutter/lib/src/widgets/scrollable.dart b/packages/flutter/lib/src/widgets/scrollable.dart index 1be6deccfec..0aee4f1affa 100644 --- a/packages/flutter/lib/src/widgets/scrollable.dart +++ b/packages/flutter/lib/src/widgets/scrollable.dart @@ -989,6 +989,11 @@ class EdgeDraggingAutoScroller { scrollRenderBox.getTransformTo(null), Rect.fromLTWH(0, 0, scrollRenderBox.size.width, scrollRenderBox.size.height) ); + assert( + globalRect.size.width >= _dragTargetRelatedToScrollOrigin.size.width && + globalRect.size.height >= _dragTargetRelatedToScrollOrigin.size.height, + 'Drag target size is larger than scrollable size, which may cause bouncing', + ); _scrolling = true; double? newOffset; const double overDragMax = 20.0; @@ -1000,23 +1005,27 @@ class EdgeDraggingAutoScroller { final double proxyStart = _offsetExtent(_dragTargetRelatedToScrollOrigin.topLeft, _scrollDirection); final double proxyEnd = _offsetExtent(_dragTargetRelatedToScrollOrigin.bottomRight, _scrollDirection); - late double overDrag; - if (_axisDirection == AxisDirection.up || _axisDirection == AxisDirection.left) { - if (proxyEnd > viewportEnd && scrollable.position.pixels > scrollable.position.minScrollExtent) { - overDrag = math.max(proxyEnd - viewportEnd, overDragMax); - newOffset = math.max(scrollable.position.minScrollExtent, scrollable.position.pixels - overDrag); - } else if (proxyStart < viewportStart && scrollable.position.pixels < scrollable.position.maxScrollExtent) { - overDrag = math.max(viewportStart - proxyStart, overDragMax); - newOffset = math.min(scrollable.position.maxScrollExtent, scrollable.position.pixels + overDrag); - } - } else { - if (proxyStart < viewportStart && scrollable.position.pixels > scrollable.position.minScrollExtent) { - overDrag = math.max(viewportStart - proxyStart, overDragMax); - newOffset = math.max(scrollable.position.minScrollExtent, scrollable.position.pixels - overDrag); - } else if (proxyEnd > viewportEnd && scrollable.position.pixels < scrollable.position.maxScrollExtent) { - overDrag = math.max(proxyEnd - viewportEnd, overDragMax); - newOffset = math.min(scrollable.position.maxScrollExtent, scrollable.position.pixels + overDrag); - } + switch (_axisDirection) { + case AxisDirection.up: + case AxisDirection.left: + if (proxyEnd > viewportEnd && scrollable.position.pixels > scrollable.position.minScrollExtent) { + final double overDrag = math.min(proxyEnd - viewportEnd, overDragMax); + newOffset = math.max(scrollable.position.minScrollExtent, scrollable.position.pixels - overDrag); + } else if (proxyStart < viewportStart && scrollable.position.pixels < scrollable.position.maxScrollExtent) { + final double overDrag = math.min(viewportStart - proxyStart, overDragMax); + newOffset = math.min(scrollable.position.maxScrollExtent, scrollable.position.pixels + overDrag); + } + break; + case AxisDirection.right: + case AxisDirection.down: + if (proxyStart < viewportStart && scrollable.position.pixels > scrollable.position.minScrollExtent) { + final double overDrag = math.min(viewportStart - proxyStart, overDragMax); + newOffset = math.max(scrollable.position.minScrollExtent, scrollable.position.pixels - overDrag); + } else if (proxyEnd > viewportEnd && scrollable.position.pixels < scrollable.position.maxScrollExtent) { + final double overDrag = math.min(proxyEnd - viewportEnd, overDragMax); + newOffset = math.min(scrollable.position.maxScrollExtent, scrollable.position.pixels + overDrag); + } + break; } if (newOffset == null || (newOffset - scrollable.position.pixels).abs() < 1.0) { @@ -1055,7 +1064,10 @@ class _ScrollableSelectionContainerDelegate extends MultiSelectableSelectionCont _position.addListener(_scheduleLayoutChange); } - static const double _kDefaultDragTargetSize = 200; + // Pointer drag is a single point, it should not have a size. + static const double _kDefaultDragTargetSize = 0; + + // An eye-balled value for a smooth scrolling speed. static const double _kDefaultSelectToScrollVelocityScalar = 30; final ScrollableState state; diff --git a/packages/flutter/test/widgets/scrollable_selection_test.dart b/packages/flutter/test/widgets/scrollable_selection_test.dart index dd5d8056f54..c8ea17290ce 100644 --- a/packages/flutter/test/widgets/scrollable_selection_test.dart +++ b/packages/flutter/test/widgets/scrollable_selection_test.dart @@ -125,7 +125,8 @@ void main() { expect(controller.offset, 0.0); double previousOffset = controller.offset; - await gesture.moveTo(tester.getBottomRight(find.byType(ListView))); + // Scrollable only auto scroll if the drag passes the boundary. + await gesture.moveTo(tester.getBottomRight(find.byType(ListView)) + const Offset(0, 20)); await tester.pump(); await tester.pump(const Duration(seconds: 1)); expect(controller.offset > previousOffset, isTrue); @@ -150,6 +151,50 @@ void main() { await gesture.up(); }); + testWidgets('select to scroll works for small scrollable', (WidgetTester tester) async { + final ScrollController controller = ScrollController(); + await tester.pumpWidget(MaterialApp( + home: SelectionArea( + selectionControls: materialTextSelectionControls, + child: Scaffold( + body: SizedBox( + height: 10, + child: ListView.builder( + controller: controller, + itemCount: 100, + itemBuilder: (BuildContext context, int index) { + return Text('Item $index'); + }, + ), + ), + ), + ), + )); + + final RenderParagraph paragraph1 = tester.renderObject(find.descendant(of: find.text('Item 0'), matching: find.byType(RichText))); + final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph1, 2), kind: ui.PointerDeviceKind.mouse); + addTearDown(gesture.removePointer); + await tester.pump(); + expect(controller.offset, 0.0); + double previousOffset = controller.offset; + + // Scrollable only auto scroll if the drag passes the boundary + await gesture.moveTo(tester.getBottomRight(find.byType(ListView)) + const Offset(0, 20)); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 100)); + expect(controller.offset > previousOffset, isTrue); + previousOffset = controller.offset; + + await tester.pump(); + await tester.pump(const Duration(milliseconds: 100)); + expect(controller.offset > previousOffset, isTrue); + await gesture.up(); + + // Shouldn't be stuck if gesture is up. + await tester.pumpAndSettle(const Duration(seconds: 1)); + expect(tester.takeException(), isNull); + }); + testWidgets('select to scroll backward', (WidgetTester tester) async { final ScrollController controller = ScrollController(); await tester.pumpWidget(MaterialApp( @@ -174,7 +219,7 @@ void main() { expect(controller.offset, 4000); double previousOffset = controller.offset; - await gesture.moveTo(tester.getTopLeft(find.byType(ListView))); + await gesture.moveTo(tester.getTopLeft(find.byType(ListView)) + const Offset(0, -20)); await tester.pump(); await tester.pump(const Duration(seconds: 1)); expect(controller.offset < previousOffset, isTrue); @@ -220,7 +265,8 @@ void main() { expect(controller.offset, 0.0); double previousOffset = controller.offset; - await gesture.moveTo(tester.getBottomRight(find.byType(ListView))); + // Scrollable only auto scroll if the drag passes the boundary + await gesture.moveTo(tester.getBottomRight(find.byType(ListView)) + const Offset(20, 0)); await tester.pump(); await tester.pump(const Duration(seconds: 1)); expect(controller.offset > previousOffset, isTrue); @@ -268,7 +314,7 @@ void main() { expect(controller.offset, 2080); double previousOffset = controller.offset; - await gesture.moveTo(tester.getTopLeft(find.byType(ListView))); + await gesture.moveTo(tester.getTopLeft(find.byType(ListView)) + const Offset(-10, 0)); await tester.pump(); await tester.pump(const Duration(seconds: 1)); expect(controller.offset < previousOffset, isTrue); @@ -427,8 +473,8 @@ void main() { expect(controller.offset, 0.0); double previousOffset = controller.offset; - - await gesture.moveTo(tester.getBottomRight(find.byType(ListView))); + // Scrollable only auto scroll if the drag passes the boundary + await gesture.moveTo(tester.getBottomRight(find.byType(ListView)) + const Offset(0, 40)); await tester.pump(); await tester.pump(const Duration(seconds: 1)); expect(controller.offset > previousOffset, isTrue);