Unverified Commit 7bbcdc23 authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Prevent Scrollbar axis flipping when there is an oriented scroll controller (#87698)

parent 3dd81601
...@@ -1533,15 +1533,36 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv ...@@ -1533,15 +1533,36 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
); );
} }
// ScrollController takes precedence over ScrollNotification
bool _shouldUpdatePainter(Axis notificationAxis) {
final ScrollController? scrollController = widget.controller ??
PrimaryScrollController.of(context);
// Only update the painter of this scrollbar if the notification
// metrics do not conflict with the information we have from the scroll
// controller.
return
// We do not have a scroll controller dictating axis.
scrollController == null
// The scroll controller is not attached to a position.
|| !scrollController.hasClients
// The notification matches the scroll controller's axis.
|| scrollController.position.axis == notificationAxis;
}
bool _handleScrollMetricsNotification(ScrollMetricsNotification notification) { bool _handleScrollMetricsNotification(ScrollMetricsNotification notification) {
if (!widget.notificationPredicate(ScrollUpdateNotification(metrics: notification.metrics, context: notification.context))) if (!widget.notificationPredicate(ScrollUpdateNotification(metrics: notification.metrics, context: notification.context)))
return false; return false;
if (showScrollbar) { if (showScrollbar) {
if (_fadeoutAnimationController.status != AnimationStatus.forward if (_fadeoutAnimationController.status != AnimationStatus.forward
&& _fadeoutAnimationController.status != AnimationStatus.completed) && _fadeoutAnimationController.status != AnimationStatus.completed)
_fadeoutAnimationController.forward(); _fadeoutAnimationController.forward();
} }
scrollbarPainter.update(notification.metrics, notification.metrics.axisDirection);
final ScrollMetrics metrics = notification.metrics;
if (_shouldUpdatePainter(metrics.axis)) {
scrollbarPainter.update(metrics, metrics.axisDirection);
}
return false; return false;
} }
...@@ -1555,7 +1576,10 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv ...@@ -1555,7 +1576,10 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
if (_fadeoutAnimationController.status != AnimationStatus.dismissed if (_fadeoutAnimationController.status != AnimationStatus.dismissed
&& _fadeoutAnimationController.status != AnimationStatus.reverse) && _fadeoutAnimationController.status != AnimationStatus.reverse)
_fadeoutAnimationController.reverse(); _fadeoutAnimationController.reverse();
scrollbarPainter.update(metrics, metrics.axisDirection);
if (_shouldUpdatePainter(metrics.axis)) {
scrollbarPainter.update(metrics, metrics.axisDirection);
}
return false; return false;
} }
...@@ -1567,7 +1591,10 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv ...@@ -1567,7 +1591,10 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
_fadeoutAnimationController.forward(); _fadeoutAnimationController.forward();
_fadeoutTimer?.cancel(); _fadeoutTimer?.cancel();
scrollbarPainter.update(notification.metrics, notification.metrics.axisDirection);
if (_shouldUpdatePainter(metrics.axis)) {
scrollbarPainter.update(metrics, metrics.axisDirection);
}
} else if (notification is ScrollEndNotification) { } else if (notification is ScrollEndNotification) {
if (_dragScrollbarAxisOffset == null) if (_dragScrollbarAxisOffset == null)
_maybeStartFadeoutTimer(); _maybeStartFadeoutTimer();
......
...@@ -1637,4 +1637,65 @@ void main() { ...@@ -1637,4 +1637,65 @@ void main() {
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.byType(RawScrollbar), isNot(paints..rect())); // Not shown. expect(find.byType(RawScrollbar), isNot(paints..rect())); // Not shown.
}); });
testWidgets('Scrollbar will not flip axes based on notification is there is a scroll controller', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/87697
final ScrollController verticalScrollController = ScrollController();
final ScrollController horizontalScrollController = ScrollController();
Widget buildFrame() {
return Directionality(
textDirection: TextDirection.ltr,
child: MediaQuery(
data: const MediaQueryData(),
child: RawScrollbar(
isAlwaysShown: true,
controller: verticalScrollController,
// This scrollbar will receive scroll notifications from both nested
// scroll views of opposite axes, but should stay on the vertical
// axis that its scroll controller is associated with.
notificationPredicate: (ScrollNotification notification) => notification.depth <= 1,
child: SingleChildScrollView(
controller: verticalScrollController,
child: SingleChildScrollView(
scrollDirection: Axis.horizontal,
controller: horizontalScrollController,
child: const SizedBox(
width: 1000.0,
height: 1000.0,
),
),
),
),
),
);
}
await tester.pumpWidget(buildFrame());
await tester.pumpAndSettle();
expect(verticalScrollController.offset, 0.0);
expect(horizontalScrollController.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, 360.0),
color: const Color(0x66BCBCBC),
),
);
// Move the horizontal scroll view. The vertical scrollbar should not flip.
horizontalScrollController.jumpTo(10.0);
expect(verticalScrollController.offset, 0.0);
expect(horizontalScrollController.offset, 10.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, 360.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