From a3fe33af65058fe3e3c55ca4dedf283fa80bcdef Mon Sep 17 00:00:00 2001 From: xubaolin Date: Sat, 29 Aug 2020 01:38:05 +0800 Subject: [PATCH] The OverscrollIndicator should not overflow the scrollable view's edge (#64239) --- .../lib/src/widgets/overscroll_indicator.dart | 13 +- .../widgets/overscroll_indicator_test.dart | 117 +++++++++++++++++- 2 files changed, 122 insertions(+), 8 deletions(-) diff --git a/packages/flutter/lib/src/widgets/overscroll_indicator.dart b/packages/flutter/lib/src/widgets/overscroll_indicator.dart index 50f7278fd72..04dc5bcb069 100644 --- a/packages/flutter/lib/src/widgets/overscroll_indicator.dart +++ b/packages/flutter/lib/src/widgets/overscroll_indicator.dart @@ -253,9 +253,15 @@ class _GlowingOverscrollIndicatorState extends State // scroll, e.g. when scrolling in the opposite direction again to hide // the glow. Otherwise, the glow would always stay in a fixed position, // even if the top of the content already scrolled away. - _leadingController._paintOffsetScrollPixels = -notification.metrics.pixels; + // For example (CustomScrollView with sliver before center), the scroll + // extent is [-200.0, 300.0], scroll in the opposite direction with 10.0 pixels + // before glow disappears, so the current pixels is -190.0, + // in this case, we should move the glow up 10.0 pixels and should not + // overflow the scrollable widget's edge. https://github.com/flutter/flutter/issues/64149. + _leadingController._paintOffsetScrollPixels = + -math.min(notification.metrics.pixels - notification.metrics.minScrollExtent, _leadingController._paintOffset); _trailingController._paintOffsetScrollPixels = - -(notification.metrics.maxScrollExtent - notification.metrics.pixels); + -math.min(notification.metrics.maxScrollExtent - notification.metrics.pixels, _trailingController._paintOffset); if (notification is OverscrollNotification) { _GlowController controller; @@ -269,9 +275,6 @@ class _GlowingOverscrollIndicatorState extends State final bool isLeading = controller == _leadingController; if (_lastNotificationType != OverscrollNotification) { final OverscrollIndicatorNotification confirmationNotification = OverscrollIndicatorNotification(leading: isLeading); - // It is possible that the scroll extent starts at non-zero. - if (isLeading) - confirmationNotification.paintOffset = notification.metrics.minScrollExtent; confirmationNotification.dispatch(context); _accepted[isLeading] = confirmationNotification._accepted; if (_accepted[isLeading]) { diff --git a/packages/flutter/test/widgets/overscroll_indicator_test.dart b/packages/flutter/test/widgets/overscroll_indicator_test.dart index c9e3cb6adfb..e080d247f6b 100644 --- a/packages/flutter/test/widgets/overscroll_indicator_test.dart +++ b/packages/flutter/test/widgets/overscroll_indicator_test.dart @@ -362,7 +362,116 @@ void main() { expect(painter, paints..save()..translate(y: 0.0)..scale()..circle()); }); - group('Modify glow position', () { + testWidgets('CustomScrollView overscroll indicator works well with [CustomScrollView.center] and [OverscrollIndicatorNotification.paintOffset]', (WidgetTester tester) async { + final Key centerKey = UniqueKey(); + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: ScrollConfiguration( + behavior: TestScrollBehavior2(), + child: NotificationListener( + onNotification: (OverscrollIndicatorNotification notification) { + if (notification.leading) { + notification.paintOffset = 50.0; + } + return false; + }, + child: CustomScrollView( + center: centerKey, + physics: const AlwaysScrollableScrollPhysics(), + slivers: [ + SliverList( + delegate: SliverChildBuilderDelegate( + (BuildContext context, int index) => Text('First sliver $index',), + childCount: 2, + ), + ), + SliverList( + key: centerKey, + delegate: SliverChildBuilderDelegate( + (BuildContext context, int index) => Text('Second sliver $index',), + childCount: 5, + ), + ), + ], + ), + ), + ), + ), + ); + + expect(find.text('First sliver 1'), findsNothing); + + await slowDrag(tester, const Offset(200.0, 200.0), const Offset(0.0, 5.0)); // offset will be magnified ten times + expect(find.text('First sliver 1'), findsOneWidget); + final RenderObject painter = tester.renderObject(find.byType(CustomPaint)); + // The OverscrollIndicator should respect the [OverscrollIndicatorNotification.paintOffset] setting. + expect(painter, paints..save()..translate(y: 50.0)..scale()..circle()); + }); + + testWidgets('The OverscrollIndicator should not overflow the scrollable view edge', (WidgetTester tester) async { + // Regressing test for https://github.com/flutter/flutter/issues/64149 + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: NotificationListener( + onNotification: (OverscrollIndicatorNotification notification) { + notification.paintOffset = 50.0; // both the leading and trailing indicator have a 50.0 pixels offset. + return false; + }, + child: const CustomScrollView( + slivers: [ + SliverToBoxAdapter(child: SizedBox(height: 2000.0)), + ], + ), + ), + ), + ); + final RenderObject painter = tester.renderObject(find.byType(CustomPaint)); + await slowDrag(tester, const Offset(200.0, 200.0), const Offset(0.0, 5.0)); + expect(painter, paints..save()..translate(y: 50.0)..scale()..circle()); + // Reverse scroll (30 pixels), and the offset < notification.paintOffset. + await tester.dragFrom(const Offset(200.0, 200.0), const Offset(0.0, -30.0)); + await tester.pump(); + // The OverscrollIndicator should move with the CustomScrollView. + expect(painter, paints..save()..translate(y: 50.0 - 30.0)..scale()..circle()); + + // Reverse scroll (30+20 pixels) and offset == notification.paintOffset. + await tester.dragFrom(const Offset(200.0, 200.0), const Offset(0.0, -20.0)); + await tester.pump(); + expect(painter, paints..save()..translate(y: 50.0 - 50.0)..scale()..circle()); + + // Reverse scroll (30+20+10 pixels) and offset > notification.paintOffset. + await tester.dragFrom(const Offset(200.0, 200.0), const Offset(0.0, -10.0)); + await tester.pump(); + // The OverscrollIndicator should not overflow the CustomScrollView's edge. + expect(painter, paints..save()..translate(y: 50.0 - 50.0)..scale()..circle()); + + await tester.pumpAndSettle(); // Finish the leading indicator. + + // trigger the trailing indicator + await slowDrag(tester, const Offset(200.0, 200.0), const Offset(0.0, -200.0)); + expect(painter, paints..scale(y: -1.0)..save()..translate(y: 50.0)..scale()..circle()); + + // Reverse scroll (30 pixels), and the offset < notification.paintOffset. + await tester.dragFrom(const Offset(200.0, 200.0), const Offset(0.0, 30.0)); + await tester.pump(); + // The OverscrollIndicator should move with the CustomScrollView. + expect(painter, paints..scale(y: -1.0)..save()..translate(y: 50.0 - 30.0)..scale()..circle()); + + // Reverse scroll (30+20 pixels) and offset == notification.paintOffset. + await tester.dragFrom(const Offset(200.0, 200.0), const Offset(0.0, 20.0)); + await tester.pump(); + expect(painter, paints..scale(y: -1.0)..save()..translate(y: 50.0 - 50.0)..scale()..circle()); + + // Reverse scroll (30+20+10 pixels) and offset > notification.paintOffset. + await tester.dragFrom(const Offset(200.0, 200.0), const Offset(0.0, 10.0)); + await tester.pump(); + // The OverscrollIndicator should not overflow the CustomScrollView's edge. + expect(painter, paints..scale(y: -1.0)..save()..translate(y: 50.0 - 50.0)..scale()..circle()); + }); + + group('[OverscrollIndicatorNotification.paintOffset] test', () { testWidgets('Leading', (WidgetTester tester) async { await tester.pumpWidget( Directionality( @@ -384,11 +493,12 @@ void main() { ); final RenderObject painter = tester.renderObject(find.byType(CustomPaint)); await slowDrag(tester, const Offset(200.0, 200.0), const Offset(0.0, 5.0)); + // The OverscrollIndicator should respect the [OverscrollIndicatorNotification.paintOffset] setting. expect(painter, paints..save()..translate(y: 50.0)..scale()..circle()); // Reverse scroll direction. await tester.dragFrom(const Offset(200.0, 200.0), const Offset(0.0, -30.0)); await tester.pump(); - // The painter should follow the scroll direction. + // The OverscrollIndicator should move with the CustomScrollView. expect(painter, paints..save()..translate(y: 50.0 - 30.0)..scale()..circle()); }); @@ -415,11 +525,12 @@ void main() { await tester.dragFrom(const Offset(200.0, 200.0), const Offset(200.0, -10000.0)); await tester.pump(); await slowDrag(tester, const Offset(200.0, 200.0), const Offset(0.0, -5.0)); + // The OverscrollIndicator should respect the [OverscrollIndicatorNotification.paintOffset] setting. expect(painter, paints..scale(y: -1.0)..save()..translate(y: 50.0)..scale()..circle()); // Reverse scroll direction. await tester.dragFrom(const Offset(200.0, 200.0), const Offset(0.0, 30.0)); await tester.pump(); - // The painter should follow the scroll direction. + // The OverscrollIndicator should move with the CustomScrollView. expect(painter, paints..scale(y: -1.0)..save()..translate(y: 50.0 - 30.0)..scale()..circle()); }); });