Unverified Commit 27f29e75 authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Fix scrollbar config for hover events (#72833)

parent 3abb9e50
...@@ -98,9 +98,6 @@ class _ScrollbarState extends RawScrollbarState<Scrollbar> { ...@@ -98,9 +98,6 @@ class _ScrollbarState extends RawScrollbarState<Scrollbar> {
late ColorScheme _colorScheme; late ColorScheme _colorScheme;
// On Android, scrollbars should match native appearance. // On Android, scrollbars should match native appearance.
late bool _useAndroidScrollbar; late bool _useAndroidScrollbar;
// Hover events should be ignored on mobile, the exit event cannot be
// triggered, but the enter event can on tap.
late bool _isMobile;
Set<MaterialState> get _states => <MaterialState>{ Set<MaterialState> get _states => <MaterialState>{
if (_dragIsActive) MaterialState.dragged, if (_dragIsActive) MaterialState.dragged,
...@@ -174,7 +171,7 @@ class _ScrollbarState extends RawScrollbarState<Scrollbar> { ...@@ -174,7 +171,7 @@ class _ScrollbarState extends RawScrollbarState<Scrollbar> {
if (states.contains(MaterialState.hovered) && widget.showTrackOnHover) if (states.contains(MaterialState.hovered) && widget.showTrackOnHover)
return widget.hoverThickness ?? _kScrollbarThicknessWithTrack; return widget.hoverThickness ?? _kScrollbarThicknessWithTrack;
// The default scrollbar thickness is smaller on mobile. // The default scrollbar thickness is smaller on mobile.
return widget.thickness ?? (_kScrollbarThickness / (_isMobile ? 2 : 1)); return widget.thickness ?? (_kScrollbarThickness / (_useAndroidScrollbar ? 2 : 1));
}); });
} }
...@@ -196,18 +193,13 @@ class _ScrollbarState extends RawScrollbarState<Scrollbar> { ...@@ -196,18 +193,13 @@ class _ScrollbarState extends RawScrollbarState<Scrollbar> {
switch (theme.platform) { switch (theme.platform) {
case TargetPlatform.android: case TargetPlatform.android:
_useAndroidScrollbar = true; _useAndroidScrollbar = true;
_isMobile = true;
break; break;
case TargetPlatform.iOS: case TargetPlatform.iOS:
_useAndroidScrollbar = false;
_isMobile = true;
break;
case TargetPlatform.linux: case TargetPlatform.linux:
case TargetPlatform.fuchsia: case TargetPlatform.fuchsia:
case TargetPlatform.macOS: case TargetPlatform.macOS:
case TargetPlatform.windows: case TargetPlatform.windows:
_useAndroidScrollbar = false; _useAndroidScrollbar = false;
_isMobile = false;
break; break;
} }
super.didChangeDependencies(); super.didChangeDependencies();
...@@ -242,8 +234,6 @@ class _ScrollbarState extends RawScrollbarState<Scrollbar> { ...@@ -242,8 +234,6 @@ class _ScrollbarState extends RawScrollbarState<Scrollbar> {
@override @override
void handleHover(PointerHoverEvent event) { void handleHover(PointerHoverEvent event) {
// Hover events should not be triggered on mobile.
assert(!_isMobile);
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)) {
...@@ -259,8 +249,6 @@ class _ScrollbarState extends RawScrollbarState<Scrollbar> { ...@@ -259,8 +249,6 @@ class _ScrollbarState extends RawScrollbarState<Scrollbar> {
@override @override
void handleHoverExit(PointerExitEvent event) { void handleHoverExit(PointerExitEvent event) {
// Hover events should not be triggered on mobile.
assert(!_isMobile);
super.handleHoverExit(event); super.handleHoverExit(event);
setState(() { _hoverIsActive = false; }); setState(() { _hoverIsActive = false; });
_hoverAnimationController.reverse(); _hoverAnimationController.reverse();
......
...@@ -782,7 +782,6 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv ...@@ -782,7 +782,6 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
late Animation<double> _fadeoutOpacityAnimation; late Animation<double> _fadeoutOpacityAnimation;
final GlobalKey _scrollbarPainterKey = GlobalKey(); final GlobalKey _scrollbarPainterKey = GlobalKey();
bool _hoverIsActive = false; bool _hoverIsActive = false;
late bool _isMobile;
/// Used to paint the scrollbar. /// Used to paint the scrollbar.
...@@ -813,18 +812,6 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv ...@@ -813,18 +812,6 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
@override @override
void didChangeDependencies() { void didChangeDependencies() {
super.didChangeDependencies(); super.didChangeDependencies();
switch (defaultTargetPlatform) {
case TargetPlatform.android:
case TargetPlatform.iOS:
_isMobile = true;
break;
case TargetPlatform.fuchsia:
case TargetPlatform.linux:
case TargetPlatform.macOS:
case TargetPlatform.windows:
_isMobile = false;
break;
}
_maybeTriggerScrollbar(); _maybeTriggerScrollbar();
} }
...@@ -1165,27 +1152,42 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv ...@@ -1165,27 +1152,42 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
Widget build(BuildContext context) { Widget build(BuildContext context) {
updateScrollbarPainter(); updateScrollbarPainter();
Widget child = CustomPaint(
key: _scrollbarPainterKey,
foregroundPainter: scrollbarPainter,
child: RepaintBoundary(child: widget.child),
);
if (!_isMobile) {
// Hover events not supported on mobile.
child = MouseRegion(
onExit: handleHoverExit,
onHover: handleHover,
child: child
);
}
return NotificationListener<ScrollNotification>( return NotificationListener<ScrollNotification>(
onNotification: _handleScrollNotification, onNotification: _handleScrollNotification,
child: RepaintBoundary( child: RepaintBoundary(
child: RawGestureDetector( child: RawGestureDetector(
gestures: _gestures, gestures: _gestures,
child: child, child: MouseRegion(
onExit: (PointerExitEvent event) {
switch(event.kind) {
case PointerDeviceKind.mouse:
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:
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),
)
),
), ),
), ),
); );
......
...@@ -842,7 +842,7 @@ void main() { ...@@ -842,7 +842,7 @@ void main() {
}), }),
); );
testWidgets('Hover animation is not triggered on mobile', (WidgetTester tester) async { testWidgets('Hover animation is not triggered by tap gestures', (WidgetTester tester) async {
final ScrollController scrollController = ScrollController(); final ScrollController scrollController = ScrollController();
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
...@@ -864,7 +864,7 @@ void main() { ...@@ -864,7 +864,7 @@ void main() {
find.byType(Scrollbar), find.byType(Scrollbar),
paints..rrect( paints..rrect(
rrect: RRect.fromRectAndRadius( rrect: RRect.fromRectAndRadius(
const Rect.fromLTRB(794.0, 0.0, 798.0, 90.0), const Rect.fromLTRB(790.0, 0.0, 798.0, 90.0),
const Radius.circular(8.0), const Radius.circular(8.0),
), ),
color: const Color(0x1a000000), color: const Color(0x1a000000),
...@@ -873,18 +873,50 @@ void main() { ...@@ -873,18 +873,50 @@ void main() {
await tester.tapAt(const Offset(794.0, 5.0)); await tester.tapAt(const Offset(794.0, 5.0));
await tester.pumpAndSettle(); await tester.pumpAndSettle();
// Tapping on mobile triggers a hover enter event. In this case, the // Tapping triggers a hover enter event. In this case, the Scrollbar should
// Scrollbar should be unchanged since it ignores hover events on mobile. // be unchanged since it ignores hover events that aren't from a mouse.
expect( expect(
find.byType(Scrollbar), find.byType(Scrollbar),
paints..rrect( paints..rrect(
rrect: RRect.fromRectAndRadius( rrect: RRect.fromRectAndRadius(
const Rect.fromLTRB(794.0, 0.0, 798.0, 90.0), const Rect.fromLTRB(790.0, 0.0, 798.0, 90.0),
const Radius.circular(8.0), const Radius.circular(8.0),
), ),
color: const Color(0x1a000000), color: const Color(0x1a000000),
), ),
); );
// Now trigger hover with a mouse.
final TestGesture gesture = await tester.createGesture(kind: ui.PointerDeviceKind.mouse);
await gesture.addPointer();
addTearDown(gesture.removePointer);
await gesture.moveTo(const Offset(794.0, 5.0));
await tester.pump();
expect(
find.byType(Scrollbar),
paints
..rect(
rect: const Rect.fromLTRB(784.0, 0.0, 800.0, 600.0),
color: const Color(0x08000000),
)
..line(
p1: const Offset(784.0, 0.0),
p2: const Offset(784.0, 600.0),
strokeWidth: 1.0,
color: const Color(0x1a000000),
)
..rrect(
rrect: RRect.fromRectAndRadius(
// Scrollbar thumb is larger
const Rect.fromLTRB(786.0, 0.0, 798.0, 90.0),
const Radius.circular(8.0),
),
// Hover color
color: const Color(0x80000000),
),
);
}, },
variant: const TargetPlatformVariant(<TargetPlatform>{ variant: const TargetPlatformVariant(<TargetPlatform>{
TargetPlatform.iOS, TargetPlatform.iOS,
......
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