From 65724e7dd446ae96dae2a5f6fe39ee2a6489267f Mon Sep 17 00:00:00 2001 From: Daichi Fujita <68946713+fujidaiti@users.noreply.github.com> Date: Sat, 14 Sep 2024 15:28:53 +0900 Subject: [PATCH] Fix: touches are ignored at top edge of list view (#228) ## Related issues (optional) Fixes #212. ## Description This PR fixes a problem related to floating-point precision errors in the same manner as #209, taking the top edge of scrollable widgets into account (the previous fix addressed only the bottom edge). ## Summary (check all that apply) - [x] Modified / added code - [x] Modified / added tests - [ ] Modified / added examples - [ ] Modified / added others (pubspec.yaml, workflows, etc...) - [ ] Updated README - [ ] Contains breaking changes - [ ] Created / updated migration guide - [ ] Incremented version number - [ ] Updated CHANGELOG --- lib/src/internal/float_comp.dart | 12 ++++ .../scrollable/scrollable_sheet_extent.dart | 13 ++-- test/scrollable/scrollable_sheet_test.dart | 71 +++++++++++++------ 3 files changed, 67 insertions(+), 29 deletions(-) diff --git a/lib/src/internal/float_comp.dart b/lib/src/internal/float_comp.dart index 4f4acfe..bf4d1b8 100644 --- a/lib/src/internal/float_comp.dart +++ b/lib/src/internal/float_comp.dart @@ -74,4 +74,16 @@ class FloatComp { /// Returns [b] if [a] is approximately equal to [b], otherwise [a]. double roundToIfApprox(double a, double b) => isApprox(a, b) ? b : a; + + /// Rounds the given values to the nearest edge value if + /// they are approximately equal. + /// + /// If `a` is approximately equal to `b`, returns `b`. + /// If `a` is approximately equal to `c`, returns `c`. + /// Otherwise, returns `a`. + double roundToEdgeIfApprox(double a, double b, double c) { + if (isApprox(a, b)) return b; + if (isApprox(a, c)) return c; + return a; + } } diff --git a/lib/src/scrollable/scrollable_sheet_extent.dart b/lib/src/scrollable/scrollable_sheet_extent.dart index 857da63..573cc15 100644 --- a/lib/src/scrollable/scrollable_sheet_extent.dart +++ b/lib/src/scrollable/scrollable_sheet_extent.dart @@ -208,18 +208,19 @@ class ScrollableSheetExtent extends SheetExtent draggableDistance + scrollableDistance; final scrollMetricsForScrollPhysics = scrollPosition.copyWith( minScrollExtent: 0, - // How many pixels the user can scroll and drag + // How many pixels the user can scroll and drag. maxScrollExtent: maxScrollExtentForScrollPhysics, - // How many pixels the user has scrolled and dragged - pixels: FloatComp.distance(context.devicePixelRatio).roundToIfApprox( - // Round the scrollPixelsForScrollPhysics to the maxScrollExtent if - // necessary to prevents issues with floating-point precision errors. - // For example, issue #207 was caused by infinite recursion of + // How many pixels the user has scrolled and dragged. + pixels: FloatComp.distance(context.devicePixelRatio).roundToEdgeIfApprox( + // Round the scrollPixelsForScrollPhysics to 0.0 or the maxScrollExtent + // if necessary to prevents issues with floating-point precision errors. + // For example, issue #207 and #212 were caused by infinite recursion of // SheetContentScrollPositionOwner.goBallisticWithScrollPosition calls, // triggered by ScrollMetrics.outOfRange always being true in // ScrollPhysics.createBallisticSimulation due to such a floating-point // precision error. scrollPixelsForScrollPhysics, + 0, maxScrollExtentForScrollPhysics, ), ); diff --git a/test/scrollable/scrollable_sheet_test.dart b/test/scrollable/scrollable_sheet_test.dart index df28af9..6c16996 100644 --- a/test/scrollable/scrollable_sheet_test.dart +++ b/test/scrollable/scrollable_sheet_test.dart @@ -339,13 +339,16 @@ void main() { }); }); - // Regression test for https://github.com/fujidaiti/smooth_sheets/issues/207 - testWidgets('Infinite ballistic scroll activity test', (tester) async { + // Regression tests for: + // - https://github.com/fujidaiti/smooth_sheets/issues/207 + // - https://github.com/fujidaiti/smooth_sheets/issues/212 + group('Infinite ballistic scroll activity test', () { late ScrollController scrollController; late ScrollableSheetExtent sheetExtent; + late Widget testWidget; - await tester.pumpWidget( - ScrollableSheet( + setUp(() { + testWidget = ScrollableSheet( child: Builder( builder: (context) { scrollController = PrimaryScrollController.of(context); @@ -360,25 +363,47 @@ void main() { ); }, ), - ), - ); + ); + }); - scrollController.jumpTo(600.0); - await tester.pumpAndSettle(); - expect(scrollController.position.extentAfter, 0, - reason: 'Ensure that the scroll view cannot be scrolled anymore'); - - // Start a ballistic animation from a position extremely close to, - // but not equal, to the current position. - scrollController.position.correctPixels(600.000000001); - sheetExtent.goBallisticWithScrollPosition( - velocity: 0, - scrollPosition: scrollController.position as SheetContentScrollPosition, - ); - await tester.pumpAndSettle(); - expect(scrollController.position.pixels, 600.0); - expect(sheetExtent.activity, isA(), - reason: 'Should not enter an infinite recursion ' - 'of BallisticScrollDrivenSheetActivity'); + testWidgets('top edge', (tester) async { + await tester.pumpWidget(testWidget); + await tester.pumpAndSettle(); + expect(scrollController.position.pixels, 0); + + // Start a ballistic animation from a position extremely close to, + // but not equal, to the initial position. + scrollController.position.correctPixels(-0.000000001); + sheetExtent.goBallisticWithScrollPosition( + velocity: 0, + scrollPosition: scrollController.position as SheetContentScrollPosition, + ); + await tester.pumpAndSettle(); + expect(scrollController.position.pixels, 0); + expect(sheetExtent.activity, isA(), + reason: 'Should not enter an infinite recursion ' + 'of BallisticScrollDrivenSheetActivity'); + }); + + testWidgets('bottom edge', (tester) async { + await tester.pumpWidget(testWidget); + scrollController.jumpTo(600.0); + await tester.pumpAndSettle(); + expect(scrollController.position.extentAfter, 0, + reason: 'Ensure that the scroll view cannot be scrolled anymore'); + + // Start a ballistic animation from a position extremely close to, + // but not equal, to the current position. + scrollController.position.correctPixels(600.000000001); + sheetExtent.goBallisticWithScrollPosition( + velocity: 0, + scrollPosition: scrollController.position as SheetContentScrollPosition, + ); + await tester.pumpAndSettle(); + expect(scrollController.position.pixels, 600.0); + expect(sheetExtent.activity, isA(), + reason: 'Should not enter an infinite recursion ' + 'of BallisticScrollDrivenSheetActivity'); + }); }); }