Skip to content

Commit e4c18b7

Browse files
HansMullerHansMuller
authored
Improve the behavior of scrollbar drag-scrolls triggered by the trackpad (#150275)
Corrects some problems related to trackpad scrolls introduced by flutter/flutter#146654. Fixes flutter/flutter#149999 Fixes #150342 Fixes flutter/flutter#150236
1 parent e706d7d commit e4c18b7

File tree

2 files changed

+277
-23
lines changed

2 files changed

+277
-23
lines changed

‎packages/flutter/lib/src/widgets/scrollbar.dart

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,6 +1324,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
13241324
final GlobalKey _scrollbarPainterKey = GlobalKey();
13251325
bool _hoverIsActive = false;
13261326
Drag? _thumbDrag;
1327+
bool _maxScrollExtentPermitsScrolling = false;
13271328
ScrollHoldController? _thumbHold;
13281329
Axis? _axis;
13291330
final GlobalKey<RawGestureDetectorState> _gestureDetectorKey = GlobalKey<RawGestureDetectorState>();
@@ -1618,7 +1619,8 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
16181619
// Convert primaryDelta, the amount that the scrollbar moved since the last
16191620
// time when drag started or last updated, into the coordinate space of the scroll
16201621
// position.
1621-
double scrollOffsetGlobal = scrollbarPainter.getTrackToScroll(primaryDeltaFromDragStart + _startDragThumbOffset!);
1622+
double scrollOffsetGlobal = scrollbarPainter.getTrackToScroll(_startDragThumbOffset! + primaryDeltaFromDragStart);
1623+
16221624
if (primaryDeltaFromDragStart > 0 && scrollOffsetGlobal < position.pixels
16231625
|| primaryDeltaFromDragStart < 0 && scrollOffsetGlobal > position.pixels) {
16241626
// Adjust the position value if the scrolling direction conflicts with
@@ -1642,7 +1644,6 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
16421644
case TargetPlatform.android:
16431645
// We can only drag the scrollbar into overscroll on mobile
16441646
// platforms, and only then if the physics allow it.
1645-
break;
16461647
}
16471648
final bool isReversed = axisDirectionIsReversed(position.axisDirection);
16481649
return isReversed ? newPosition - position.pixels : position.pixels - newPosition;
@@ -1760,25 +1761,22 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
17601761
}
17611762

17621763
// On mobile platforms flinging the scrollbar thumb causes a ballistic
1763-
// scroll, just like it via a touch drag.
1764+
// scroll, just like it does via a touch drag. Likewise for desktops when
1765+
// dragging on the trackpad or with a stylus.
17641766
final TargetPlatform platform = ScrollConfiguration.of(context).getPlatform(context);
1765-
final (Velocity adjustedVelocity, double primaryVelocity) = switch (platform) {
1766-
TargetPlatform.iOS || TargetPlatform.android => (
1767-
-velocity,
1768-
switch (direction) {
1769-
Axis.horizontal => -velocity.pixelsPerSecond.dx,
1770-
Axis.vertical => -velocity.pixelsPerSecond.dy,
1771-
},
1772-
),
1773-
_ => (Velocity.zero, 0),
1767+
final Velocity adjustedVelocity = switch (platform) {
1768+
TargetPlatform.iOS || TargetPlatform.android => -velocity,
1769+
_ => Velocity.zero,
17741770
};
1775-
17761771
final RenderBox renderBox = _scrollbarPainterKey.currentContext!.findRenderObject()! as RenderBox;
17771772
final DragEndDetails details = DragEndDetails(
17781773
localPosition: localPosition,
17791774
globalPosition: renderBox.localToGlobal(localPosition),
17801775
velocity: adjustedVelocity,
1781-
primaryVelocity: primaryVelocity,
1776+
primaryVelocity: switch (direction) {
1777+
Axis.horizontal => adjustedVelocity.pixelsPerSecond.dx,
1778+
Axis.vertical => adjustedVelocity.pixelsPerSecond.dy,
1779+
},
17821780
);
17831781

17841782
_thumbDrag?.end(details);
@@ -1869,6 +1867,9 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
18691867
if (metrics.axis != _axis) {
18701868
setState(() { _axis = metrics.axis; });
18711869
}
1870+
if (_maxScrollExtentPermitsScrolling != notification.metrics.maxScrollExtent > 0.0) {
1871+
setState(() { _maxScrollExtentPermitsScrolling = !_maxScrollExtentPermitsScrolling; });
1872+
}
18721873

18731874
return false;
18741875
}
@@ -1891,8 +1892,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
18911892
return false;
18921893
}
18931894

1894-
if (notification is ScrollUpdateNotification ||
1895-
notification is OverscrollNotification) {
1895+
if (notification is ScrollUpdateNotification || notification is OverscrollNotification) {
18961896
// Any movements always makes the scrollbar start showing up.
18971897
if (!_fadeoutAnimationController.isForwardOrCompleted) {
18981898
_fadeoutAnimationController.forward();
@@ -1915,16 +1915,26 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
19151915
handleThumbPress();
19161916
}
19171917

1918+
// The protected RawScrollbar API methods - handleThumbPressStart,
1919+
// handleThumbPressUpdate, handleThumbPressEnd - all depend on a
1920+
// localPosition parameter that defines the event's location relative
1921+
// to the scrollbar. Ensure that the localPosition is reported consistently,
1922+
// even if the source of the event is a trackpad or a stylus.
1923+
Offset _globalToScrollbar(Offset offset) {
1924+
final RenderBox renderBox = _scrollbarPainterKey.currentContext!.findRenderObject()! as RenderBox;
1925+
return renderBox.globalToLocal(offset);
1926+
}
1927+
19181928
void _handleThumbDragStart(DragStartDetails details) {
1919-
handleThumbPressStart(details.localPosition);
1929+
handleThumbPressStart(_globalToScrollbar(details.globalPosition));
19201930
}
19211931

19221932
void _handleThumbDragUpdate(DragUpdateDetails details) {
1923-
handleThumbPressUpdate(details.localPosition);
1933+
handleThumbPressUpdate(_globalToScrollbar(details.globalPosition));
19241934
}
19251935

19261936
void _handleThumbDragEnd(DragEndDetails details) {
1927-
handleThumbPressEnd(details.localPosition, details.velocity);
1937+
handleThumbPressEnd(_globalToScrollbar(details.globalPosition), details.velocity);
19281938
}
19291939

19301940
void _handleThumbDragCancel() {
@@ -2251,6 +2261,11 @@ class _VerticalThumbDragGestureRecognizer extends VerticalDragGestureRecognizer
22512261

22522262
final GlobalKey _customPaintKey;
22532263

2264+
@override
2265+
bool isPointerPanZoomAllowed(PointerPanZoomStartEvent event) {
2266+
return false;
2267+
}
2268+
22542269
@override
22552270
bool isPointerAllowed(PointerEvent event) {
22562271
return _isThumbEvent(_customPaintKey, event) && super.isPointerAllowed(event);
@@ -2265,6 +2280,11 @@ class _HorizontalThumbDragGestureRecognizer extends HorizontalDragGestureRecogni
22652280

22662281
final GlobalKey _customPaintKey;
22672282

2283+
@override
2284+
bool isPointerPanZoomAllowed(PointerPanZoomStartEvent event) {
2285+
return false;
2286+
}
2287+
22682288
@override
22692289
bool isPointerAllowed(PointerEvent event) {
22702290
return _isThumbEvent(_customPaintKey, event) && super.isPointerAllowed(event);

‎packages/flutter/test/widgets/scrollbar_test.dart

Lines changed: 238 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3240,20 +3240,254 @@ The provided ScrollController cannot be shared by multiple ScrollView widgets.''
32403240
expect(scrollController.offset, 0.0);
32413241
expect(scrollController.position.maxScrollExtent, 0.0);
32423242

3243-
await tester.drag(find.byType(SingleChildScrollView), const Offset(0, -100), kind: PointerDeviceKind.trackpad);
3243+
await tester.trackpadFling(find.byType(SingleChildScrollView), const Offset(0, -100), 500);
3244+
await tester.pumpAndSettle();
3245+
expect(scrollController.offset, 0.0);
3246+
3247+
await tester.trackpadFling(find.byType(SingleChildScrollView), const Offset(0, 100), 500);
32443248
await tester.pumpAndSettle();
32453249
expect(scrollController.offset, 0.0);
3246-
expect(scrollController.position.maxScrollExtent, 0.0);
32473250

32483251
await tester.pumpWidget(buildFrame(700));
32493252
await tester.pumpAndSettle();
32503253
expect(scrollController.offset, 0.0);
32513254
expect(scrollController.position.maxScrollExtent, 100.0);
32523255

3253-
await tester.drag(find.byType(SingleChildScrollView), const Offset(0, -100), kind: PointerDeviceKind.trackpad);
3256+
await tester.trackpadFling(find.byType(SingleChildScrollView), const Offset(0, -100), 500);
32543257
await tester.pumpAndSettle();
32553258
expect(scrollController.offset, 100.0);
3256-
expect(scrollController.position.maxScrollExtent, 100.0);
32573259

3260+
await tester.trackpadFling(find.byType(SingleChildScrollView), const Offset(0, 100), 500);
3261+
await tester.pumpAndSettle();
3262+
expect(scrollController.offset, 0.0);
3263+
});
3264+
3265+
testWidgets('Desktop trackpad drag direction: -X,-Y produces positive scroll offset changes', (WidgetTester tester) async {
3266+
// Regression test for https://github.com/flutter/flutter/issues/149999.
3267+
// This test doesn't strictly test the scrollbar: trackpad flings
3268+
// that begin in the center of the scrollable are handled by the
3269+
// scrollable, not the scrollbar. However: the scrollbar widget does
3270+
// contain the scrollable and this test verifies that it doesn't
3271+
// inadvertantly handle thumb down/start/update/end gestures due
3272+
// to trackpad pan/zoom events. Those callbacks are prevented by
3273+
// the overrides of isPointerPanZoomAllowed in the scrollbar
3274+
// gesture recognizers.
3275+
3276+
final ScrollController scrollController = ScrollController();
3277+
addTearDown(scrollController.dispose);
3278+
3279+
Widget buildFrame(Axis scrollDirection) {
3280+
return Directionality(
3281+
textDirection: TextDirection.ltr,
3282+
child: MediaQuery(
3283+
data: const MediaQueryData(),
3284+
child: RawScrollbar(
3285+
controller: scrollController,
3286+
child: SingleChildScrollView(
3287+
scrollDirection: scrollDirection,
3288+
controller: scrollController,
3289+
child: const SizedBox(width: 1600, height: 1200),
3290+
),
3291+
),
3292+
),
3293+
);
3294+
}
3295+
3296+
// Vertical scrolling: -Y trackpad motion produces positive scroll offset change
3297+
3298+
await tester.pumpWidget(buildFrame(Axis.vertical));
3299+
expect(scrollController.offset, 0);
3300+
expect(scrollController.position.maxScrollExtent, 600);
3301+
3302+
await tester.trackpadFling(find.byType(SingleChildScrollView), const Offset(0, -600), 500);
3303+
await tester.pumpAndSettle();
3304+
expect(scrollController.offset, 600);
3305+
3306+
await tester.trackpadFling(find.byType(SingleChildScrollView), const Offset(0, 600), 500);
3307+
await tester.pumpAndSettle();
3308+
expect(scrollController.offset, 0);
3309+
3310+
// Overscroll is OK for (vertical) trackpad gestures.
3311+
3312+
await tester.trackpadFling(find.byType(SingleChildScrollView), const Offset(0, -100), 500);
3313+
await tester.pumpAndSettle();
3314+
expect(scrollController.offset, greaterThan(100));
3315+
scrollController.jumpTo(600);
3316+
3317+
await tester.trackpadFling(find.byType(SingleChildScrollView), const Offset(0, 100), 500);
3318+
await tester.pumpAndSettle();
3319+
expect(scrollController.offset, lessThan(500));
3320+
scrollController.jumpTo(0);
3321+
3322+
// Horizontal scrolling: -X trackpad motion produces positive scroll offset change
3323+
3324+
await tester.pumpWidget(buildFrame(Axis.horizontal));
3325+
expect(scrollController.offset, 0);
3326+
expect(scrollController.position.maxScrollExtent, 800);
3327+
3328+
await tester.trackpadFling(find.byType(SingleChildScrollView), const Offset(-800, 0), 500);
3329+
await tester.pumpAndSettle();
3330+
expect(scrollController.offset, 800);
3331+
3332+
await tester.trackpadFling(find.byType(SingleChildScrollView), const Offset(800, 0), 500);
3333+
await tester.pumpAndSettle();
3334+
expect(scrollController.offset, 0);
3335+
3336+
// Overscroll is OK for (horizontal) trackpad gestures.
3337+
3338+
await tester.trackpadFling(find.byType(SingleChildScrollView), const Offset(-100, 0), 500);
3339+
await tester.pumpAndSettle();
3340+
expect(scrollController.offset, greaterThan(100));
3341+
scrollController.jumpTo(800);
3342+
3343+
await tester.trackpadFling(find.byType(SingleChildScrollView), const Offset(100, 0), 500);
3344+
await tester.pumpAndSettle();
3345+
expect(scrollController.offset, lessThan(700));
3346+
scrollController.jumpTo(0);
3347+
3348+
}, variant: const TargetPlatformVariant(<TargetPlatform>{
3349+
TargetPlatform.macOS,
3350+
TargetPlatform.linux,
3351+
TargetPlatform.windows,
3352+
TargetPlatform.fuchsia,
3353+
}));
3354+
3355+
testWidgets('Desktop trackpad, nested ListViews, no explicit scrollbars, horizontal drag succeeds', (WidgetTester tester) async {
3356+
// Regression test for https://github.com/flutter/flutter/issues/150236.
3357+
// This test is similar to "Desktop trackpad drag direction: -X,-Y...".
3358+
// It's really only verifying that trackpad gestures are being handled
3359+
// by the scrollable, not the scrollbar.
3360+
3361+
final Key outerListViewKey = UniqueKey();
3362+
final ScrollController scrollControllerY = ScrollController();
3363+
final ScrollController scrollControllerX = ScrollController();
3364+
addTearDown(scrollControllerY.dispose);
3365+
addTearDown(scrollControllerX.dispose);
3366+
3367+
await tester.pumpWidget(
3368+
Directionality(
3369+
textDirection: TextDirection.ltr,
3370+
child: MediaQuery(
3371+
data: const MediaQueryData(),
3372+
child: ListView(
3373+
key: outerListViewKey,
3374+
controller: scrollControllerY,
3375+
children: <Widget>[
3376+
const SizedBox(width: 200, height: 200),
3377+
SizedBox(
3378+
height: 200,
3379+
child: ListView( // vertically centered within the 600 high viewport
3380+
scrollDirection: Axis.horizontal,
3381+
controller: scrollControllerX,
3382+
children: List<Widget>.generate(5, (int index) {
3383+
return SizedBox(
3384+
width: 200,
3385+
child: Center(child: Text('item $index')),
3386+
);
3387+
}),
3388+
),
3389+
),
3390+
const SizedBox(width: 200, height: 200),
3391+
const SizedBox(width: 200, height: 200),
3392+
const SizedBox(width: 200, height: 200),
3393+
],
3394+
),
3395+
),
3396+
),
3397+
);
3398+
3399+
Finder outerListView() => find.byKey(outerListViewKey);
3400+
3401+
// 800x600 viewport content is 1000x1000
3402+
expect(tester.getSize(outerListView()), const Size(800, 600));
3403+
expect(scrollControllerY.offset, 0);
3404+
expect(scrollControllerY.position.maxScrollExtent, 400);
3405+
expect(scrollControllerX.offset, 0);
3406+
expect(scrollControllerX.position.maxScrollExtent, 200);
3407+
3408+
// Vertical scrolling: -Y trackpad motion produces positive scroll offset change
3409+
await tester.trackpadFling(outerListView(), const Offset(0, -600), 500);
3410+
await tester.pumpAndSettle();
3411+
expect(scrollControllerY.offset, 400);
3412+
await tester.trackpadFling(outerListView(), const Offset(0, 600), 500);
3413+
await tester.pumpAndSettle();
3414+
expect(scrollControllerY.offset, 0);
3415+
3416+
// Horizontal scrolling: -X trackpad motion produces positive scroll offset change
3417+
await tester.trackpadFling(outerListView(), const Offset(-800, 0), 500);
3418+
await tester.pumpAndSettle();
3419+
expect(scrollControllerX.offset, 200);
3420+
await tester.trackpadFling(outerListView(), const Offset(800, 0), 500);
3421+
await tester.pumpAndSettle();
3422+
expect(scrollControllerX.offset, 0);
3423+
3424+
}, variant: const TargetPlatformVariant(<TargetPlatform>{
3425+
TargetPlatform.macOS,
3426+
TargetPlatform.linux,
3427+
TargetPlatform.windows,
3428+
TargetPlatform.fuchsia,
3429+
}));
3430+
3431+
testWidgets('Desktop trackpad, nested ListViews, no explicit scrollbars, horizontal drag succeeds', (WidgetTester tester) async {
3432+
// Regression test for https://github.com/flutter/flutter/issues/150342
3433+
3434+
final ScrollController scrollController = ScrollController();
3435+
addTearDown(scrollController.dispose);
3436+
3437+
late Size childSize;
3438+
late StateSetter rebuildScrollViewChild;
3439+
3440+
Widget buildFrame(Axis scrollDirection) {
3441+
return Directionality(
3442+
textDirection: TextDirection.ltr,
3443+
child: MediaQuery(
3444+
data: const MediaQueryData(),
3445+
child: RawScrollbar(
3446+
controller: scrollController,
3447+
child: SingleChildScrollView(
3448+
controller: scrollController,
3449+
scrollDirection: scrollDirection,
3450+
child: StatefulBuilder(
3451+
builder: (BuildContext context, StateSetter setState) {
3452+
rebuildScrollViewChild = setState;
3453+
return SizedBox(width: childSize.width, height: childSize.height);
3454+
},
3455+
),
3456+
),
3457+
),
3458+
),
3459+
);
3460+
}
3461+
3462+
RawGestureDetector getScrollbarGestureDetector() {
3463+
return tester.widget<RawGestureDetector>(
3464+
find.descendant(of: find.byType(RawScrollbar), matching: find.byType(RawGestureDetector)).first
3465+
);
3466+
}
3467+
3468+
// Vertical scrollDirection
3469+
3470+
childSize = const Size(800, 600);
3471+
await tester.pumpWidget(buildFrame(Axis.vertical));
3472+
// Scrolling isn't possible, so there are no scrollbar gesture recognizers.
3473+
expect(getScrollbarGestureDetector().gestures.length, 0);
3474+
3475+
rebuildScrollViewChild(() { childSize = const Size(800, 800); });
3476+
await tester.pumpAndSettle();
3477+
// Scrolling is now possible, so there are scrollbar (thumb and track) gesture recognizers.
3478+
expect(getScrollbarGestureDetector().gestures.length, greaterThan(1));
3479+
3480+
// Horizontal scrollDirection
3481+
3482+
childSize = const Size(800, 600);
3483+
await tester.pumpWidget(buildFrame(Axis.horizontal));
3484+
await tester.pumpAndSettle();
3485+
// Scrolling isn't possible, so there are no scrollbar gesture recognizers.
3486+
expect(getScrollbarGestureDetector().gestures.length, 0);
3487+
3488+
rebuildScrollViewChild(() { childSize = const Size(1000, 600); });
3489+
await tester.pumpAndSettle();
3490+
// Scrolling is now possible, so there are scrollbar (thumb and track) gesture recognizers.
3491+
expect(getScrollbarGestureDetector().gestures.length, greaterThan(1));
32583492
});
32593493
}

0 commit comments

Comments
 (0)