Updating PrimaryScrollController for Desktop (#102099)

Piinks · web-flow · commit 784520bd0429 · 2022-06-09T16:49:45.000-05:00
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/colors_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/colors_demo.dart
@@ -96,6 +96,7 @@ class PaletteTabView extends StatelessWidget {
     final TextStyle blackTextStyle = textTheme.bodyText2!.copyWith(color: Colors.black);
     return Scrollbar(
       child: ListView(
+        primary: true,
         itemExtent: kColorItemHeight,
         children: <Widget>[
           ...primaryKeys.map<Widget>((int index) {
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/cupertino/cupertino_alert_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/cupertino/cupertino_alert_demo.dart
@@ -60,6 +60,7 @@ class _CupertinoAlertDemoState extends State<CupertinoAlertDemo> {
               children: <Widget>[
                 CupertinoScrollbar(
                   child: ListView(
+                    primary: true,
                     // Add more padding to the normal safe area.
                     padding: const EdgeInsets.symmetric(vertical: 24.0, horizontal: 72.0)
                         + MediaQuery.of(context).padding,
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/cupertino/cupertino_navigation_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/cupertino/cupertino_navigation_demo.dart
@@ -446,6 +446,7 @@ class CupertinoDemoTab2 extends StatelessWidget {
       ),
       child: CupertinoScrollbar(
         child: ListView(
+          primary: true,
           children: <Widget>[
             const CupertinoUserInterfaceLevel(
               data: CupertinoUserInterfaceLevelData.elevated,
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/cupertino/cupertino_text_field_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/cupertino/cupertino_text_field_demo.dart
@@ -168,6 +168,7 @@ class _CupertinoTextFieldDemoState extends State<CupertinoTextFieldDemo> {
         ),
         child: CupertinoScrollbar(
           child: ListView(
+            primary: true,
             children: <Widget>[
               Padding(
                 padding: const EdgeInsets.symmetric(vertical: 32.0, horizontal: 16.0),
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/backdrop_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/backdrop_demo.dart
@@ -104,6 +104,7 @@ class CategoryView extends StatelessWidget {
     final ThemeData theme = Theme.of(context);
     return Scrollbar(
       child: ListView(
+        primary: true,
         key: PageStorageKey<Category?>(category),
         padding: const EdgeInsets.symmetric(
           vertical: 16.0,
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/bottom_app_bar_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/bottom_app_bar_demo.dart
@@ -161,6 +161,7 @@ class _BottomAppBarDemoState extends State<BottomAppBarDemo> {
           ),
           body: Scrollbar(
             child: ListView(
+              primary: true,
               padding: const EdgeInsets.only(bottom: 88.0),
               children: <Widget>[
                 const _Heading('FAB Shape'),
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/cards_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/cards_demo.dart
@@ -375,6 +375,7 @@ class _CardsDemoState extends State<CardsDemo> {
       ),
       body: Scrollbar(
         child: ListView(
+          primary: true,
           padding: const EdgeInsets.only(top: 8.0, left: 8.0, right: 8.0),
           children: destinations.map<Widget>((TravelDestination destination) {
             Widget? child;
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/chip_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/chip_demo.dart
@@ -347,7 +347,12 @@ class _ChipDemoState extends State<ChipDemo> {
                 borderRadius: BorderRadius.circular(10.0),
               ))
             : theme.chipTheme,
-        child: Scrollbar(child: ListView(children: tiles)),
+        child: Scrollbar(
+          child: ListView(
+            primary: true,
+            children: tiles,
+          )
+        ),
       ),
       floatingActionButton: FloatingActionButton(
         onPressed: () => setState(_reset),
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/data_table_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/data_table_demo.dart
@@ -175,6 +175,7 @@ class _DataTableDemoState extends State<DataTableDemo> {
       ),
       body: Scrollbar(
         child: ListView(
+          primary: true,
           padding: const EdgeInsets.all(20.0),
           children: <Widget>[
             PaginatedDataTable(
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/elevation_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/elevation_demo.dart
@@ -64,7 +64,12 @@ class _ElevationDemoState extends State<ElevationDemo> {
           ),
         ],
       ),
-      body: Scrollbar(child: ListView(children: buildCards())),
+      body: Scrollbar(
+        child: ListView(
+          primary: true,
+          children: buildCards(),
+        ),
+      ),
     );
   }
 }
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/expansion_tile_list_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/expansion_tile_list_demo.dart
@@ -20,6 +20,7 @@ class ExpansionTileListDemo extends StatelessWidget {
       ),
       body: Scrollbar(
         child: ListView(
+          primary: true,
           children: <Widget>[
             const ListTile(title: Text('Top')),
             ExpansionTile(
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/full_screen_dialog_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/full_screen_dialog_demo.dart
@@ -162,6 +162,7 @@ class FullScreenDialogDemoState extends State<FullScreenDialogDemo> {
         onWillPop: _onWillPop,
         child: Scrollbar(
           child: ListView(
+            primary: true,
             padding: const EdgeInsets.all(16.0),
             children: <Widget>[
               Container(
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/icons_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/icons_demo.dart
@@ -62,6 +62,7 @@ class IconsDemoState extends State<IconsDemo> {
           bottom: false,
           child: Scrollbar(
             child: ListView(
+              primary: true,
               padding: const EdgeInsets.all(24.0),
               children: <Widget>[
                 _IconsDemoCard(handleIconButtonPress, Icons.face), // direction-agnostic icon
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/leave_behind_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/leave_behind_demo.dart
@@ -131,6 +131,7 @@ class LeaveBehindDemoState extends State<LeaveBehindDemo> {
     } else {
       body = Scrollbar(
         child: ListView(
+          primary: true,
           children: leaveBehindItems.map<Widget>((LeaveBehindItem item) {
             return _LeaveBehindListItem(
               confirmDismiss: _confirmDismiss,
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/list_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/list_demo.dart
@@ -256,6 +256,7 @@ class _ListDemoState extends State<ListDemo> {
       ),
       body: Scrollbar(
         child: ListView(
+          primary: true,
           padding: EdgeInsets.symmetric(vertical: _dense != null ? 4.0 : 8.0),
           children: listTiles.toList(),
         ),
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/overscroll_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/overscroll_demo.dart
@@ -64,6 +64,7 @@ class OverscrollDemoState extends State<OverscrollDemo> {
         onRefresh: _handleRefresh,
         child: Scrollbar(
           child: ListView.builder(
+            primary: true,
             padding: kMaterialListPadding,
             itemCount: _items.length,
             itemBuilder: (BuildContext context, int index) {
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/reorderable_list_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/reorderable_list_demo.dart
@@ -208,6 +208,7 @@ class _ListDemoState extends State<ReorderableListDemo> {
       ),
       body: Scrollbar(
         child: ReorderableListView(
+          primary: true,
           header: _itemType != _ReorderableListType.threeLine
               ? Padding(
                   padding: const EdgeInsets.all(8.0),
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/material/text_form_field_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/material/text_form_field_demo.dart
@@ -182,6 +182,7 @@ class TextFormFieldDemoState extends State<TextFormFieldDemo> {
           onWillPop: _warnUserAboutInvalidData,
           child: Scrollbar(
             child: SingleChildScrollView(
+              primary: true,
               dragStartBehavior: DragStartBehavior.down,
               padding: const EdgeInsets.symmetric(horizontal: 16.0),
               child: Column(
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/typography_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/typography_demo.dart
@@ -66,7 +66,12 @@ class TypographyDemo extends StatelessWidget {
       body: SafeArea(
         top: false,
         bottom: false,
-        child: Scrollbar(child: ListView(children: styleItems)),
+        child: Scrollbar(
+          child: ListView(
+            primary: true,
+            children: styleItems,
+          ),
+        ),
       ),
     );
   }
diff --git a/dev/integration_tests/flutter_gallery/lib/demo/video_demo.dart b/dev/integration_tests/flutter_gallery/lib/demo/video_demo.dart
@@ -418,6 +418,7 @@ class _VideoDemoState extends State<VideoDemo> with SingleTickerProviderStateMix
             connectedCompleter: connectedCompleter,
             child: Scrollbar(
               child: ListView(
+                primary: true,
                 children: <Widget>[
                   VideoCard(
                     title: 'Butterfly',
diff --git a/examples/api/lib/widgets/scrollbar/raw_scrollbar.0.dart b/examples/api/lib/widgets/scrollbar/raw_scrollbar.0.dart
@@ -45,8 +45,9 @@ class _MyStatefulWidgetState extends State<MyStatefulWidget> {
         children: <Widget>[
           SizedBox(
               width: constraints.maxWidth / 2,
-              // Only one scroll position can be attached to the
-              // PrimaryScrollController if using Scrollbars. Providing a
+              // When using the PrimaryScrollController and a Scrollbar
+              // together, only one ScrollPosition can be attached to the
+              // PrimaryScrollController at a time. Providing a
               // unique scroll controller to this scroll view prevents it
               // from attaching to the PrimaryScrollController.
               child: Scrollbar(
@@ -64,12 +65,15 @@ class _MyStatefulWidgetState extends State<MyStatefulWidget> {
               )),
           SizedBox(
               width: constraints.maxWidth / 2,
-              // This vertical scroll view has not been provided a
-              // ScrollController, so it is using the
-              // PrimaryScrollController.
+              // This vertical scroll view has primary set to true, so it is
+              // using the PrimaryScrollController. On mobile platforms, the
+              // PrimaryScrollController automatically attaches to vertical
+              // ScrollViews, unlike on Desktop platforms, where the primary
+              // parameter is required.
               child: Scrollbar(
                 thumbVisibility: true,
                 child: ListView.builder(
+                    primary: true,
                     itemCount: 100,
                     itemBuilder: (BuildContext context, int index) {
                       return Container(
diff --git a/examples/api/lib/widgets/scrollbar/raw_scrollbar.shape.0.dart b/examples/api/lib/widgets/scrollbar/raw_scrollbar.shape.0.dart
@@ -35,6 +35,11 @@ class MyStatelessWidget extends StatelessWidget {
         thumbColor: Colors.blue,
         thumbVisibility: true,
         child: ListView(
+          // On mobile platforms, setting primary to true is not required, as
+          // the PrimaryScrollController automatically attaches to vertical
+          // ScrollPositions. On desktop platforms however, using the
+          // PrimaryScrollController requires ScrollView.primary be set.
+          primary: true,
           physics: const BouncingScrollPhysics(),
           children: List<Text>.generate(
               100, (int index) => Text((index * index).toString())),
diff --git a/packages/flutter/lib/src/material/dropdown.dart b/packages/flutter/lib/src/material/dropdown.dart
@@ -309,6 +309,8 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
                   child: Scrollbar(
                     thumbVisibility: true,
                     child: ListView(
+                      // Ensure this always inherits the PrimaryScrollController
+                      primary: true,
                       padding: kMaterialListPadding,
                       shrinkWrap: true,
                       children: children,
diff --git a/packages/flutter/lib/src/widgets/nested_scroll_view.dart b/packages/flutter/lib/src/widgets/nested_scroll_view.dart
@@ -312,7 +312,19 @@ class NestedScrollView extends StatefulWidget {
     return <Widget>[
       ...headerSliverBuilder(context, bodyIsScrolled),
       SliverFillRemaining(
+        // The inner (body) scroll view must use this scroll controller so that
+        // the independent scroll positions can be kept in sync.
         child: PrimaryScrollController(
+          // The inner scroll view should always inherit this
+          // PrimaryScrollController, on every platform.
+          automaticallyInheritForPlatforms: TargetPlatform.values.toSet(),
+          // `PrimaryScrollController.scrollDirection` is not set, and so it is
+          // restricted to the default Axis.vertical.
+          // Ideally the inner and outer views would have the same
+          // scroll direction, and so we could assume
+          // `NestedScrollView.scrollDirection` for the PrimaryScrollController,
+          // but use cases already exist where the axes are mismatched.
+          // https://github.com/flutter/flutter/issues/102001
           controller: innerController,
           child: body,
         ),
diff --git a/packages/flutter/lib/src/widgets/primary_scroll_controller.dart b/packages/flutter/lib/src/widgets/primary_scroll_controller.dart
@@ -3,19 +3,33 @@
 // found in the LICENSE file.
 
 import 'package:flutter/foundation.dart';
+import 'package:flutter/painting.dart';
 
 import 'framework.dart';
+import 'scroll_configuration.dart';
 import 'scroll_controller.dart';
 
+const Set<TargetPlatform> _kMobilePlatforms = <TargetPlatform>{
+  TargetPlatform.android,
+  TargetPlatform.iOS,
+  TargetPlatform.fuchsia,
+};
+
 /// Associates a [ScrollController] with a subtree.
 ///
-/// When a [ScrollView] has [ScrollView.primary] set to true and is not given
-/// an explicit [ScrollController], the [ScrollView] uses [of] to find the
-/// [ScrollController] associated with its subtree.
+/// When a [ScrollView] has [ScrollView.primary] set to true, the [ScrollView]
+/// uses [of] to inherit the [PrimaryScrollController] associated with its
+/// subtree.
+///
+/// A ScrollView that doesn't have a controller or the primary flag set will
+/// inherit the PrimarySCrollController, if [shouldInherit] allows it. By
+/// default [shouldInherit] is true for mobile platforms when the ScrollView has
+/// a scroll direction of [Axis.vertical]. This automatic inheritance can be
+/// configured with [automaticallyInheritForPlatforms] and [scrollDirection].
 ///
-/// This mechanism can be used to provide default behavior for scroll views in a
-/// subtree. For example, the [Scaffold] uses this mechanism to implement the
-/// scroll-to-top gesture on iOS.
+/// Inheriting this ScrollController can provide default behavior for scroll
+/// views in a subtree. For example, the [Scaffold] uses this mechanism to
+/// implement the scroll-to-top gesture on iOS.
 ///
 /// Another default behavior handled by the PrimaryScrollController is default
 /// [ScrollAction]s. If a ScrollAction is not handled by an otherwise focused
@@ -34,14 +48,18 @@ class PrimaryScrollController extends InheritedWidget {
   const PrimaryScrollController({
     super.key,
     required ScrollController this.controller,
+    this.automaticallyInheritForPlatforms = _kMobilePlatforms,
+    this.scrollDirection = Axis.vertical,
     required super.child,
   }) : assert(controller != null);
 
   /// Creates a subtree without an associated [ScrollController].
   const PrimaryScrollController.none({
     super.key,
     required super.child,
-  }) : controller = null;
+  }) : automaticallyInheritForPlatforms = const <TargetPlatform>{},
+       scrollDirection = null,
+       controller = null;
 
   /// The [ScrollController] associated with the subtree.
   ///
@@ -51,6 +69,57 @@ class PrimaryScrollController extends InheritedWidget {
   ///    scroll controller.
   final ScrollController? controller;
 
+  /// The [Axis] this controller is configured for [ScrollView]s to
+  /// automatically inherit.
+  ///
+  /// Used in conjunction with [automaticallyInheritForPlatforms]. If the
+  /// current [TargetPlatform] is not included in
+  /// [automaticallyInheritForPlatforms], this is ignored.
+  ///
+  /// When null, no [ScrollView] in any Axis will automatically inherit this
+  /// controller. This is dissimilar to [PrimaryScrollController.none]. When a
+  /// PrimaryScrollController is inherited, ScrollView will insert
+  /// PrimaryScrollController.none into the tree to prevent further descendant
+  /// ScrollViews from inheriting the current PrimaryScrollController.
+  ///
+  /// Defaults to [Axis.vertical].
+  final Axis? scrollDirection;
+
+  /// The [TargetPlatform]s this controller is configured for [ScrollView]s to
+  /// automatically inherit.
+  ///
+  /// Used in conjunction with [scrollDirection]. If the [Axis] provided to
+  /// [shouldInherit] is not [scrollDirection], this is ignored.
+  ///
+  /// When empty, no ScrollView in any Axis will automatically inherit this
+  /// controller. Defaults to [TargetPlatformVariant.mobile].
+  final Set<TargetPlatform> automaticallyInheritForPlatforms;
+
+  /// Returns true if this PrimaryScrollController is configured to be
+  /// automatically inherited for the current [TargetPlatform] and the given
+  /// [Axis].
+  ///
+  /// This method is typically not called directly. [ScrollView] will call this
+  /// method if it has not been provided a [ScrollController] and
+  /// [ScrollView.primary] is unset.
+  ///
+  /// If a ScrollController has already been provided to
+  /// [ScrollView.controller], or [ScrollView.primary] is set, this is method is
+  /// not called by ScrollView as it will have determined whether or not to
+  /// inherit the PrimaryScrollController.
+  static bool shouldInherit(BuildContext context, Axis scrollDirection) {
+    final PrimaryScrollController? result = context.findAncestorWidgetOfExactType<PrimaryScrollController>();
+    if (result == null) {
+      return false;
+    }
+
+    final TargetPlatform platform = ScrollConfiguration.of(context).getPlatform(context);
+    if (result.automaticallyInheritForPlatforms.contains(platform)) {
+      return result.scrollDirection == scrollDirection;
+    }
+    return false;
+  }
+
   /// Returns the [ScrollController] most closely associated with the given
   /// context.
   ///
diff --git a/packages/flutter/lib/src/widgets/scroll_view.dart b/packages/flutter/lib/src/widgets/scroll_view.dart
diff --git a/packages/flutter/lib/src/widgets/scrollable.dart b/packages/flutter/lib/src/widgets/scrollable.dart
diff --git a/packages/flutter/lib/src/widgets/scrollbar.dart b/packages/flutter/lib/src/widgets/scrollbar.dart
diff --git a/packages/flutter/test/widgets/range_maintaining_scroll_physics_test.dart b/packages/flutter/test/widgets/range_maintaining_scroll_physics_test.dart
diff --git a/packages/flutter/test/widgets/scroll_view_test.dart b/packages/flutter/test/widgets/scroll_view_test.dart
diff --git a/packages/flutter/test/widgets/scrollbar_test.dart b/packages/flutter/test/widgets/scrollbar_test.dart

Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,12 @@ class _ElevationDemoState extends State<ElevationDemo> {`
`64`	`64`	`),`
`65`	`65`	`],`
`66`	`66`	`),`
`67`		`- body: Scrollbar(child: ListView(children: buildCards())),`
	`67`	`+ body: Scrollbar(`
	`68`	`+ child: ListView(`
	`69`	`+ primary: true,`
	`70`	`+ children: buildCards(),`
	`71`	`+ ),`
	`72`	`+ ),`
`68`	`73`	`);`
`69`	`74`	`}`
`70`	`75`	`}`