From 9f4b9bfd494a209f5dc2e0cf3cec6322be4c5b57 Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Thu, 23 Jun 2022 12:27:09 -0500 Subject: [PATCH] Apply PrimaryScrollController updates to SingleChildScrollView (#106430) --- .../scroll_metrics_notification.0.dart | 1 + .../src/widgets/single_child_scroll_view.dart | 39 ++++++++--------- .../flutter/test/widgets/scrollbar_test.dart | 1 + .../single_child_scroll_view_test.dart | 43 ++++++++++++++++--- 4 files changed, 56 insertions(+), 28 deletions(-) diff --git a/examples/api/lib/widgets/scroll_position/scroll_metrics_notification.0.dart b/examples/api/lib/widgets/scroll_position/scroll_metrics_notification.0.dart index c5522cfa336c..75bd6a8e94f4 100644 --- a/examples/api/lib/widgets/scroll_position/scroll_metrics_notification.0.dart +++ b/examples/api/lib/widgets/scroll_position/scroll_metrics_notification.0.dart @@ -46,6 +46,7 @@ class ScrollMetricsDemoState extends State { height: windowSize, width: double.infinity, child: const SingleChildScrollView( + primary: true, child: FlutterLogo( size: 300.0, ), diff --git a/packages/flutter/lib/src/widgets/single_child_scroll_view.dart b/packages/flutter/lib/src/widgets/single_child_scroll_view.dart index 007c4dd87c6b..ea34041e07d5 100644 --- a/packages/flutter/lib/src/widgets/single_child_scroll_view.dart +++ b/packages/flutter/lib/src/widgets/single_child_scroll_view.dart @@ -142,7 +142,7 @@ class SingleChildScrollView extends StatelessWidget { this.scrollDirection = Axis.vertical, this.reverse = false, this.padding, - bool? primary, + this.primary, this.physics, this.controller, this.child, @@ -153,11 +153,12 @@ class SingleChildScrollView extends StatelessWidget { }) : assert(scrollDirection != null), assert(dragStartBehavior != null), assert(clipBehavior != null), - assert(!(controller != null && (primary ?? false)), - 'Primary ScrollViews obtain their ScrollController via inheritance from a PrimaryScrollController widget. ' - 'You cannot both set primary to true and pass an explicit controller.', - ), - primary = primary ?? controller == null && identical(scrollDirection, Axis.vertical); + assert( + !(controller != null && (primary ?? false)), + 'Primary ScrollViews obtain their ScrollController via inheritance ' + 'from a PrimaryScrollController widget. You cannot both set primary to ' + 'true and pass an explicit controller.', + ); /// The axis along which the scroll view scrolls. /// @@ -195,20 +196,8 @@ class SingleChildScrollView extends StatelessWidget { /// [ScrollController.animateTo]). final ScrollController? controller; - /// Whether this is the primary scroll view associated with the parent - /// [PrimaryScrollController]. - /// - /// When true, the scroll view is used for default [ScrollAction]s. If a - /// ScrollAction is not handled by an otherwise focused part of the application, - /// the ScrollAction will be evaluated using this scroll view, for example, - /// when executing [Shortcuts] key events like page up and down. - /// - /// On iOS, this identifies the scroll view that will scroll to top in - /// response to a tap in the status bar. - /// - /// Defaults to true when [scrollDirection] is vertical and [controller] is - /// not specified. - final bool primary; + /// {@macro flutter.widgets.scroll_view.primary} + final bool? primary; /// How the scroll view should respond to user input. /// @@ -248,9 +237,13 @@ class SingleChildScrollView extends StatelessWidget { if (padding != null) { contents = Padding(padding: padding!, child: contents); } - final ScrollController? scrollController = primary + final bool effectivePrimary = primary + ?? controller == null && PrimaryScrollController.shouldInherit(context, scrollDirection); + + final ScrollController? scrollController = effectivePrimary ? PrimaryScrollController.of(context) : controller; + Widget scrollable = Scrollable( dragStartBehavior: dragStartBehavior, axisDirection: axisDirection, @@ -280,7 +273,9 @@ class SingleChildScrollView extends StatelessWidget { ); } - return primary && scrollController != null + return effectivePrimary && scrollController != null + // Further descendant ScrollViews will not inherit the same + // PrimaryScrollController ? PrimaryScrollController.none(child: scrollable) : scrollable; } diff --git a/packages/flutter/test/widgets/scrollbar_test.dart b/packages/flutter/test/widgets/scrollbar_test.dart index 74f9ba1b54a8..c0ad0a8b0bf1 100644 --- a/packages/flutter/test/widgets/scrollbar_test.dart +++ b/packages/flutter/test/widgets/scrollbar_test.dart @@ -1079,6 +1079,7 @@ void main() { isAlwaysShown: true, controller: scrollController, child: const SingleChildScrollView( + primary: true, child: SizedBox(width: 4000.0, height: 4000.0), ), ), diff --git a/packages/flutter/test/widgets/single_child_scroll_view_test.dart b/packages/flutter/test/widgets/single_child_scroll_view_test.dart index 2ed8aac15945..2906650f2aa2 100644 --- a/packages/flutter/test/widgets/single_child_scroll_view_test.dart +++ b/packages/flutter/test/widgets/single_child_scroll_view_test.dart @@ -34,6 +34,19 @@ class TestScrollController extends ScrollController { } } +Widget primaryScrollControllerBoilerplate({ required Widget child, required ScrollController controller }) { + return Directionality( + textDirection: TextDirection.ltr, + child: MediaQuery( + data: const MediaQueryData(), + child: PrimaryScrollController( + controller: controller, + child: child, + ), + ), + ); +} + void main() { testWidgets('SingleChildScrollView overflow and clipRect test', (WidgetTester tester) async { // the test widowSize is Size(800.0, 600.0) @@ -210,23 +223,41 @@ void main() { )); }); - testWidgets('Vertical SingleChildScrollViews are primary by default', (WidgetTester tester) async { + testWidgets('Vertical SingleChildScrollViews are not primary by default', (WidgetTester tester) async { const SingleChildScrollView view = SingleChildScrollView(); - expect(view.primary, isTrue); + expect(view.primary, isNull); }); - testWidgets('Horizontal SingleChildScrollViews are non-primary by default', (WidgetTester tester) async { + testWidgets('Horizontal SingleChildScrollViews are not primary by default', (WidgetTester tester) async { const SingleChildScrollView view = SingleChildScrollView(scrollDirection: Axis.horizontal); - expect(view.primary, isFalse); + expect(view.primary, isNull); }); - testWidgets('SingleChildScrollViews with controllers are non-primary by default', (WidgetTester tester) async { + testWidgets('SingleChildScrollViews with controllers are not primary by default', (WidgetTester tester) async { final SingleChildScrollView view = SingleChildScrollView( controller: ScrollController(), ); - expect(view.primary, isFalse); + expect(view.primary, isNull); }); + testWidgets('Vertical SingleChildScrollViews use PrimaryScrollController by default on mobile', (WidgetTester tester) async { + final ScrollController controller = ScrollController(); + await tester.pumpWidget(primaryScrollControllerBoilerplate( + child: const SingleChildScrollView(), + controller: controller, + )); + expect(controller.hasClients, isTrue); + }, variant: TargetPlatformVariant.mobile()); + + testWidgets("Vertical SingleChildScrollViews don't use PrimaryScrollController by default on desktop", (WidgetTester tester) async { + final ScrollController controller = ScrollController(); + await tester.pumpWidget(primaryScrollControllerBoilerplate( + child: const SingleChildScrollView(), + controller: controller, + )); + expect(controller.hasClients, isFalse); + }, variant: TargetPlatformVariant.desktop()); + testWidgets('Nested scrollables have a null PrimaryScrollController', (WidgetTester tester) async { const Key innerKey = Key('inner'); final ScrollController primaryScrollController = ScrollController();