Reland Refactor reorderable list semantics (#124395)

Reland Refactor reorderable list semantics

packages/flutter/lib/src/material/reorderable_list.dart

@@ -5,13 +5,11 @@
 import 'dart:ui' show lerpDouble;
 
 import 'package:flutter/gestures.dart';
-import 'package:flutter/rendering.dart';
 import 'package:flutter/widgets.dart';
 
 import 'debug.dart';
 import 'icons.dart';
 import 'material.dart';
-import 'material_localizations.dart';
 import 'theme.dart';
 
 /// A list whose items the user can interactively reorder by dragging.
@@ -266,64 +264,6 @@ class ReorderableListView extends StatefulWidget {
 }
 
 class _ReorderableListViewState extends State<ReorderableListView> {
-  Widget _wrapWithSemantics(Widget child, int index) {
-    void reorder(int startIndex, int endIndex) {
-      if (startIndex != endIndex) {
-        widget.onReorder(startIndex, endIndex);
-      }
-    }
-
-    // First, determine which semantics actions apply.
-    final Map<CustomSemanticsAction, VoidCallback> semanticsActions = <CustomSemanticsAction, VoidCallback>{};
-
-    // Create the appropriate semantics actions.
-    void moveToStart() => reorder(index, 0);
-    void moveToEnd() => reorder(index, widget.itemCount);
-    void moveBefore() => reorder(index, index - 1);
-    // To move after, we go to index+2 because we are moving it to the space
-    // before index+2, which is after the space at index+1.
-    void moveAfter() => reorder(index, index + 2);
-
-    final MaterialLocalizations localizations = MaterialLocalizations.of(context);
-
-    // If the item can move to before its current position in the list.
-    if (index > 0) {
-      semanticsActions[CustomSemanticsAction(label: localizations.reorderItemToStart)] = moveToStart;
-      String reorderItemBefore = localizations.reorderItemUp;
-      if (widget.scrollDirection == Axis.horizontal) {
-        reorderItemBefore = Directionality.of(context) == TextDirection.ltr
-            ? localizations.reorderItemLeft
-            : localizations.reorderItemRight;
-      }
-      semanticsActions[CustomSemanticsAction(label: reorderItemBefore)] = moveBefore;
-    }
-
-    // If the item can move to after its current position in the list.
-    if (index < widget.itemCount - 1) {
-      String reorderItemAfter = localizations.reorderItemDown;
-      if (widget.scrollDirection == Axis.horizontal) {
-        reorderItemAfter = Directionality.of(context) == TextDirection.ltr
-            ? localizations.reorderItemRight
-            : localizations.reorderItemLeft;
-      }
-      semanticsActions[CustomSemanticsAction(label: reorderItemAfter)] = moveAfter;
-      semanticsActions[CustomSemanticsAction(label: localizations.reorderItemToEnd)] = moveToEnd;
-    }
-
-    // We pass toWrap with a GlobalKey into the item so that when it
-    // gets dragged, the accessibility framework can preserve the selected
-    // state of the dragging item.
-    //
-    // We also apply the relevant custom accessibility actions for moving the item
-    // up, down, to the start, and to the end of the list.
-    return MergeSemantics(
-      child: Semantics(
-        customSemanticsActions: semanticsActions,
-        child: child,
-      ),
-    );
-  }
-
   Widget _itemBuilder(BuildContext context, int index) {
     final Widget item = widget.itemBuilder(context, index);
     assert(() {
@@ -335,9 +275,6 @@ class _ReorderableListViewState extends State<ReorderableListView> {
       return true;
     }());
 
-    // TODO(goderbauer): The semantics stuff should probably happen inside
-    //   _ReorderableItem so the widget versions can have them as well.
-    final Widget itemWithSemantics = _wrapWithSemantics(item, index);
     final Key itemGlobalKey = _ReorderableListViewChildGlobalKey(item.key!, this);
 
     if (widget.buildDefaultDragHandles) {
@@ -350,7 +287,7 @@ class _ReorderableListViewState extends State<ReorderableListView> {
               return Stack(
                 key: itemGlobalKey,
                 children: <Widget>[
-                  itemWithSemantics,
+                  item,
                   Positioned.directional(
                     textDirection: Directionality.of(context),
                     start: 0,
@@ -370,7 +307,7 @@ class _ReorderableListViewState extends State<ReorderableListView> {
               return Stack(
                 key: itemGlobalKey,
                 children: <Widget>[
-                  itemWithSemantics,
+                  item,
                   Positioned.directional(
                     textDirection: Directionality.of(context),
                     top: 0,
@@ -394,14 +331,14 @@ class _ReorderableListViewState extends State<ReorderableListView> {
           return ReorderableDelayedDragStartListener(
             key: itemGlobalKey,
             index: index,
-            child: itemWithSemantics,
+            child: item,
           );
       }
     }
 
     return KeyedSubtree(
       key: itemGlobalKey,
-      child: itemWithSemantics,
+      child: item,
     );
   }
 

packages/flutter/lib/src/widgets/reorderable_list.dart

@@ -10,6 +10,7 @@ import 'basic.dart';
 import 'debug.dart';
 import 'framework.dart';
 import 'inherited_theme.dart';
+import 'localizations.dart';
 import 'media_query.dart';
 import 'overlay.dart';
 import 'scroll_controller.dart';
@@ -928,6 +929,63 @@ class SliverReorderableListState extends State<SliverReorderableList> with Ticke
       key: _ReorderableItemGlobalKey(child.key!, index, this),
       index: index,
       capturedThemes: InheritedTheme.capture(from: context, to: overlay.context),
+      child: _wrapWithSemantics(child, index),
+    );
+  }
+
+  Widget _wrapWithSemantics(Widget child, int index) {
+    void reorder(int startIndex, int endIndex) {
+      if (startIndex != endIndex) {
+        widget.onReorder(startIndex, endIndex);
+      }
+    }
+
+    // First, determine which semantics actions apply.
+    final Map<CustomSemanticsAction, VoidCallback> semanticsActions = <CustomSemanticsAction, VoidCallback>{};
+
+    // Create the appropriate semantics actions.
+    void moveToStart() => reorder(index, 0);
+    void moveToEnd() => reorder(index, widget.itemCount);
+    void moveBefore() => reorder(index, index - 1);
+    // To move after, go to index+2 because it is moved to the space
+    // before index+2, which is after the space at index+1.
+    void moveAfter() => reorder(index, index + 2);
+
+    final WidgetsLocalizations localizations = WidgetsLocalizations.of(context);
+    final bool isHorizontal = _scrollDirection == Axis.horizontal;
+    // If the item can move to before its current position in the list.
+    if (index > 0) {
+      semanticsActions[CustomSemanticsAction(label: localizations.reorderItemToStart)] = moveToStart;
+      String reorderItemBefore = localizations.reorderItemUp;
+      if (isHorizontal) {
+        reorderItemBefore = Directionality.of(context) == TextDirection.ltr
+            ? localizations.reorderItemLeft
+            : localizations.reorderItemRight;
+      }
+      semanticsActions[CustomSemanticsAction(label: reorderItemBefore)] = moveBefore;
+    }
+
+    // If the item can move to after its current position in the list.
+    if (index < widget.itemCount - 1) {
+      String reorderItemAfter = localizations.reorderItemDown;
+      if (isHorizontal) {
+        reorderItemAfter = Directionality.of(context) == TextDirection.ltr
+            ? localizations.reorderItemRight
+            : localizations.reorderItemLeft;
+      }
+      semanticsActions[CustomSemanticsAction(label: reorderItemAfter)] = moveAfter;
+      semanticsActions[CustomSemanticsAction(label: localizations.reorderItemToEnd)] = moveToEnd;
+    }
+
+    // Pass toWrap with a GlobalKey into the item so that when it
+    // gets dragged, the accessibility framework can preserve the selected
+    // state of the dragging item.
+    //
+    // Also apply the relevant custom accessibility actions for moving the item
+    // up, down, to the start, and to the end of the list.
+    return Semantics(
+      container: true,
+      customSemanticsActions: semanticsActions,
       child: child,
     );
   }

packages/flutter/test/material/reorderable_list_test.dart

@@ -673,8 +673,23 @@ void main() {
           // Get the switch tile's semantics:
           final SemanticsNode semanticsNode = tester.getSemantics(find.byKey(const Key('Switch tile')));
 
-          // Check for properties of both SwitchTile semantics and the ReorderableListView custom semantics actions.
+          // Check for ReorderableListView custom semantics actions.
           expect(semanticsNode, matchesSemantics(
+            customActions: const <CustomSemanticsAction>[
+              CustomSemanticsAction(label: 'Move up'),
+              CustomSemanticsAction(label: 'Move down'),
+              CustomSemanticsAction(label: 'Move to the end'),
+              CustomSemanticsAction(label: 'Move to the start'),
+            ],
+          ));
+
+          // Check for properties of SwitchTile semantics.
+          late SemanticsNode child;
+          semanticsNode.visitChildren((SemanticsNode node) {
+            child = node;
+            return false;
+          });
+          expect(child, matchesSemantics(
             hasToggledState: true,
             isToggled: true,
             isEnabled: true,
@@ -682,12 +697,6 @@ void main() {
             hasEnabledState: true,
             label: 'Switch tile',
             hasTapAction: true,
-            customActions: const <CustomSemanticsAction>[
-              CustomSemanticsAction(label: 'Move up'),
-              CustomSemanticsAction(label: 'Move down'),
-              CustomSemanticsAction(label: 'Move to the end'),
-              CustomSemanticsAction(label: 'Move to the start'),
-            ],
           ));
           handle.dispose();
         });
@@ -1644,7 +1653,7 @@ void main() {
         DefaultMaterialLocalizations.delegate,
         DefaultWidgetsLocalizations.delegate,
       ],
-      child:SizedBox(
+      child: SizedBox(
         width: 100.0,
         height: 100.0,
         child: Directionality(

packages/flutter/test/widgets/reorderable_list_test.dart

@@ -4,8 +4,11 @@
 
 import 'package:flutter/gestures.dart';
 import 'package:flutter/material.dart';
+import 'package:flutter/rendering.dart';
 import 'package:flutter_test/flutter_test.dart';
 
+import 'semantics_tester.dart';
+
 void main() {
   testWidgets('SliverReorderableList works well when having gestureSettings', (WidgetTester tester) async {
     // Regression test for https://github.com/flutter/flutter/issues/103404
@@ -64,6 +67,103 @@ void main() {
     expect(items, orderedEquals(<int>[1, 0, 2, 3, 4]));
   });
 
+  testWidgets('SliverReorderableList item has correct semantics', (WidgetTester tester) async {
+    final SemanticsTester semantics = SemanticsTester(tester);
+    const int itemCount = 5;
+    int onReorderCallCount = 0;
+    final List<int> items = List<int>.generate(itemCount, (int index) => index);
+
+    void handleReorder(int fromIndex, int toIndex) {
+      onReorderCallCount += 1;
+      if (toIndex > fromIndex) {
+        toIndex -= 1;
+      }
+      items.insert(toIndex, items.removeAt(fromIndex));
+    }
+    // The list has five elements of height 100
+    await tester.pumpWidget(
+      MaterialApp(
+        home: MediaQuery(
+          data: const MediaQueryData(gestureSettings: DeviceGestureSettings(touchSlop: 8.0)),
+          child: CustomScrollView(
+            slivers: <Widget>[
+              SliverReorderableList(
+                itemCount: itemCount,
+                itemBuilder: (BuildContext context, int index) {
+                  return SizedBox(
+                    key: ValueKey<int>(items[index]),
+                    height: 100,
+                    child: ReorderableDragStartListener(
+                      index: index,
+                      child: Text('item ${items[index]}'),
+                    ),
+                  );
+                },
+                onReorder: handleReorder,
+              )
+            ],
+          ),
+        ),
+      ),
+    );
+
+    expect(
+      semantics,
+      includesNodeWith(
+        label: 'item 0',
+        actions: <SemanticsAction>[SemanticsAction.customAction],
+      ),
+    );
+    final SemanticsNode node = tester.getSemantics(find.text('item 0'));
+
+    // perform custom action 'move down'.
+    tester.binding.pipelineOwner.semanticsOwner!.performAction(node.id, SemanticsAction.customAction, 0);
+    await tester.pumpAndSettle();
+
+    expect(onReorderCallCount, 1);
+    expect(items, orderedEquals(<int>[1, 0, 2, 3, 4]));
+
+    semantics.dispose();
+  });
+
+  testWidgets('SliverReorderableList custom semantics action has correct label', (WidgetTester tester) async {
+    const int itemCount = 5;
+    final List<int> items = List<int>.generate(itemCount, (int index) => index);
+    // The list has five elements of height 100
+    await tester.pumpWidget(
+      MaterialApp(
+        home: MediaQuery(
+          data: const MediaQueryData(gestureSettings: DeviceGestureSettings(touchSlop: 8.0)),
+          child: CustomScrollView(
+            slivers: <Widget>[
+              SliverReorderableList(
+                itemCount: itemCount,
+                itemBuilder: (BuildContext context, int index) {
+                  return SizedBox(
+                    key: ValueKey<int>(items[index]),
+                    height: 100,
+                    child: ReorderableDragStartListener(
+                      index: index,
+                      child: Text('item ${items[index]}'),
+                    ),
+                  );
+                },
+                onReorder: (int _, int __) { },
+              )
+            ],
+          ),
+        ),
+      ),
+    );
+    final SemanticsNode node = tester.getSemantics(find.text('item 0'));
+    final SemanticsData data = node.getSemanticsData();
+    expect(data.customSemanticsActionIds!.length, 2);
+    final CustomSemanticsAction action1 = CustomSemanticsAction.getAction(data.customSemanticsActionIds![0])!;
+    expect(action1.label, 'Move down');
+    final CustomSemanticsAction action2 = CustomSemanticsAction.getAction(data.customSemanticsActionIds![1])!;
+    expect(action2.label, 'Move to the end');
+  });
+
   // Regression test for https://github.com/flutter/flutter/issues/100451
   testWidgets('SliverReorderableList.builder respects findChildIndexCallback', (WidgetTester tester) async {
     bool finderCalled = false;