From 48b51c3f722eb579cffe0eec9fced10f2a1851bf Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Wed, 20 Oct 2021 17:36:34 -0500 Subject: [PATCH] Fix ScrollBehavior copyWith (#91834) --- .../lib/src/widgets/scroll_configuration.dart | 51 ++--- .../test/widgets/scroll_behavior_test.dart | 177 ++++++++++++++++++ 2 files changed, 205 insertions(+), 23 deletions(-) diff --git a/packages/flutter/lib/src/widgets/scroll_configuration.dart b/packages/flutter/lib/src/widgets/scroll_configuration.dart index 4aec4c324f..50ef4d25d2 100644 --- a/packages/flutter/lib/src/widgets/scroll_configuration.dart +++ b/packages/flutter/lib/src/widgets/scroll_configuration.dart @@ -85,19 +85,21 @@ class ScrollBehavior { /// the widget level, like [PageView.scrollBehavior], in order to change the /// default. ScrollBehavior copyWith({ - bool scrollbars = true, - bool overscroll = true, + bool? scrollbars, + bool? overscroll, Set? dragDevices, ScrollPhysics? physics, TargetPlatform? platform, + AndroidOverscrollIndicator? androidOverscrollIndicator, }) { return _WrappedScrollBehavior( delegate: this, - scrollbar: scrollbars, - overscrollIndicator: overscroll, + scrollbars: scrollbars ?? true, + overscroll: overscroll ?? true, physics: physics, platform: platform, dragDevices: dragDevices, + androidOverscrollIndicator: androidOverscrollIndicator ); } @@ -251,38 +253,40 @@ class ScrollBehavior { class _WrappedScrollBehavior implements ScrollBehavior { const _WrappedScrollBehavior({ required this.delegate, - this.scrollbar = true, - this.overscrollIndicator = true, + this.scrollbars = true, + this.overscroll = true, this.physics, this.platform, Set? dragDevices, - }) : _dragDevices = dragDevices; + AndroidOverscrollIndicator? androidOverscrollIndicator, + }) : _androidOverscrollIndicator = androidOverscrollIndicator, + _dragDevices = dragDevices; final ScrollBehavior delegate; - final bool scrollbar; - final bool overscrollIndicator; + final bool scrollbars; + final bool overscroll; final ScrollPhysics? physics; final TargetPlatform? platform; final Set? _dragDevices; + @override + final AndroidOverscrollIndicator? _androidOverscrollIndicator; @override Set get dragDevices => _dragDevices ?? delegate.dragDevices; @override - AndroidOverscrollIndicator get androidOverscrollIndicator => delegate.androidOverscrollIndicator; - @override - AndroidOverscrollIndicator? get _androidOverscrollIndicator => throw UnimplementedError(); + AndroidOverscrollIndicator get androidOverscrollIndicator => _androidOverscrollIndicator ?? delegate.androidOverscrollIndicator; @override Widget buildOverscrollIndicator(BuildContext context, Widget child, ScrollableDetails details) { - if (overscrollIndicator) + if (overscroll) return delegate.buildOverscrollIndicator(context, child, details); return child; } @override Widget buildScrollbar(BuildContext context, Widget child, ScrollableDetails details) { - if (scrollbar) + if (scrollbars) return delegate.buildScrollbar(context, child, details); return child; } @@ -294,19 +298,20 @@ class _WrappedScrollBehavior implements ScrollBehavior { @override ScrollBehavior copyWith({ - bool scrollbars = true, - bool overscroll = true, + bool? scrollbars, + bool? overscroll, ScrollPhysics? physics, TargetPlatform? platform, Set? dragDevices, AndroidOverscrollIndicator? androidOverscrollIndicator }) { return delegate.copyWith( - scrollbars: scrollbars, - overscroll: overscroll, - physics: physics, - platform: platform, - dragDevices: dragDevices, + scrollbars: scrollbars ?? this.scrollbars, + overscroll: overscroll ?? this.overscroll, + physics: physics ?? this.physics, + platform: platform ?? this.platform, + dragDevices: dragDevices ?? this.dragDevices, + androidOverscrollIndicator: androidOverscrollIndicator ?? this.androidOverscrollIndicator, ); } @@ -323,8 +328,8 @@ class _WrappedScrollBehavior implements ScrollBehavior { @override bool shouldNotify(_WrappedScrollBehavior oldDelegate) { return oldDelegate.delegate.runtimeType != delegate.runtimeType - || oldDelegate.scrollbar != scrollbar - || oldDelegate.overscrollIndicator != overscrollIndicator + || oldDelegate.scrollbars != scrollbars + || oldDelegate.overscroll != overscroll || oldDelegate.physics != physics || oldDelegate.platform != platform || setEquals(oldDelegate.dragDevices, dragDevices) diff --git a/packages/flutter/test/widgets/scroll_behavior_test.dart b/packages/flutter/test/widgets/scroll_behavior_test.dart index bbfe6c3e5e..a1f6afa0a2 100644 --- a/packages/flutter/test/widgets/scroll_behavior_test.dart +++ b/packages/flutter/test/widgets/scroll_behavior_test.dart @@ -125,4 +125,181 @@ void main() { expect(find.byType(StretchingOverscrollIndicator), findsOneWidget); expect(find.byType(GlowingOverscrollIndicator), findsNothing); }, variant: TargetPlatformVariant.only(TargetPlatform.android)); + + group('ScrollBehavior configuration is maintained over multiple copies', () { + testWidgets('dragDevices', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/91673 + const ScrollBehavior defaultBehavior = ScrollBehavior(); + expect(defaultBehavior.dragDevices, { + PointerDeviceKind.touch, + PointerDeviceKind.stylus, + PointerDeviceKind.invertedStylus, + }); + + // Use copyWith to modify drag devices + final ScrollBehavior onceCopiedBehavior = defaultBehavior.copyWith( + dragDevices: PointerDeviceKind.values.toSet(), + ); + expect(onceCopiedBehavior.dragDevices, PointerDeviceKind.values.toSet()); + + // Copy again. The previously modified drag devices should carry over. + final ScrollBehavior twiceCopiedBehavior = onceCopiedBehavior.copyWith(); + expect(twiceCopiedBehavior.dragDevices, PointerDeviceKind.values.toSet()); + }); + + testWidgets('androidOverscrollIndicator', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/91673 + const ScrollBehavior defaultBehavior = ScrollBehavior(); + expect(defaultBehavior.androidOverscrollIndicator, AndroidOverscrollIndicator.glow); + + // Use copyWith to modify androidOverscrollIndicator + final ScrollBehavior onceCopiedBehavior = defaultBehavior.copyWith( + androidOverscrollIndicator: AndroidOverscrollIndicator.stretch, + ); + expect(onceCopiedBehavior.androidOverscrollIndicator, AndroidOverscrollIndicator.stretch); + + // Copy again. The previously modified value should carry over. + final ScrollBehavior twiceCopiedBehavior = onceCopiedBehavior.copyWith(); + expect(twiceCopiedBehavior.androidOverscrollIndicator, AndroidOverscrollIndicator.stretch); + }); + + testWidgets('physics', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/91673 + late ScrollPhysics defaultPhysics; + late ScrollPhysics onceCopiedPhysics; + late ScrollPhysics twiceCopiedPhysics; + + await tester.pumpWidget(ScrollConfiguration( + // Default ScrollBehavior + behavior: const ScrollBehavior(), + child: Builder( + builder: (BuildContext context) { + final ScrollBehavior defaultBehavior = ScrollConfiguration.of(context); + // Copy once to change physics + defaultPhysics = defaultBehavior.getScrollPhysics(context); + return ScrollConfiguration( + behavior: defaultBehavior.copyWith(physics: const BouncingScrollPhysics()), + child: Builder( + builder: (BuildContext context) { + final ScrollBehavior onceCopiedBehavior = ScrollConfiguration.of(context); + onceCopiedPhysics = onceCopiedBehavior.getScrollPhysics(context); + return ScrollConfiguration( + // Copy again, physics should follow + behavior: onceCopiedBehavior.copyWith(), + child: Builder( + builder: (BuildContext context) { + twiceCopiedPhysics = ScrollConfiguration.of(context).getScrollPhysics(context); + return SingleChildScrollView(child: Container(height: 1000)); + } + ) + ); + } + ) + ); + } + ), + )); + + expect(defaultPhysics, const ClampingScrollPhysics(parent: RangeMaintainingScrollPhysics())); + expect(onceCopiedPhysics, const BouncingScrollPhysics()); + expect(twiceCopiedPhysics, const BouncingScrollPhysics()); + }); + + testWidgets('platform', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/91673 + late TargetPlatform defaultPlatform; + late TargetPlatform onceCopiedPlatform; + late TargetPlatform twiceCopiedPlatform; + + await tester.pumpWidget(ScrollConfiguration( + // Default ScrollBehavior + behavior: const ScrollBehavior(), + child: Builder( + builder: (BuildContext context) { + final ScrollBehavior defaultBehavior = ScrollConfiguration.of(context); + // Copy once to change physics + defaultPlatform = defaultBehavior.getPlatform(context); + return ScrollConfiguration( + behavior: defaultBehavior.copyWith(platform: TargetPlatform.fuchsia), + child: Builder( + builder: (BuildContext context) { + final ScrollBehavior onceCopiedBehavior = ScrollConfiguration.of(context); + onceCopiedPlatform = onceCopiedBehavior.getPlatform(context); + return ScrollConfiguration( + // Copy again, physics should follow + behavior: onceCopiedBehavior.copyWith(), + child: Builder( + builder: (BuildContext context) { + twiceCopiedPlatform = ScrollConfiguration.of(context).getPlatform(context); + return SingleChildScrollView(child: Container(height: 1000)); + } + ) + ); + } + ) + ); + } + ), + )); + + expect(defaultPlatform, TargetPlatform.android); + expect(onceCopiedPlatform, TargetPlatform.fuchsia); + expect(twiceCopiedPlatform, TargetPlatform.fuchsia); + }); + + Widget wrap(ScrollBehavior behavior) { + return Directionality( + textDirection: TextDirection.ltr, + child: MediaQuery( + data: const MediaQueryData(size: Size(500, 500)), + child: ScrollConfiguration( + behavior: behavior, + child: Builder( + builder: (BuildContext context) => SingleChildScrollView(child: Container(height: 1000)) + ) + ), + ) + ); + } + + testWidgets('scrollbar', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/91673 + const ScrollBehavior defaultBehavior = ScrollBehavior(); + await tester.pumpWidget(wrap(defaultBehavior)); + // Default adds a scrollbar + expect(find.byType(RawScrollbar), findsOneWidget); + final ScrollBehavior onceCopiedBehavior = defaultBehavior.copyWith(scrollbars: false); + + await tester.pumpWidget(wrap(onceCopiedBehavior)); + // Copy does not add scrollbar + expect(find.byType(RawScrollbar), findsNothing); + final ScrollBehavior twiceCopiedBehavior = onceCopiedBehavior.copyWith(); + + await tester.pumpWidget(wrap(twiceCopiedBehavior)); + // Second copy maintains scrollbar setting + expect(find.byType(RawScrollbar), findsNothing); + + // For default scrollbars + }, variant: TargetPlatformVariant.desktop()); + + testWidgets('overscroll', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/91673 + const ScrollBehavior defaultBehavior = ScrollBehavior(); + await tester.pumpWidget(wrap(defaultBehavior)); + // Default adds a glowing overscroll indicator + expect(find.byType(GlowingOverscrollIndicator), findsOneWidget); + final ScrollBehavior onceCopiedBehavior = defaultBehavior.copyWith(overscroll: false); + + await tester.pumpWidget(wrap(onceCopiedBehavior)); + // Copy does not add indicator + expect(find.byType(GlowingOverscrollIndicator), findsNothing); + final ScrollBehavior twiceCopiedBehavior = onceCopiedBehavior.copyWith(); + + await tester.pumpWidget(wrap(twiceCopiedBehavior)); + // Second copy maintains overscroll setting + expect(find.byType(GlowingOverscrollIndicator), findsNothing); + + // For default glowing indicator + }, variant: TargetPlatformVariant.only(TargetPlatform.android)); + }); }