Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reland Refactor reorderable list semantics #124395

Merged
merged 1 commit into from
Apr 7, 2023
Merged

Reland Refactor reorderable list semantics #124395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
71 changes: 4 additions & 67 deletions packages/flutter/lib/src/material/reorderable_list.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(() {
Expand All @@ -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) {
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
);
}

Expand Down
58 changes: 58 additions & 0 deletions packages/flutter/lib/src/widgets/reorderable_list.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
);
}
Expand Down
25 changes: 17 additions & 8 deletions packages/flutter/test/material/reorderable_list_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,21 +673,30 @@ 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,
isFocusable: true,
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();
});
Expand Down Expand Up @@ -1644,7 +1653,7 @@ void main() {
DefaultMaterialLocalizations.delegate,
DefaultWidgetsLocalizations.delegate,
],
child:SizedBox(
child: SizedBox(
width: 100.0,
height: 100.0,
child: Directionality(
Expand Down
100 changes: 100 additions & 0 deletions packages/flutter/test/widgets/reorderable_list_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down