Skip to content

Commit e869144

Browse files
chunhtaichunhtai
authored
Reland "Fixes ability to call nextFocus() on a node to focus its desc… (flutter#136898)
�endant" (flutter#136894)" This reverts commit c2bd2c1. fixes flutter#134854 This is a straight reland, the internal test is testing a wrong behave. https://critique.corp.google.com/cl/575028981
1 parent af129b6 commit e869144

File tree

2 files changed

+122
-6
lines changed

2 files changed

+122
-6
lines changed

‎packages/flutter/lib/src/widgets/focus_traversal.dart

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ abstract class FocusTraversalPolicy with Diagnosticable {
306306
final FocusScopeNode scope = currentNode.nearestScope!;
307307
FocusNode? candidate = scope.focusedChild;
308308
if (ignoreCurrentFocus || candidate == null && scope.descendants.isNotEmpty) {
309-
final Iterable<FocusNode> sorted = _sortAllDescendants(scope, currentNode);
309+
final Iterable<FocusNode> sorted = _sortAllDescendants(scope, currentNode).where((FocusNode node) => _canRequestTraversalFocus(node));
310310
if (sorted.isEmpty) {
311311
candidate = null;
312312
} else {
@@ -405,13 +405,17 @@ abstract class FocusTraversalPolicy with Diagnosticable {
405405
@protected
406406
Iterable<FocusNode> sortDescendants(Iterable<FocusNode> descendants, FocusNode currentNode);
407407

408+
static bool _canRequestTraversalFocus(FocusNode node) {
409+
return node.canRequestFocus && !node.skipTraversal;
410+
}
411+
408412
static Iterable<FocusNode> _getDescendantsWithoutExpandingScope(FocusNode node) {
409413
final List<FocusNode> result = <FocusNode>[];
410414
for (final FocusNode child in node.children) {
415+
result.add(child);
411416
if (child is! FocusScopeNode) {
412417
result.addAll(_getDescendantsWithoutExpandingScope(child));
413418
}
414-
result.add(child);
415419
}
416420
return result;
417421
}
@@ -488,15 +492,15 @@ abstract class FocusTraversalPolicy with Diagnosticable {
488492
// They were left in above because they were needed to find their members
489493
// during sorting.
490494
sortedDescendants.removeWhere((FocusNode node) {
491-
return node != currentNode && (!node.canRequestFocus || node.skipTraversal);
495+
return node != currentNode && !_canRequestTraversalFocus(node);
492496
});
493497

494498
// Sanity check to make sure that the algorithm above doesn't diverge from
495499
// the one in FocusScopeNode.traversalDescendants in terms of which nodes it
496500
// finds.
497501
assert((){
498502
final Set<FocusNode> difference = sortedDescendants.toSet().difference(scope.traversalDescendants.toSet());
499-
if (currentNode.skipTraversal || !currentNode.canRequestFocus) {
503+
if (!_canRequestTraversalFocus(currentNode)) {
500504
// The scope.traversalDescendants will not contain currentNode if it
501505
// skips traversal or not focusable.
502506
assert(

‎packages/flutter/test/widgets/focus_traversal_test.dart

Lines changed: 114 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,113 @@ void main() {
9696
expect(scope.hasFocus, isTrue);
9797
});
9898

99+
testWidgetsWithLeakTracking('focus traversal should work case 1', (WidgetTester tester) async {
100+
final FocusNode outer1 = FocusNode(debugLabel: 'outer1', skipTraversal: true);
101+
final FocusNode outer2 = FocusNode(debugLabel: 'outer2', skipTraversal: true);
102+
final FocusNode inner1 = FocusNode(debugLabel: 'inner1', );
103+
final FocusNode inner2 = FocusNode(debugLabel: 'inner2', );
104+
addTearDown(() {
105+
outer1.dispose();
106+
outer2.dispose();
107+
inner1.dispose();
108+
inner2.dispose();
109+
});
110+
111+
await tester.pumpWidget(
112+
Directionality(
113+
textDirection: TextDirection.ltr,
114+
child: FocusTraversalGroup(
115+
child: Row(
116+
children: <Widget>[
117+
FocusScope(
118+
child: Focus(
119+
focusNode: outer1,
120+
child: Focus(
121+
focusNode: inner1,
122+
child: const SizedBox(width: 10, height: 10),
123+
),
124+
),
125+
),
126+
FocusScope(
127+
child: Focus(
128+
focusNode: outer2,
129+
// Add a padding to ensure both Focus widgets have different
130+
// sizes.
131+
child: Padding(
132+
padding: const EdgeInsets.all(5),
133+
child: Focus(
134+
focusNode: inner2,
135+
child: const SizedBox(width: 10, height: 10),
136+
),
137+
),
138+
),
139+
),
140+
],
141+
),
142+
),
143+
),
144+
);
145+
146+
expect(FocusManager.instance.primaryFocus, isNull);
147+
inner1.requestFocus();
148+
await tester.pump();
149+
expect(FocusManager.instance.primaryFocus, inner1);
150+
outer2.nextFocus();
151+
await tester.pump();
152+
expect(FocusManager.instance.primaryFocus, inner2);
153+
});
154+
155+
testWidgetsWithLeakTracking('focus traversal should work case 2', (WidgetTester tester) async {
156+
final FocusNode outer1 = FocusNode(debugLabel: 'outer1', skipTraversal: true);
157+
final FocusNode outer2 = FocusNode(debugLabel: 'outer2', skipTraversal: true);
158+
final FocusNode inner1 = FocusNode(debugLabel: 'inner1', );
159+
final FocusNode inner2 = FocusNode(debugLabel: 'inner2', );
160+
addTearDown(() {
161+
outer1.dispose();
162+
outer2.dispose();
163+
inner1.dispose();
164+
inner2.dispose();
165+
});
166+
167+
await tester.pumpWidget(
168+
Directionality(
169+
textDirection: TextDirection.ltr,
170+
child: FocusTraversalGroup(
171+
child: Row(
172+
children: <Widget>[
173+
FocusScope(
174+
child: Focus(
175+
focusNode: outer1,
176+
child: Focus(
177+
focusNode: inner1,
178+
child: const SizedBox(width: 10, height: 10),
179+
),
180+
),
181+
),
182+
FocusScope(
183+
child: Focus(
184+
focusNode: outer2,
185+
child: Focus(
186+
focusNode: inner2,
187+
child: const SizedBox(width: 10, height: 10),
188+
),
189+
),
190+
),
191+
],
192+
),
193+
),
194+
),
195+
);
196+
197+
expect(FocusManager.instance.primaryFocus, isNull);
198+
inner1.requestFocus();
199+
await tester.pump();
200+
expect(FocusManager.instance.primaryFocus, inner1);
201+
outer2.nextFocus();
202+
await tester.pump();
203+
expect(FocusManager.instance.primaryFocus, inner2);
204+
});
205+
99206
testWidgetsWithLeakTracking('Move focus to next node.', (WidgetTester tester) async {
100207
final GlobalKey key1 = GlobalKey(debugLabel: '1');
101208
final GlobalKey key2 = GlobalKey(debugLabel: '2');
@@ -716,8 +823,13 @@ void main() {
716823
final bool didFindNode = node1.nextFocus();
717824
await tester.pump();
718825
expect(didFindNode, isTrue);
719-
expect(node1.hasPrimaryFocus, isFalse);
720-
expect(node2.hasPrimaryFocus, isTrue);
826+
if (canRequestFocus) {
827+
expect(node1.hasPrimaryFocus, isTrue);
828+
expect(node2.hasPrimaryFocus, isFalse);
829+
} else {
830+
expect(node1.hasPrimaryFocus, isFalse);
831+
expect(node2.hasPrimaryFocus, isTrue);
832+
}
721833
}
722834
});
723835

0 commit comments

Comments
 (0)