From 4adc21a277633b49bd689b13e4f3b0f78d13ff55 Mon Sep 17 00:00:00 2001 From: R-Bharathraj Date: Fri, 18 Sep 2026 17:01:17 +0530 Subject: [PATCH] refactor: rely on platform scroll physics and add scroll behavior tests --- README.md | 13 +++-- lib/main.dart | 24 ++++----- test/scroll_behaviour_test.dart | 91 +++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 18 deletions(-) create mode 100644 test/scroll_behaviour_test.dart diff --git a/README.md b/README.md index 3e8e7cc..da6756e 100644 --- a/README.md +++ b/README.md @@ -95,10 +95,15 @@ would mean scrolling back to the top just to change what you are looking at. Shifts keeps its whole header fixed for the same reason — the Booked / Open / History switch has to stay reachable. -Scrolling uses bouncing physics on every platform (`KrowScrollBehavior`). Under -a pinned header a rubber-band edge tells you that you have reached the end of -the content rather than the end of the screen; clamping just stops dead. The -overscroll glow is turned off, since the bounce already says it. +Scroll physics is left to the platform. Forcing bouncing everywhere looked +smoother on a long list and broke every short one: `ScrollView` hands any +vertical list without a controller an `AlwaysScrollableScrollPhysics`, applied +over whatever the behaviour returns, so the drag is accepted however little +content there is. Bouncing then let a list that already fits be dragged away +from the header, leaving a gap down the screen. Android's clamping accepts the +same drag and pins it to the boundary; iOS bounces, which is its own +convention. `KrowScrollBehavior` now only adds mouse and trackpad dragging for +the web and desktop builds. Two deliberate exceptions: diff --git a/lib/main.dart b/lib/main.dart index 255c641..1828e77 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -46,12 +46,16 @@ class KrowApp extends StatelessWidget { class KrowScrollBehavior extends MaterialScrollBehavior { const KrowScrollBehavior(); - /// Bouncing on every platform. The lists here sit under a pinned header, and - /// a rubber-band edge shows you have reached the end of the content rather - /// than the end of the screen — clamping just stops dead against the header. - @override - ScrollPhysics getScrollPhysics(BuildContext context) => - const BouncingScrollPhysics(parent: AlwaysScrollableScrollPhysics()); + // Scroll physics is deliberately left to the platform. + // + // Forcing BouncingScrollPhysics everywhere looked smoother on a long list + // and broke every short one: ScrollView's constructor hands any vertical + // list without a controller an AlwaysScrollableScrollPhysics, which is + // applied over whatever the behaviour returns and answers + // shouldAcceptUserOffset true regardless of content. Bouncing then let + // content that already fits be dragged away from the header, leaving a gap + // down the screen. Android's clamping accepts the same drag and pins it to + // the boundary, so nothing moves; iOS bounces, which is its own convention. /// Let the web and desktop builds be dragged with a mouse or trackpad, not /// only scrolled with a wheel. @@ -62,12 +66,4 @@ class KrowScrollBehavior extends MaterialScrollBehavior { PointerDeviceKind.trackpad, PointerDeviceKind.stylus, }; - - /// The bounce is the end-of-list signal; a glow on top of it is noise. - @override - Widget buildOverscrollIndicator( - BuildContext context, - Widget child, - ScrollableDetails details, - ) => child; } diff --git a/test/scroll_behaviour_test.dart b/test/scroll_behaviour_test.dart new file mode 100644 index 0000000..e4249dc --- /dev/null +++ b/test/scroll_behaviour_test.dart @@ -0,0 +1,91 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:krow_worker_app/main.dart'; + +/// A list whose content is shorter than the viewport, so there is nothing to +/// scroll to. +Widget shortList() => MaterialApp( + scrollBehavior: const KrowScrollBehavior(), + home: Scaffold( + body: ListView( + children: const [SizedBox(height: 100, child: Text('only item'))], + ), + ), +); + +Widget longList() => MaterialApp( + scrollBehavior: const KrowScrollBehavior(), + home: Scaffold( + body: ListView( + children: [ + for (var i = 0; i < 40; i++) SizedBox(height: 100, child: Text('$i')), + ], + ), + ), +); + +ScrollPosition positionOf(WidgetTester tester) => + tester.state(find.byType(Scrollable)).position; + +void main() { + // TargetPlatformVariant sets and restores the override around each test; + // assigning it by hand trips the framework's debug-variable invariant check. + final android = TargetPlatformVariant.only(TargetPlatform.android); + final ios = TargetPlatformVariant.only(TargetPlatform.iOS); + + group('on Android', () { + testWidgets('content that already fits does not move when dragged', ( + tester, + ) async { + await tester.pumpWidget(shortList()); + final position = positionOf(tester); + + // Every vertical list without a controller is handed an + // AlwaysScrollableScrollPhysics by ScrollView itself, so the drag is + // accepted whatever the content. Clamping is what pins it to the + // boundary. Forcing bouncing globally instead let a one-item list be + // dragged hundreds of pixels down, leaving a gap under the header. + await tester.drag(find.text('only item'), const Offset(0, 300)); + await tester.pump(); + + expect(position.pixels, 0); + }, variant: android); + + testWidgets('a list that overflows still scrolls', (tester) async { + await tester.pumpWidget(longList()); + final position = positionOf(tester); + + await tester.drag(find.text('0'), const Offset(0, -200)); + await tester.pump(); + + expect(position.pixels, greaterThan(0)); + }, variant: android); + + testWidgets('and does not overscroll past the top', (tester) async { + await tester.pumpWidget(longList()); + final position = positionOf(tester); + + await tester.drag(find.byType(Scrollable), const Offset(0, 400)); + await tester.pump(); + + expect(position.pixels, 0); + }, variant: android); + }); + + group('on iOS', () { + testWidgets('short content rubber-bands, which is the convention there', ( + tester, + ) async { + await tester.pumpWidget(shortList()); + final position = positionOf(tester); + + await tester.drag(find.text('only item'), const Offset(0, 300)); + await tester.pump(); + expect(position.pixels, lessThan(0)); + + // And springs back on release, rather than leaving the gap behind. + await tester.pumpAndSettle(); + expect(position.pixels, 0); + }, variant: ios); + }); +}