fix mouse wheel scroll miscontrol of ScrollPosition. (#66039)

This commit is contained in:
YeungKC 2020-11-03 05:13:03 +08:00 committed by GitHub
parent 8291f4810f
commit 60a8b333b0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 514 additions and 13 deletions

View file

@ -1072,6 +1072,63 @@ class _NestedScrollCoordinator implements ScrollActivityDelegate, ScrollHoldCont
goBallistic(0.0);
}
void pointerScroll(double delta) {
assert(delta != 0.0);
goIdle();
updateUserScrollDirection(
delta < 0.0 ? ScrollDirection.forward : ScrollDirection.reverse
);
if (_innerPositions.isEmpty) {
// Does not enter overscroll.
_outerPosition!.applyClampedPointerSignalUpdate(delta);
} else if (delta > 0.0) {
// Dragging "up" - delta is positive
// Prioritize getting rid of any inner overscroll, and then the outer
// view, so that the app bar will scroll out of the way asap.
double outerDelta = delta;
for (final _NestedScrollPosition position in _innerPositions) {
if (position.pixels < 0.0) { // This inner position is in overscroll.
final double potentialOuterDelta = position.applyClampedPointerSignalUpdate(delta);
// In case there are multiple positions in varying states of
// overscroll, the first to 'reach' the outer view above takes
// precedence.
outerDelta = math.max(outerDelta, potentialOuterDelta);
}
}
if (outerDelta != 0.0) {
final double innerDelta = _outerPosition!.applyClampedPointerSignalUpdate(
outerDelta
);
if (innerDelta != 0.0) {
for (final _NestedScrollPosition position in _innerPositions)
position.applyClampedPointerSignalUpdate(innerDelta);
}
}
} else {
// Dragging "down" - delta is negative
double innerDelta = delta;
// Apply delta to the outer header first if it is configured to float.
if (_floatHeaderSlivers)
innerDelta = _outerPosition!.applyClampedPointerSignalUpdate(delta);
if (innerDelta != 0.0) {
// Apply the innerDelta, if we have not floated in the outer scrollable,
// any leftover delta after this will be passed on to the outer
// scrollable by the outerDelta.
double outerDelta = 0.0; // it will go negative if it changes
for (final _NestedScrollPosition position in _innerPositions) {
final double overscroll = position.applyClampedPointerSignalUpdate(innerDelta);
outerDelta = math.min(outerDelta, overscroll);
}
if (outerDelta != 0.0)
_outerPosition!.applyClampedPointerSignalUpdate(outerDelta);
}
}
goBallistic(0.0);
}
@override
double setPixels(double newPixels) {
assert(false);
@ -1386,6 +1443,32 @@ class _NestedScrollPosition extends ScrollPosition implements ScrollActivityDele
return 0.0;
}
// Returns the amount of delta that was not used.
//
// Negative delta represents a forward ScrollDirection, while the positive
// would be a reverse ScrollDirection.
//
// The method doesn't take into account the effects of [ScrollPhysics].
double applyClampedPointerSignalUpdate(double delta) {
assert(delta != 0.0);
final double min = delta > 0.0
? -double.infinity
: math.min(minScrollExtent, pixels);
// The logic for max is equivalent but on the other side.
final double max = delta < 0.0
? double.infinity
: math.max(maxScrollExtent, pixels);
final double newPixels = (pixels + delta).clamp(min, max);
final double clampedDelta = newPixels - pixels;
if (clampedDelta == 0.0)
return delta;
forcePixels(newPixels);
didUpdateScrollPositionBy(clampedDelta);
return delta - clampedDelta;
}
@override
ScrollDirection get userScrollDirection => coordinator.userScrollDirection;
@ -1475,6 +1558,12 @@ class _NestedScrollPosition extends ScrollPosition implements ScrollActivityDele
return coordinator.jumpTo(coordinator.unnestOffset(value, this));
}
@override
void pointerScroll(double delta) {
return coordinator.pointerScroll(delta);
}
@override
void jumpToWithoutSettling(double value) {
assert(false);

View file

@ -761,6 +761,22 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
@override
void jumpTo(double value);
/// Changes the scrolling position based on a pointer signal from current
/// value to delta without animation and without checking if new value is in
/// range, taking min/max scroll extent into account.
///
/// Any active animation is canceled. If the user is currently scrolling, that
/// action is canceled.
///
/// This method dispatches the start/update/end sequence of scrolling
/// notifications.
///
/// This method is very similar to [jumpTo], but [pointerScroll] will
/// update the [ScrollDirection].
///
// TODO(YeungKC): Support trackpad scroll, https://github.com/flutter/flutter/issues/23604.
void pointerScroll(double delta);
/// Calls [jumpTo] if duration is null or [Duration.zero], otherwise
/// [animateTo] is called.
///

View file

@ -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:math' as math;
import 'package:flutter/gestures.dart';
import 'package:flutter/physics.dart';
import 'package:flutter/rendering.dart';
@ -202,6 +204,27 @@ class ScrollPositionWithSingleContext extends ScrollPosition implements ScrollAc
goBallistic(0.0);
}
@override
void pointerScroll(double delta) {
assert(delta != 0.0);
final double targetPixels =
math.min(math.max(pixels + delta, minScrollExtent), maxScrollExtent);
if (targetPixels != pixels) {
goIdle();
updateUserScrollDirection(
-delta > 0.0 ? ScrollDirection.forward : ScrollDirection.reverse
);
final double oldPixels = pixels;
forcePixels(targetPixels);
didStartScroll();
didUpdateScrollPositionBy(pixels - oldPixels);
didEndScroll();
goBallistic(0.0);
}
}
@Deprecated('This will lead to bugs.') // ignore: flutter_deprecation_syntax, https://github.com/flutter/flutter/issues/44609
@override
void jumpToWithoutSettling(double value) {

View file

@ -3,7 +3,6 @@
// found in the LICENSE file.
import 'dart:async';
import 'dart:math' as math;
import 'dart:ui';
import 'package:flutter/gestures.dart';
@ -624,9 +623,9 @@ class ScrollableState extends State<Scrollable> with TickerProviderStateMixin, R
// SCROLL WHEEL
// Returns the offset that should result from applying [event] to the current
// position, taking min/max scroll extent into account.
double _targetScrollOffsetForPointerScroll(PointerScrollEvent event) {
// Returns the delta that should result from applying [event] with axis and
// direction taken into account.
double _targetScrollDeltaForPointerScroll(PointerScrollEvent event) {
double delta = widget.axis == Axis.horizontal
? event.scrollDelta.dx
: event.scrollDelta.dy;
@ -635,15 +634,14 @@ class ScrollableState extends State<Scrollable> with TickerProviderStateMixin, R
delta *= -1;
}
return math.min(math.max(position.pixels + delta, position.minScrollExtent),
position.maxScrollExtent);
return delta;
}
void _receivedPointerSignal(PointerSignalEvent event) {
if (event is PointerScrollEvent && _position != null) {
final double targetScrollOffset = _targetScrollOffsetForPointerScroll(event);
final double targetScrollOffset = _targetScrollDeltaForPointerScroll(event);
// Only express interest in the event if it would actually result in a scroll.
if (targetScrollOffset != position.pixels) {
if (targetScrollOffset != 0) {
GestureBinding.instance!.pointerSignalResolver.register(event, _handlePointerScroll);
}
}
@ -654,9 +652,9 @@ class ScrollableState extends State<Scrollable> with TickerProviderStateMixin, R
if (_physics != null && !_physics!.shouldAcceptUserOffset(position)) {
return;
}
final double targetScrollOffset = _targetScrollOffsetForPointerScroll(event as PointerScrollEvent);
if (targetScrollOffset != position.pixels) {
position.jumpTo(targetScrollOffset);
final double targetScrollOffset = _targetScrollDeltaForPointerScroll(event as PointerScrollEvent);
if (targetScrollOffset != 0) {
position.pointerScroll(targetScrollOffset);
}
}

View file

@ -2,11 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'dart:ui' as ui;
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/gestures.dart' show DragStartBehavior;
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart';
import '../rendering/rendering_tester.dart';
@ -1430,6 +1432,123 @@ void main() {
verifyGeometry(key: appBarKey, paintExtent: 200.0, visible: true);
});
testWidgets('float with pointer signal', (WidgetTester tester) async {
final GlobalKey appBarKey = GlobalKey();
await tester.pumpWidget(buildFloatTest(
floating: true,
nestedFloat: true,
appBarKey: appBarKey,
));
final Offset scrollEventLocation = tester.getCenter(find.byType(NestedScrollView));
final TestPointer testPointer = TestPointer(1, ui.PointerDeviceKind.mouse);
// Create a hover event so that |testPointer| has a location when generating the scroll.
testPointer.hover(scrollEventLocation);
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsOneWidget);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
56.0,
);
verifyGeometry(key: appBarKey, paintExtent: 56.0, visible: true);
// Scroll away the outer scroll view and some of the inner scroll view.
// We will not scroll back the same amount to indicate that we are
// floating in before reaching the top of the inner scrollable.
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, 300.0)));
await tester.pump();
expect(find.text('Test Title'), findsNothing);
expect(find.text('Item 1'), findsNothing);
expect(find.text('Item 5'), findsOneWidget);
verifyGeometry(key: appBarKey, paintExtent: 0.0, visible: false);
// The outer scrollable should float back in, inner should not change
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, -50.0)));
await tester.pump();
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsNothing);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
56.0,
);
verifyGeometry(key: appBarKey, paintExtent: 50.0, visible: true);
// Float the rest of the way in.
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, -150.0)));
await tester.pump();
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsNothing);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
56.0,
);
verifyGeometry(key: appBarKey, paintExtent: 56.0, visible: true);
});
testWidgets('float expanded with pointer signal', (WidgetTester tester) async {
final GlobalKey appBarKey = GlobalKey();
await tester.pumpWidget(buildFloatTest(
floating: true,
nestedFloat: true,
expanded: true,
appBarKey: appBarKey,
));
final Offset scrollEventLocation = tester.getCenter(find.byType(NestedScrollView));
final TestPointer testPointer = TestPointer(1, ui.PointerDeviceKind.mouse);
// Create a hover event so that |testPointer| has a location when generating the scroll.
testPointer.hover(scrollEventLocation);
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsOneWidget);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
200.0,
);
verifyGeometry(key: appBarKey, paintExtent: 200.0, visible: true);
// Scroll away the outer scroll view and some of the inner scroll view.
// We will not scroll back the same amount to indicate that we are
// floating in before reaching the top of the inner scrollable.
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, 300.0)));
await tester.pump();
expect(find.text('Test Title'), findsNothing);
expect(find.text('Item 1'), findsNothing);
expect(find.text('Item 5'), findsOneWidget);
verifyGeometry(key: appBarKey, paintExtent: 0.0, visible: false);
// The outer scrollable should float back in, inner should not change
// On initial float in, the app bar is collapsed.
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, -50.0)));
await tester.pump();
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsNothing);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
56.0,
);
verifyGeometry(key: appBarKey, paintExtent: 50.0, visible: true);
// The inner scrollable should receive leftover delta after the outer has
// been scrolled back in fully.
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, -200.0)));
await tester.pump();
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsOneWidget);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
200.0,
);
verifyGeometry(key: appBarKey, paintExtent: 200.0, visible: true);
});
testWidgets('only snap', (WidgetTester tester) async {
final GlobalKey appBarKey = GlobalKey();
final GlobalKey<NestedScrollViewState> nestedKey = GlobalKey();
@ -1814,6 +1933,130 @@ void main() {
);
verifyGeometry(key: appBarKey, paintExtent: 200.0, visible: true);
});
testWidgets('float pinned with pointer signal', (WidgetTester tester) async {
// This configuration should have the same behavior of a pinned app bar.
// No floating should happen, and the app bar should persist.
final GlobalKey appBarKey = GlobalKey();
await tester.pumpWidget(buildFloatTest(
floating: true,
pinned: true,
nestedFloat: true,
appBarKey: appBarKey,
));
final Offset scrollEventLocation = tester.getCenter(find.byType(NestedScrollView));
final TestPointer testPointer = TestPointer(1, ui.PointerDeviceKind.mouse);
// Create a hover event so that |testPointer| has a location when generating the scroll.
testPointer.hover(scrollEventLocation);
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsOneWidget);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
56.0,
);
verifyGeometry(key: appBarKey, paintExtent: 56.0, visible: true);
// Scroll away the outer scroll view and some of the inner scroll view.
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, 300.0)));
await tester.pump();
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsNothing);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
56.0,
);
verifyGeometry(key: appBarKey, paintExtent: 56.0, visible: true);
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, -50.0)));
await tester.pump();
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsNothing);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
56.0,
);
verifyGeometry(key: appBarKey, paintExtent: 56.0, visible: true);
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, -150.0)));
await tester.pump();
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsOneWidget);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
56.0,
);
verifyGeometry(key: appBarKey, paintExtent: 56.0, visible: true);
});
testWidgets('float pinned expanded with pointer signal', (WidgetTester tester) async {
// Only the expanded portion (flexible space) of the app bar should float
// in and out.
final GlobalKey appBarKey = GlobalKey();
await tester.pumpWidget(buildFloatTest(
floating: true,
pinned: true,
expanded: true,
nestedFloat: true,
appBarKey: appBarKey,
));
final Offset scrollEventLocation = tester.getCenter(find.byType(NestedScrollView));
final TestPointer testPointer = TestPointer(1, ui.PointerDeviceKind.mouse);
// Create a hover event so that |testPointer| has a location when generating the scroll.
testPointer.hover(scrollEventLocation);
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsOneWidget);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
200.0,
);
verifyGeometry(key: appBarKey, paintExtent: 200.0, visible: true);
// Scroll away the outer scroll view and some of the inner scroll view.
// The expanded portion of the app bar should collapse.
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, 300.0)));
await tester.pump();
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsNothing);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
56.0,
);
verifyGeometry(key: appBarKey, paintExtent: 56.0, visible: true);
// Scroll back some, the app bar should expand.
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, -50.0)));
await tester.pump();
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsNothing);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
106.0, // 56.0 + 50.0
);
verifyGeometry(key: appBarKey, paintExtent: 106.0, visible: true);
// Finish scrolling the rest of the way in.
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, -150.0)));
await tester.pump();
expect(find.text('Test Title'), findsOneWidget);
expect(find.text('Item 1'), findsOneWidget);
expect(find.text('Item 5'), findsOneWidget);
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
200.0,
);
verifyGeometry(key: appBarKey, paintExtent: 200.0, visible: true);
});
});
group('Correctly handles 0 velocity inner ballistic scroll activity:', () {
@ -1961,6 +2204,105 @@ void main() {
);
expect(nestedScrollView.currentState!.innerController.position.pixels, 295.0);
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.macOS }));
testWidgets('Scroll pointer signal should not cause overscroll.',
(WidgetTester tester) async {
final ScrollController controller = ScrollController();
await tester.pumpWidget(buildTest(controller: controller));
final Offset scrollEventLocation = tester.getCenter(find.byType(NestedScrollView));
final TestPointer testPointer = TestPointer(1, ui.PointerDeviceKind.mouse);
// Create a hover event so that |testPointer| has a location when generating the scroll.
testPointer.hover(scrollEventLocation);
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, 20.0)));
expect(controller.offset, 20);
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, -40.0)));
expect(controller.offset, 0);
await tester.tap(find.text('DD'));
await tester.pumpAndSettle();
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, 1000000.0)));
expect(find.text('ddd1'), findsOneWidget);
});
testWidgets('NestedScrollView basic scroll with pointer signal', (WidgetTester tester) async{
await tester.pumpWidget(buildTest());
expect(find.text('aaa2'), findsOneWidget);
expect(find.text('aaa3'), findsNothing);
expect(find.text('bbb1'), findsNothing);
await tester.pump(const Duration(milliseconds: 250));
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
200.0,
);
// Regression test for https://github.com/flutter/flutter/issues/55362
final TestPointer testPointer = TestPointer(1, ui.PointerDeviceKind.mouse);
// The offset is the responsibility of innerPosition.
testPointer.hover(const Offset(0, 201));
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, 20.0)));
await tester.pump(const Duration(milliseconds: 250));
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
180.0,
);
testPointer.hover(const Offset(0, 179));
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, 20.0)));
await tester.pump(const Duration(milliseconds: 250));
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
160.0,
);
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, 20.0)));
await tester.pump(const Duration(milliseconds: 250));
expect(
tester.renderObject<RenderBox>(find.byType(AppBar)).size.height,
140.0,
);
});
// Related to https://github.com/flutter/flutter/issues/64266
testWidgets(
'Holding scroll and Scroll pointer signal will update ScrollDirection.forward / ScrollDirection.reverse',
(WidgetTester tester) async {
ScrollDirection? lastUserScrollingDirection;
final ScrollController controller = ScrollController();
await tester.pumpWidget(buildTest(controller: controller));
controller.addListener(() {
if (controller.position.userScrollDirection != ScrollDirection.idle)
lastUserScrollingDirection = controller.position.userScrollDirection;
});
await tester.drag(find.byType(NestedScrollView), const Offset(0.0, -20.0),
touchSlopY: 0.0);
expect(lastUserScrollingDirection, ScrollDirection.reverse);
final Offset scrollEventLocation = tester.getCenter(find.byType(NestedScrollView));
final TestPointer testPointer = TestPointer(1, ui.PointerDeviceKind.mouse);
// Create a hover event so that |testPointer| has a location when generating the scroll.
testPointer.hover(scrollEventLocation);
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, 20.0)));
expect(lastUserScrollingDirection, ScrollDirection.reverse);
await tester.drag(find.byType(NestedScrollView), const Offset(0.0, 20.0),
touchSlopY: 0.0);
expect(lastUserScrollingDirection, ScrollDirection.forward);
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, -20.0)));
expect(lastUserScrollingDirection, ScrollDirection.forward);
});
}
class TestHeader extends SliverPersistentHeaderDelegate {

View file

@ -323,6 +323,39 @@ void main() {
expect(getScrollOffset(tester), 0.0);
});
testWidgets('Holding scroll and Scroll pointer signal will update ScrollDirection.forward / ScrollDirection.reverse', (WidgetTester tester) async {
ScrollDirection? lastUserScrollingDirection;
final ScrollController controller = ScrollController();
await pumpTest(tester, TargetPlatform.fuchsia, controller: controller);
controller.addListener(() {
if(controller.position.userScrollDirection != ScrollDirection.idle)
lastUserScrollingDirection = controller.position.userScrollDirection;
});
await tester.drag(find.byType(Viewport), const Offset(0.0, -20.0), touchSlopY: 0.0);
expect(lastUserScrollingDirection, ScrollDirection.reverse);
final Offset scrollEventLocation = tester.getCenter(find.byType(Viewport));
final TestPointer testPointer = TestPointer(1, ui.PointerDeviceKind.mouse);
// Create a hover event so that |testPointer| has a location when generating the scroll.
testPointer.hover(scrollEventLocation);
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, 20.0)));
expect(lastUserScrollingDirection, ScrollDirection.reverse);
await tester.drag(find.byType(Viewport), const Offset(0.0, 20.0), touchSlopY: 0.0);
expect(lastUserScrollingDirection, ScrollDirection.forward);
await tester.sendEventToBinding(testPointer.scroll(const Offset(0.0, -20.0)));
expect(lastUserScrollingDirection, ScrollDirection.forward);
});
testWidgets('Scrolls in correct direction when scroll axis is reversed', (WidgetTester tester) async {
await pumpTest(tester, TargetPlatform.fuchsia, reverse: true);