From 7bbcdc239b092cd26ccd81351406077a5512061c Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Thu, 5 Aug 2021 16:10:06 -0400 Subject: [PATCH] Prevent Scrollbar axis flipping when there is an oriented scroll controller (#87698) --- .../flutter/lib/src/widgets/scrollbar.dart | 33 +++++++++- .../flutter/test/widgets/scrollbar_test.dart | 61 +++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/packages/flutter/lib/src/widgets/scrollbar.dart b/packages/flutter/lib/src/widgets/scrollbar.dart index b23d6876db9..64013771e43 100644 --- a/packages/flutter/lib/src/widgets/scrollbar.dart +++ b/packages/flutter/lib/src/widgets/scrollbar.dart @@ -1533,15 +1533,36 @@ class RawScrollbarState extends State 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) { if (!widget.notificationPredicate(ScrollUpdateNotification(metrics: notification.metrics, context: notification.context))) return false; + if (showScrollbar) { if (_fadeoutAnimationController.status != AnimationStatus.forward && _fadeoutAnimationController.status != AnimationStatus.completed) _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; } @@ -1555,7 +1576,10 @@ class RawScrollbarState extends State with TickerProv if (_fadeoutAnimationController.status != AnimationStatus.dismissed && _fadeoutAnimationController.status != AnimationStatus.reverse) _fadeoutAnimationController.reverse(); - scrollbarPainter.update(metrics, metrics.axisDirection); + + if (_shouldUpdatePainter(metrics.axis)) { + scrollbarPainter.update(metrics, metrics.axisDirection); + } return false; } @@ -1567,7 +1591,10 @@ class RawScrollbarState extends State with TickerProv _fadeoutAnimationController.forward(); _fadeoutTimer?.cancel(); - scrollbarPainter.update(notification.metrics, notification.metrics.axisDirection); + + if (_shouldUpdatePainter(metrics.axis)) { + scrollbarPainter.update(metrics, metrics.axisDirection); + } } else if (notification is ScrollEndNotification) { if (_dragScrollbarAxisOffset == null) _maybeStartFadeoutTimer(); diff --git a/packages/flutter/test/widgets/scrollbar_test.dart b/packages/flutter/test/widgets/scrollbar_test.dart index 400805793f6..1554c6ff881 100644 --- a/packages/flutter/test/widgets/scrollbar_test.dart +++ b/packages/flutter/test/widgets/scrollbar_test.dart @@ -1637,4 +1637,65 @@ void main() { await tester.pumpAndSettle(); 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), + ), + ); + }); }