Fix first focus determination. (#33279)
Replacing the algorithm for finding the first focusable item in the focus tree. Somehow it was a kind of gibberish before, and really didn't work or make sense.
diff --git a/packages/flutter/lib/src/widgets/focus_traversal.dart b/packages/flutter/lib/src/widgets/focus_traversal.dart
index bca61f0..7fb6c72 100644
--- a/packages/flutter/lib/src/widgets/focus_traversal.dart
+++ b/packages/flutter/lib/src/widgets/focus_traversal.dart
@@ -6,6 +6,7 @@
import 'package:flutter/painting.dart';
import 'basic.dart';
+import 'binding.dart';
import 'focus_manager.dart';
import 'framework.dart';
@@ -498,16 +499,19 @@
// Moves the focus to the next or previous node, depending on whether forward
// is true or not.
- bool _move(FocusNode node, {@required bool forward}) {
- if (node == null) {
+ bool _move(FocusNode currentNode, {@required bool forward}) {
+ if (currentNode == null) {
return false;
}
- final FocusScopeNode nearestScope = node.nearestScope;
+ final FocusScopeNode nearestScope = currentNode.nearestScope;
invalidateScopeData(nearestScope);
final FocusNode focusedChild = nearestScope.focusedChild;
if (focusedChild == null) {
- findFirstFocus(node).requestFocus();
- return true;
+ final FocusNode firstFocus = findFirstFocus(currentNode);
+ if (firstFocus != null) {
+ firstFocus.requestFocus();
+ return true;
+ }
}
FocusNode previousNode;
FocusNode firstNode;
@@ -593,40 +597,22 @@
@override
FocusNode findFirstFocus(FocusNode currentNode) {
assert(currentNode != null);
- FocusScopeNode scope = currentNode.nearestScope;
- // Start with the candidate focus as the focused child of this scope, if
- // there is one. Otherwise start with this node itself. Keep going down
- // through scopes until an ultimately focusable item is found, a scope
- // doesn't have a focusedChild, or a non-scope is encountered.
+ final FocusScopeNode scope = currentNode.nearestScope;
FocusNode candidate = scope.focusedChild;
- while (candidate == null) {
- if (candidate.nearestScope.traversalChildren.isNotEmpty) {
- candidate = _sortByGeometry(scope).first;
- }
- if (candidate is FocusScopeNode) {
- scope = candidate;
- candidate = scope.focusedChild;
- continue;
- }
+ if (candidate == null && scope.traversalChildren.isNotEmpty) {
+ candidate = _sortByGeometry(scope).first;
}
- if (candidate == null) {
- if (scope.traversalChildren.isNotEmpty) {
- candidate = _sortByGeometry(scope).first;
- } else {
- candidate = currentNode;
- }
- }
- while (candidate is FocusScopeNode && candidate.focusedChild != null) {
- final FocusScopeNode candidateScope = candidate;
- candidate = candidateScope.focusedChild;
- }
+ // If we still didn't find any candidate, use the current node as a
+ // fallback.
+ candidate ??= currentNode;
+ candidate ??= WidgetsBinding.instance.focusManager.rootScope;
return candidate;
}
// Sorts the list of nodes based on their geometry into the desired reading
// order based on the directionality of the context for each node.
- Iterable<FocusNode> _sortByGeometry(FocusNode scope) {
+ Iterable<FocusNode> _sortByGeometry(FocusScopeNode scope) {
final Iterable<FocusNode> nodes = scope.traversalDescendants;
if (nodes.length <= 1) {
return nodes;
@@ -691,8 +677,11 @@
invalidateScopeData(nearestScope);
final FocusNode focusedChild = nearestScope.focusedChild;
if (focusedChild == null) {
- findFirstFocus(currentNode).requestFocus();
- return true;
+ final FocusNode firstFocus = findFirstFocus(currentNode);
+ if (firstFocus != null) {
+ firstFocus.requestFocus();
+ return true;
+ }
}
final List<FocusNode> sortedNodes = _sortByGeometry(nearestScope).toList();
if (forward && focusedChild == sortedNodes.last) {
diff --git a/packages/flutter/test/widgets/focus_traversal_test.dart b/packages/flutter/test/widgets/focus_traversal_test.dart
index 1838456..693ffb1 100644
--- a/packages/flutter/test/widgets/focus_traversal_test.dart
+++ b/packages/flutter/test/widgets/focus_traversal_test.dart
@@ -8,6 +8,44 @@
void main() {
group(WidgetOrderFocusTraversalPolicy, () {
+ testWidgets('Find the initial focus if there is none yet.', (WidgetTester tester) async {
+ final GlobalKey key1 = GlobalKey(debugLabel: '1');
+ final GlobalKey key2 = GlobalKey(debugLabel: '2');
+ final GlobalKey key3 = GlobalKey(debugLabel: '3');
+ final GlobalKey key4 = GlobalKey(debugLabel: '4');
+ final GlobalKey key5 = GlobalKey(debugLabel: '5');
+ await tester.pumpWidget(DefaultFocusTraversal(
+ policy: WidgetOrderFocusTraversalPolicy(),
+ child: FocusScope(
+ key: key1,
+ child: Column(
+ children: <Widget>[
+ Focus(
+ key: key2,
+ child: Container(key: key3, width: 100, height: 100),
+ ),
+ Focus(
+ key: key4,
+ child: Container(key: key5, width: 100, height: 100),
+ ),
+ ],
+ ),
+ ),
+ ));
+
+ final Element firstChild = tester.element(find.byKey(key3));
+ final Element secondChild = tester.element(find.byKey(key5));
+ final FocusNode firstFocusNode = Focus.of(firstChild);
+ final FocusNode secondFocusNode = Focus.of(secondChild);
+ final FocusNode scope = Focus.of(firstChild).enclosingScope;
+ secondFocusNode.nextFocus();
+
+ await tester.pump();
+
+ expect(firstFocusNode.hasFocus, isTrue);
+ expect(secondFocusNode.hasFocus, isFalse);
+ expect(scope.hasFocus, isTrue);
+ });
testWidgets('Move focus to next node.', (WidgetTester tester) async {
final GlobalKey key1 = GlobalKey(debugLabel: '1');
final GlobalKey key2 = GlobalKey(debugLabel: '2');
@@ -199,6 +237,44 @@
});
});
group(ReadingOrderTraversalPolicy, () {
+ testWidgets('Find the initial focus if there is none yet.', (WidgetTester tester) async {
+ final GlobalKey key1 = GlobalKey(debugLabel: '1');
+ final GlobalKey key2 = GlobalKey(debugLabel: '2');
+ final GlobalKey key3 = GlobalKey(debugLabel: '3');
+ final GlobalKey key4 = GlobalKey(debugLabel: '4');
+ final GlobalKey key5 = GlobalKey(debugLabel: '5');
+ await tester.pumpWidget(DefaultFocusTraversal(
+ policy: ReadingOrderTraversalPolicy(),
+ child: FocusScope(
+ key: key1,
+ child: Column(
+ children: <Widget>[
+ Focus(
+ key: key2,
+ child: Container(key: key3, width: 100, height: 100),
+ ),
+ Focus(
+ key: key4,
+ child: Container(key: key5, width: 100, height: 100),
+ ),
+ ],
+ ),
+ ),
+ ));
+
+ final Element firstChild = tester.element(find.byKey(key3));
+ final Element secondChild = tester.element(find.byKey(key5));
+ final FocusNode firstFocusNode = Focus.of(firstChild);
+ final FocusNode secondFocusNode = Focus.of(secondChild);
+ final FocusNode scope = Focus.of(firstChild).enclosingScope;
+ secondFocusNode.nextFocus();
+
+ await tester.pump();
+
+ expect(firstFocusNode.hasFocus, isTrue);
+ expect(secondFocusNode.hasFocus, isFalse);
+ expect(scope.hasFocus, isTrue);
+ });
testWidgets('Move reading focus to next node.', (WidgetTester tester) async {
final GlobalKey key1 = GlobalKey(debugLabel: '1');
final GlobalKey key2 = GlobalKey(debugLabel: '2');
