Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 130 additions & 61 deletions packages/flutter/lib/src/widgets/scrollbar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1458,13 +1458,17 @@ class RawScrollbar extends StatefulWidget {
/// scrollbar track.
class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProviderStateMixin<T> {
Offset? _startDragScrollbarAxisOffset;
Offset? _lastDragUpdateOffset;
double? _startDragThumbOffset;
ScrollController? _currentController;
ScrollController? _cachedController;
Timer? _fadeoutTimer;
late AnimationController _fadeoutAnimationController;
late Animation<double> _fadeoutOpacityAnimation;
final GlobalKey _scrollbarPainterKey = GlobalKey();
bool _hoverIsActive = false;
bool _thumbDragging = false;

ScrollController? get _effectiveScrollController => widget.controller ?? PrimaryScrollController.maybeOf(context);

/// Used to paint the scrollbar.
///
Expand Down Expand Up @@ -1550,12 +1554,11 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
}

void _validateInteractions(AnimationStatus status) {
final ScrollController? scrollController = widget.controller ?? PrimaryScrollController.maybeOf(context);
if (status == AnimationStatus.dismissed) {
assert(_fadeoutOpacityAnimation.value == 0.0);
// We do not check for a valid scroll position if the scrollbar is not
// visible, because it cannot be interacted with.
} else if (scrollController != null && enableGestures) {
} else if (_effectiveScrollController != null && enableGestures) {
// Interactive scrollbars need to be properly configured. If it is visible
// for interaction, ensure we are set up properly.
assert(_debugCheckHasValidScrollPosition());
Expand All @@ -1566,7 +1569,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
if (!mounted) {
return true;
}
final ScrollController? scrollController = widget.controller ?? PrimaryScrollController.maybeOf(context);
final ScrollController? scrollController = _effectiveScrollController;
final bool tryPrimary = widget.controller == null;
final String controllerForError = tryPrimary
? 'PrimaryScrollController'
Expand Down Expand Up @@ -1698,11 +1701,11 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
}

void _updateScrollPosition(Offset updatedOffset) {
assert(_currentController != null);
assert(_cachedController != null);
assert(_startDragScrollbarAxisOffset != null);
assert(_startDragThumbOffset != null);

final ScrollPosition position = _currentController!.position;
final ScrollPosition position = _cachedController!.position;
late double primaryDelta;
switch (position.axisDirection) {
case AxisDirection.up:
Expand Down Expand Up @@ -1761,9 +1764,9 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
/// current scroll controller does not have any attached positions.
@protected
Axis? getScrollbarDirection() {
assert(_currentController != null);
if (_currentController!.hasClients) {
return _currentController!.position.axis;
assert(_cachedController != null);
if (_cachedController!.hasClients) {
return _cachedController!.position.axis;
}
return null;
}
Expand All @@ -1788,7 +1791,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
@mustCallSuper
void handleThumbPressStart(Offset localPosition) {
assert(_debugCheckHasValidScrollPosition());
_currentController = widget.controller ?? PrimaryScrollController.maybeOf(context);
_cachedController = _effectiveScrollController;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this, very nice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Piinks Hi, this is ready for review now and is not urgent, just put it in your queue.

final Axis? direction = getScrollbarDirection();
if (direction == null) {
return;
Expand All @@ -1797,6 +1800,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
_fadeoutAnimationController.forward();
_startDragScrollbarAxisOffset = localPosition;
_startDragThumbOffset = scrollbarPainter.getThumbScrollOffset();
_thumbDragging = true;
}

/// Handler called when a currently active long press gesture moves.
Expand All @@ -1806,7 +1810,11 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
@mustCallSuper
void handleThumbPressUpdate(Offset localPosition) {
assert(_debugCheckHasValidScrollPosition());
final ScrollPosition position = _currentController!.position;
if (_lastDragUpdateOffset == localPosition) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will frequently be called when press and hold. There are benefits to short-circuiting it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call!

return;
}
_lastDragUpdateOffset = localPosition;
final ScrollPosition position = _cachedController!.position;
if (!position.physics.shouldAcceptUserOffset(position)) {
return;
}
Expand All @@ -1822,45 +1830,47 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
@mustCallSuper
void handleThumbPressEnd(Offset localPosition, Velocity velocity) {
assert(_debugCheckHasValidScrollPosition());
_thumbDragging = false;
final Axis? direction = getScrollbarDirection();
if (direction == null) {
return;
}
_maybeStartFadeoutTimer();
_startDragScrollbarAxisOffset = null;
_lastDragUpdateOffset = null;
_startDragThumbOffset = null;
_currentController = null;
_cachedController = null;
}

void _handleTrackTapDown(TapDownDetails details) {
// The Scrollbar should page towards the position of the tap on the track.
assert(_debugCheckHasValidScrollPosition());
_currentController = widget.controller ?? PrimaryScrollController.maybeOf(context);
_cachedController = _effectiveScrollController;

final ScrollPosition position = _currentController!.position;
final ScrollPosition position = _cachedController!.position;
if (!position.physics.shouldAcceptUserOffset(position)) {
return;
}

double scrollIncrement;
// Is an increment calculator available?
final ScrollIncrementCalculator? calculator = Scrollable.maybeOf(
_currentController!.position.context.notificationContext!,
_cachedController!.position.context.notificationContext!,
)?.widget.incrementCalculator;
if (calculator != null) {
scrollIncrement = calculator(
ScrollIncrementDetails(
type: ScrollIncrementType.page,
metrics: _currentController!.position,
metrics: _cachedController!.position,
),
);
} else {
// Default page increment
scrollIncrement = 0.8 * _currentController!.position.viewportDimension;
scrollIncrement = 0.8 * _cachedController!.position.viewportDimension;
}

// Adjust scrollIncrement for direction
switch (_currentController!.position.axisDirection) {
switch (_cachedController!.position.axisDirection) {
case AxisDirection.up:
if (details.localPosition.dy > scrollbarPainter._thumbOffset) {
scrollIncrement = -scrollIncrement;
Expand All @@ -1883,17 +1893,16 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
break;
}

_currentController!.position.moveTo(
_currentController!.position.pixels + scrollIncrement,
_cachedController!.position.moveTo(
_cachedController!.position.pixels + scrollIncrement,
duration: const Duration(milliseconds: 100),
curve: Curves.easeInOut,
);
}

// ScrollController takes precedence over ScrollNotification
bool _shouldUpdatePainter(Axis notificationAxis) {
final ScrollController? scrollController = widget.controller ??
PrimaryScrollController.maybeOf(context);
final ScrollController? scrollController = _effectiveScrollController;
// Only update the painter of this scrollbar if the notification
// metrics do not conflict with the information we have from the scroll
// controller.
Expand Down Expand Up @@ -1979,8 +1988,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv

Map<Type, GestureRecognizerFactory> get _gestures {
final Map<Type, GestureRecognizerFactory> gestures = <Type, GestureRecognizerFactory>{};
final ScrollController? controller = widget.controller ?? PrimaryScrollController.maybeOf(context);
if (controller == null || !enableGestures) {
if (_effectiveScrollController == null || !enableGestures) {
return gestures;
}

Expand Down Expand Up @@ -2086,6 +2094,64 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
_maybeStartFadeoutTimer();
}

// Returns the delta that should result from applying [event] with axis and
// direction taken into account.
double _pointerSignalEventDelta(PointerScrollEvent event) {
assert(_cachedController != null);
double delta = _cachedController!.position.axis == Axis.horizontal
? event.scrollDelta.dx
: event.scrollDelta.dy;

if (axisDirectionIsReversed(_cachedController!.position.axisDirection)) {
delta *= -1;
}
return delta;
}

// Returns the offset that should result from applying [event] to the current
// position, taking min/max scroll extent into account.
double _targetScrollOffsetForPointerScroll(double delta) {
assert(_cachedController != null);
return math.min(
math.max(_cachedController!.position.pixels + delta, _cachedController!.position.minScrollExtent),
_cachedController!.position.maxScrollExtent,
);
}

void _handlePointerScroll(PointerEvent event) {
assert(event is PointerScrollEvent);
_cachedController = _effectiveScrollController;
final double delta = _pointerSignalEventDelta(event as PointerScrollEvent);
final double targetScrollOffset = _targetScrollOffsetForPointerScroll(delta);
if (delta != 0.0 && targetScrollOffset != _cachedController!.position.pixels) {
_cachedController!.position.pointerScroll(delta);
}
}

void _receivedPointerSignal(PointerSignalEvent event) {
_cachedController = _effectiveScrollController;
// Only try to scroll if the bar absorb the hit test.
if ((scrollbarPainter.hitTest(event.localPosition) ?? false) &&
_cachedController != null &&
_cachedController!.hasClients &&
(!_thumbDragging || kIsWeb)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referring to the behavior of the native platforms, we only allow pointer scroll during thumb dragging on the web.

final ScrollPosition position = _cachedController!.position;
if (event is PointerScrollEvent && position != null) {
if (!position.physics.shouldAcceptUserOffset(position)) {
return;
}
final double delta = _pointerSignalEventDelta(event);
final double targetScrollOffset = _targetScrollOffsetForPointerScroll(delta);
if (delta != 0.0 && targetScrollOffset != position.pixels) {
GestureBinding.instance.pointerSignalResolver.register(event, _handlePointerScroll);
}
} else if (event is PointerScrollInertiaCancelEvent) {
position.jumpTo(position.pixels);
// Don't use the pointer signal resolver, all hit-tested scrollables should stop.
}
}
}

@override
void dispose() {
_fadeoutAnimationController.dispose();
Expand All @@ -2103,43 +2169,46 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
child: NotificationListener<ScrollNotification>(
onNotification: _handleScrollNotification,
child: RepaintBoundary(
child: RawGestureDetector(
gestures: _gestures,
child: MouseRegion(
onExit: (PointerExitEvent event) {
switch(event.kind) {
case PointerDeviceKind.mouse:
case PointerDeviceKind.trackpad:
if (enableGestures) {
handleHoverExit(event);
}
break;
case PointerDeviceKind.stylus:
case PointerDeviceKind.invertedStylus:
case PointerDeviceKind.unknown:
case PointerDeviceKind.touch:
break;
}
},
onHover: (PointerHoverEvent event) {
switch(event.kind) {
case PointerDeviceKind.mouse:
case PointerDeviceKind.trackpad:
if (enableGestures) {
handleHover(event);
}
break;
case PointerDeviceKind.stylus:
case PointerDeviceKind.invertedStylus:
case PointerDeviceKind.unknown:
case PointerDeviceKind.touch:
break;
}
},
child: CustomPaint(
key: _scrollbarPainterKey,
foregroundPainter: scrollbarPainter,
child: RepaintBoundary(child: widget.child),
child: Listener(
Copy link
Contributor

@Piinks Piinks Nov 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this took me a minute because I thought, why can't we approach this through hit testing? Of course you already had an answer for me: #99328 😜

I don't love having so many handlers and listeners here, like MouseRegion (for hovering), Listener (for pointer scrolling), NotificationListeners (for scrolling and scroll metrics), but they do seem necessary to be able to match all of the various behaviors we see on different platforms.

onPointerSignal: _receivedPointerSignal,
child: RawGestureDetector(
gestures: _gestures,
child: MouseRegion(
onExit: (PointerExitEvent event) {
switch(event.kind) {
case PointerDeviceKind.mouse:
case PointerDeviceKind.trackpad:
if (enableGestures) {
handleHoverExit(event);
}
break;
case PointerDeviceKind.stylus:
case PointerDeviceKind.invertedStylus:
case PointerDeviceKind.unknown:
case PointerDeviceKind.touch:
break;
}
},
onHover: (PointerHoverEvent event) {
switch(event.kind) {
case PointerDeviceKind.mouse:
case PointerDeviceKind.trackpad:
if (enableGestures) {
handleHover(event);
}
break;
case PointerDeviceKind.stylus:
case PointerDeviceKind.invertedStylus:
case PointerDeviceKind.unknown:
case PointerDeviceKind.touch:
break;
}
},
child: CustomPaint(
key: _scrollbarPainterKey,
foregroundPainter: scrollbarPainter,
child: RepaintBoundary(child: widget.child),
),
),
),
),
Expand Down
35 changes: 21 additions & 14 deletions packages/flutter/test/cupertino/scrollbar_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1200,25 +1200,32 @@ void main() {
pointer.hover(const Offset(793.0, 15.0));
await tester.sendEventToBinding(pointer.scroll(const Offset(0.0, 20.0)));
await tester.pumpAndSettle();
// Scrolling while holding the drag on the scrollbar and still hovered over
// the scrollbar should not have changed the scroll offset.
expect(pointer.location, const Offset(793.0, 15.0));
expect(scrollController.offset, previousOffset);
expect(
find.byType(CupertinoScrollbar),
paints..rrect(
rrect: RRect.fromRectAndRadius(
const Rect.fromLTRB(789.0, 13.0, 797.0, 102.1),
const Radius.circular(4.0),

if (!kIsWeb) {
// Scrolling while holding the drag on the scrollbar and still hovered over
// the scrollbar should not have changed the scroll offset.
expect(pointer.location, const Offset(793.0, 15.0));
expect(scrollController.offset, previousOffset);
expect(
find.byType(CupertinoScrollbar),
paints..rrect(
rrect: RRect.fromRectAndRadius(
const Rect.fromLTRB(789.0, 13.0, 797.0, 102.1),
const Radius.circular(4.0),
),
color: _kScrollbarColor.color,
),
color: _kScrollbarColor.color,
),
);
);
} else {
expect(pointer.location, const Offset(793.0, 15.0));
expect(scrollController.offset, previousOffset + 20.0);
}


// Drag is still being held, move pointer to be hovering over another area
// of the scrollable (not over the scrollbar) and execute another pointer scroll
pointer.hover(tester.getCenter(find.byType(SingleChildScrollView)));
await tester.sendEventToBinding(pointer.scroll(const Offset(0.0, -70.0)));
await tester.sendEventToBinding(pointer.scroll(const Offset(0.0, -90.0)));
await tester.pumpAndSettle();
// Scrolling while holding the drag on the scrollbar changed the offset
expect(pointer.location, const Offset(400.0, 300.0));
Expand Down
Loading