Skip to content

Commit

Permalink
Fix scrollbar hit testing based on pointer device kind (#77755)
Browse files Browse the repository at this point in the history
  • Loading branch information
Piinks committed Mar 10, 2021
1 parent 4d38ca6 commit e02621b
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 28 deletions.
2 changes: 1 addition & 1 deletion packages/flutter/lib/src/material/scrollbar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ class _MaterialScrollbarState extends RawScrollbarState<_MaterialScrollbar> {
void handleHover(PointerHoverEvent event) {
super.handleHover(event);
// Check if the position of the pointer falls over the painted scrollbar
if (isPointerOverScrollbar(event.position)) {
if (isPointerOverScrollbar(event.position, event.kind)) {
// Pointer is hovering over the scrollbar
setState(() { _hoverIsActive = true; });
_hoverAnimationController.forward();
Expand Down
70 changes: 43 additions & 27 deletions packages/flutter/lib/src/widgets/scrollbar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -478,40 +478,56 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
return _paintScrollbar(canvas, size, thumbExtent, _lastAxisDirection!);
}

/// Same as hitTest, but includes some padding to make sure that the region
/// Same as hitTest, but includes some padding when the [PointerEvent] is
/// caused by [PointerDeviceKind.touch] to make sure that the region
/// isn't too small to be interacted with by the user.
bool hitTestInteractive(Offset position) {
bool hitTestInteractive(Offset position, PointerDeviceKind kind) {
if (_thumbRect == null) {
return false;
}
// The scrollbar is not able to be hit when transparent.
if (fadeoutOpacityAnimation.value == 0.0) {
return false;
}
final Rect interactiveScrollbarRect = _trackRect == null
? _thumbRect!.expandToInclude(
Rect.fromCircle(center: _thumbRect!.center, radius: _kMinInteractiveSize / 2),
)
: _trackRect!.expandToInclude(

final Rect interactiveRect = _trackRect ?? _thumbRect!;
switch (kind) {
case PointerDeviceKind.touch:
final Rect touchScrollbarRect = interactiveRect.expandToInclude(
Rect.fromCircle(center: _thumbRect!.center, radius: _kMinInteractiveSize / 2),
);
return interactiveScrollbarRect.contains(position);
return touchScrollbarRect.contains(position);
case PointerDeviceKind.mouse:
case PointerDeviceKind.stylus:
case PointerDeviceKind.invertedStylus:
case PointerDeviceKind.unknown:
return interactiveRect.contains(position);
}
}

/// Same as hitTestInteractive, but excludes the track portion of the scrollbar.
/// Used to evaluate interactions with only the scrollbar thumb.
bool hitTestOnlyThumbInteractive(Offset position) {
bool hitTestOnlyThumbInteractive(Offset position, PointerDeviceKind kind) {
if (_thumbRect == null) {
return false;
}
// The thumb is not able to be hit when transparent.
if (fadeoutOpacityAnimation.value == 0.0) {
return false;
}
final Rect interactiveThumbRect = _thumbRect!.expandToInclude(
Rect.fromCircle(center: _thumbRect!.center, radius: _kMinInteractiveSize / 2),
);
return interactiveThumbRect.contains(position);

switch (kind) {
case PointerDeviceKind.touch:
final Rect touchThumbRect = _thumbRect!.expandToInclude(
Rect.fromCircle(center: _thumbRect!.center, radius: _kMinInteractiveSize / 2),
);
return touchThumbRect.contains(position);
case PointerDeviceKind.mouse:
case PointerDeviceKind.stylus:
case PointerDeviceKind.invertedStylus:
case PointerDeviceKind.unknown:
return _thumbRect!.contains(position);
}
}

// Scrollbars are interactive.
Expand Down Expand Up @@ -1153,33 +1169,33 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
///
/// Excludes the [RawScrollbar] thumb.
@protected
bool isPointerOverTrack(Offset position) {
bool isPointerOverTrack(Offset position, PointerDeviceKind kind) {
if (_scrollbarPainterKey.currentContext == null) {
return false;
}
final Offset localOffset = _getLocalOffset(_scrollbarPainterKey, position);
return scrollbarPainter.hitTestInteractive(localOffset)
&& !scrollbarPainter.hitTestOnlyThumbInteractive(localOffset);
return scrollbarPainter.hitTestInteractive(localOffset, kind)
&& !scrollbarPainter.hitTestOnlyThumbInteractive(localOffset, kind);
}
/// Returns true if the provided [Offset] is located over the thumb of the
/// [RawScrollbar].
@protected
bool isPointerOverThumb(Offset position) {
bool isPointerOverThumb(Offset position, PointerDeviceKind kind) {
if (_scrollbarPainterKey.currentContext == null) {
return false;
}
final Offset localOffset = _getLocalOffset(_scrollbarPainterKey, position);
return scrollbarPainter.hitTestOnlyThumbInteractive(localOffset);
return scrollbarPainter.hitTestOnlyThumbInteractive(localOffset, kind);
}
/// Returns true if the provided [Offset] is located over the track or thumb
/// of the [RawScrollbar].
@protected
bool isPointerOverScrollbar(Offset position) {
bool isPointerOverScrollbar(Offset position, PointerDeviceKind kind) {
if (_scrollbarPainterKey.currentContext == null) {
return false;
}
final Offset localOffset = _getLocalOffset(_scrollbarPainterKey, position);
return scrollbarPainter.hitTestInteractive(localOffset);
return scrollbarPainter.hitTestInteractive(localOffset, kind);
}

/// Cancels the fade out animation so the scrollbar will remain visible for
Expand All @@ -1194,7 +1210,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
@mustCallSuper
void handleHover(PointerHoverEvent event) {
// Check if the position of the pointer falls over the painted scrollbar
if (isPointerOverScrollbar(event.position)) {
if (isPointerOverScrollbar(event.position, event.kind)) {
_hoverIsActive = true;
_fadeoutTimer?.cancel();
} else if (_hoverIsActive) {
Expand Down Expand Up @@ -1292,20 +1308,20 @@ class _ThumbPressGestureRecognizer extends LongPressGestureRecognizer {

@override
bool isPointerAllowed(PointerDownEvent event) {
if (!_hitTestInteractive(_customPaintKey, event.position)) {
if (!_hitTestInteractive(_customPaintKey, event.position, event.kind)) {
return false;
}
return super.isPointerAllowed(event);
}

bool _hitTestInteractive(GlobalKey customPaintKey, Offset offset) {
bool _hitTestInteractive(GlobalKey customPaintKey, Offset offset, PointerDeviceKind kind) {
if (customPaintKey.currentContext == null) {
return false;
}
final CustomPaint customPaint = customPaintKey.currentContext!.widget as CustomPaint;
final ScrollbarPainter painter = customPaint.foregroundPainter! as ScrollbarPainter;
final Offset localOffset = _getLocalOffset(customPaintKey, offset);
return painter.hitTestOnlyThumbInteractive(localOffset);
return painter.hitTestOnlyThumbInteractive(localOffset, kind);
}
}

Expand All @@ -1322,21 +1338,21 @@ class _TrackTapGestureRecognizer extends TapGestureRecognizer {

@override
bool isPointerAllowed(PointerDownEvent event) {
if (!_hitTestInteractive(_customPaintKey, event.position)) {
if (!_hitTestInteractive(_customPaintKey, event.position, event.kind)) {
return false;
}
return super.isPointerAllowed(event);
}

bool _hitTestInteractive(GlobalKey customPaintKey, Offset offset) {
bool _hitTestInteractive(GlobalKey customPaintKey, Offset offset, PointerDeviceKind kind) {
if (customPaintKey.currentContext == null) {
return false;
}
final CustomPaint customPaint = customPaintKey.currentContext!.widget as CustomPaint;
final ScrollbarPainter painter = customPaint.foregroundPainter! as ScrollbarPainter;
final Offset localOffset = _getLocalOffset(customPaintKey, offset);
// We only receive track taps that are not on the thumb.
return painter.hitTestInteractive(localOffset) && !painter.hitTestOnlyThumbInteractive(localOffset);
return painter.hitTestInteractive(localOffset, kind) && !painter.hitTestOnlyThumbInteractive(localOffset, kind);
}
}

Expand Down
89 changes: 89 additions & 0 deletions packages/flutter/test/widgets/scrollbar_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -866,4 +866,93 @@ void main() {
paintsExactlyCountTimes(#drawRect, 2),
);
});

testWidgets('Scrollbar hit test area adjusts for PointerDeviceKind', (WidgetTester tester) async {
final ScrollController scrollController = ScrollController();
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: MediaQuery(
data: const MediaQueryData(),
child: PrimaryScrollController(
controller: scrollController,
child: RawScrollbar(
isAlwaysShown: true,
controller: scrollController,
child: const SingleChildScrollView(
child: SizedBox(width: 4000.0, height: 4000.0)
),
),
),
),
),
);
await tester.pumpAndSettle();
expect(scrollController.offset, 0.0);
expect(
find.byType(RawScrollbar),
paints
..rect(rect: const Rect.fromLTRB(794.0, 0.0, 800.0, 600.0))
..rect(
rect: const Rect.fromLTRB(794.0, 0.0, 800.0, 90.0),
color: const Color(0x66BCBCBC),
),
);

// Drag the scrollbar just outside of the painted thumb with touch input.
// The hit test area is padded to meet the minimum interactive size.
const double scrollAmount = 10.0;
final TestGesture dragScrollbarGesture = await tester.startGesture(const Offset(790.0, 45.0));
await tester.pumpAndSettle();
await dragScrollbarGesture.moveBy(const Offset(0.0, scrollAmount));
await tester.pumpAndSettle();

// The scrollbar moved by scrollAmount, and the scrollOffset moved forward.
expect(scrollController.offset, greaterThan(0.0));
expect(
find.byType(RawScrollbar),
paints
..rect(rect: const Rect.fromLTRB(794.0, 0.0, 800.0, 600.0))
..rect(
rect: const Rect.fromLTRB(794.0, 10.0, 800.0, 100.0),
color: const Color(0x66BCBCBC),
),
);

// Move back to reset.
await dragScrollbarGesture.moveBy(const Offset(0.0, -scrollAmount));
await tester.pumpAndSettle();
expect(scrollController.offset, 0.0);
expect(
find.byType(RawScrollbar),
paints
..rect(rect: const Rect.fromLTRB(794.0, 0.0, 800.0, 600.0))
..rect(
rect: const Rect.fromLTRB(794.0, 0.0, 800.0, 90.0),
color: const Color(0x66BCBCBC),
),
);

// The same should not be possible with a mouse since it is more precise,
// the padding it not necessary.
final TestGesture gesture = await tester.createGesture(kind: ui.PointerDeviceKind.mouse);
await gesture.addPointer();
addTearDown(gesture.removePointer);
await gesture.down(const Offset(790.0, 45.0));
await tester.pump();
await gesture.moveTo(const Offset(790.0, 55.0));
await gesture.up();
await tester.pumpAndSettle();
// The scrollbar/scrollable should not have moved.
expect(scrollController.offset, 0.0);
expect(
find.byType(RawScrollbar),
paints
..rect(rect: const Rect.fromLTRB(794.0, 0.0, 800.0, 600.0))
..rect(
rect: const Rect.fromLTRB(794.0, 0.0, 800.0, 90.0),
color: const Color(0x66BCBCBC),
),
);
});
}

0 comments on commit e02621b

Please sign in to comment.