Unverified Commit e02621b9 authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Fix scrollbar hit testing based on pointer device kind (#77755)

parent 4d38ca6d
...@@ -352,7 +352,7 @@ class _MaterialScrollbarState extends RawScrollbarState<_MaterialScrollbar> { ...@@ -352,7 +352,7 @@ class _MaterialScrollbarState extends RawScrollbarState<_MaterialScrollbar> {
void handleHover(PointerHoverEvent event) { void handleHover(PointerHoverEvent event) {
super.handleHover(event); super.handleHover(event);
// Check if the position of the pointer falls over the painted scrollbar // 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 // Pointer is hovering over the scrollbar
setState(() { _hoverIsActive = true; }); setState(() { _hoverIsActive = true; });
_hoverAnimationController.forward(); _hoverAnimationController.forward();
......
...@@ -478,9 +478,10 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter { ...@@ -478,9 +478,10 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
return _paintScrollbar(canvas, size, thumbExtent, _lastAxisDirection!); 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. /// isn't too small to be interacted with by the user.
bool hitTestInteractive(Offset position) { bool hitTestInteractive(Offset position, PointerDeviceKind kind) {
if (_thumbRect == null) { if (_thumbRect == null) {
return false; return false;
} }
...@@ -488,19 +489,25 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter { ...@@ -488,19 +489,25 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
if (fadeoutOpacityAnimation.value == 0.0) { if (fadeoutOpacityAnimation.value == 0.0) {
return false; return false;
} }
final Rect interactiveScrollbarRect = _trackRect == null
? _thumbRect!.expandToInclude( final Rect interactiveRect = _trackRect ?? _thumbRect!;
Rect.fromCircle(center: _thumbRect!.center, radius: _kMinInteractiveSize / 2), switch (kind) {
) case PointerDeviceKind.touch:
: _trackRect!.expandToInclude( final Rect touchScrollbarRect = interactiveRect.expandToInclude(
Rect.fromCircle(center: _thumbRect!.center, radius: _kMinInteractiveSize / 2), 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. /// Same as hitTestInteractive, but excludes the track portion of the scrollbar.
/// Used to evaluate interactions with only the scrollbar thumb. /// Used to evaluate interactions with only the scrollbar thumb.
bool hitTestOnlyThumbInteractive(Offset position) { bool hitTestOnlyThumbInteractive(Offset position, PointerDeviceKind kind) {
if (_thumbRect == null) { if (_thumbRect == null) {
return false; return false;
} }
...@@ -508,10 +515,19 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter { ...@@ -508,10 +515,19 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
if (fadeoutOpacityAnimation.value == 0.0) { if (fadeoutOpacityAnimation.value == 0.0) {
return false; return false;
} }
final Rect interactiveThumbRect = _thumbRect!.expandToInclude(
switch (kind) {
case PointerDeviceKind.touch:
final Rect touchThumbRect = _thumbRect!.expandToInclude(
Rect.fromCircle(center: _thumbRect!.center, radius: _kMinInteractiveSize / 2), Rect.fromCircle(center: _thumbRect!.center, radius: _kMinInteractiveSize / 2),
); );
return interactiveThumbRect.contains(position); return touchThumbRect.contains(position);
case PointerDeviceKind.mouse:
case PointerDeviceKind.stylus:
case PointerDeviceKind.invertedStylus:
case PointerDeviceKind.unknown:
return _thumbRect!.contains(position);
}
} }
// Scrollbars are interactive. // Scrollbars are interactive.
...@@ -1153,33 +1169,33 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv ...@@ -1153,33 +1169,33 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
/// ///
/// Excludes the [RawScrollbar] thumb. /// Excludes the [RawScrollbar] thumb.
@protected @protected
bool isPointerOverTrack(Offset position) { bool isPointerOverTrack(Offset position, PointerDeviceKind kind) {
if (_scrollbarPainterKey.currentContext == null) { if (_scrollbarPainterKey.currentContext == null) {
return false; return false;
} }
final Offset localOffset = _getLocalOffset(_scrollbarPainterKey, position); final Offset localOffset = _getLocalOffset(_scrollbarPainterKey, position);
return scrollbarPainter.hitTestInteractive(localOffset) return scrollbarPainter.hitTestInteractive(localOffset, kind)
&& !scrollbarPainter.hitTestOnlyThumbInteractive(localOffset); && !scrollbarPainter.hitTestOnlyThumbInteractive(localOffset, kind);
} }
/// Returns true if the provided [Offset] is located over the thumb of the /// Returns true if the provided [Offset] is located over the thumb of the
/// [RawScrollbar]. /// [RawScrollbar].
@protected @protected
bool isPointerOverThumb(Offset position) { bool isPointerOverThumb(Offset position, PointerDeviceKind kind) {
if (_scrollbarPainterKey.currentContext == null) { if (_scrollbarPainterKey.currentContext == null) {
return false; return false;
} }
final Offset localOffset = _getLocalOffset(_scrollbarPainterKey, position); 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 /// Returns true if the provided [Offset] is located over the track or thumb
/// of the [RawScrollbar]. /// of the [RawScrollbar].
@protected @protected
bool isPointerOverScrollbar(Offset position) { bool isPointerOverScrollbar(Offset position, PointerDeviceKind kind) {
if (_scrollbarPainterKey.currentContext == null) { if (_scrollbarPainterKey.currentContext == null) {
return false; return false;
} }
final Offset localOffset = _getLocalOffset(_scrollbarPainterKey, position); 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 /// Cancels the fade out animation so the scrollbar will remain visible for
...@@ -1194,7 +1210,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv ...@@ -1194,7 +1210,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
@mustCallSuper @mustCallSuper
void handleHover(PointerHoverEvent event) { void handleHover(PointerHoverEvent event) {
// Check if the position of the pointer falls over the painted scrollbar // Check if the position of the pointer falls over the painted scrollbar
if (isPointerOverScrollbar(event.position)) { if (isPointerOverScrollbar(event.position, event.kind)) {
_hoverIsActive = true; _hoverIsActive = true;
_fadeoutTimer?.cancel(); _fadeoutTimer?.cancel();
} else if (_hoverIsActive) { } else if (_hoverIsActive) {
...@@ -1292,20 +1308,20 @@ class _ThumbPressGestureRecognizer extends LongPressGestureRecognizer { ...@@ -1292,20 +1308,20 @@ class _ThumbPressGestureRecognizer extends LongPressGestureRecognizer {
@override @override
bool isPointerAllowed(PointerDownEvent event) { bool isPointerAllowed(PointerDownEvent event) {
if (!_hitTestInteractive(_customPaintKey, event.position)) { if (!_hitTestInteractive(_customPaintKey, event.position, event.kind)) {
return false; return false;
} }
return super.isPointerAllowed(event); return super.isPointerAllowed(event);
} }
bool _hitTestInteractive(GlobalKey customPaintKey, Offset offset) { bool _hitTestInteractive(GlobalKey customPaintKey, Offset offset, PointerDeviceKind kind) {
if (customPaintKey.currentContext == null) { if (customPaintKey.currentContext == null) {
return false; return false;
} }
final CustomPaint customPaint = customPaintKey.currentContext!.widget as CustomPaint; final CustomPaint customPaint = customPaintKey.currentContext!.widget as CustomPaint;
final ScrollbarPainter painter = customPaint.foregroundPainter! as ScrollbarPainter; final ScrollbarPainter painter = customPaint.foregroundPainter! as ScrollbarPainter;
final Offset localOffset = _getLocalOffset(customPaintKey, offset); final Offset localOffset = _getLocalOffset(customPaintKey, offset);
return painter.hitTestOnlyThumbInteractive(localOffset); return painter.hitTestOnlyThumbInteractive(localOffset, kind);
} }
} }
...@@ -1322,13 +1338,13 @@ class _TrackTapGestureRecognizer extends TapGestureRecognizer { ...@@ -1322,13 +1338,13 @@ class _TrackTapGestureRecognizer extends TapGestureRecognizer {
@override @override
bool isPointerAllowed(PointerDownEvent event) { bool isPointerAllowed(PointerDownEvent event) {
if (!_hitTestInteractive(_customPaintKey, event.position)) { if (!_hitTestInteractive(_customPaintKey, event.position, event.kind)) {
return false; return false;
} }
return super.isPointerAllowed(event); return super.isPointerAllowed(event);
} }
bool _hitTestInteractive(GlobalKey customPaintKey, Offset offset) { bool _hitTestInteractive(GlobalKey customPaintKey, Offset offset, PointerDeviceKind kind) {
if (customPaintKey.currentContext == null) { if (customPaintKey.currentContext == null) {
return false; return false;
} }
...@@ -1336,7 +1352,7 @@ class _TrackTapGestureRecognizer extends TapGestureRecognizer { ...@@ -1336,7 +1352,7 @@ class _TrackTapGestureRecognizer extends TapGestureRecognizer {
final ScrollbarPainter painter = customPaint.foregroundPainter! as ScrollbarPainter; final ScrollbarPainter painter = customPaint.foregroundPainter! as ScrollbarPainter;
final Offset localOffset = _getLocalOffset(customPaintKey, offset); final Offset localOffset = _getLocalOffset(customPaintKey, offset);
// We only receive track taps that are not on the thumb. // 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);
} }
} }
......
...@@ -866,4 +866,93 @@ void main() { ...@@ -866,4 +866,93 @@ void main() {
paintsExactlyCountTimes(#drawRect, 2), 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),
),
);
});
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment