Skip to content

Commit

Permalink
Fix first focus determination. (flutter#33279)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@gspencergoog
gspencergoog authored May 28, 2019
1 parent 841e1cf commit 7c811b6
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 33 deletions.
55 changes: 22 additions & 33 deletions packages/flutter/lib/src/widgets/focus_traversal.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:flutter/foundation.dart';
import 'package:flutter/painting.dart';

import 'basic.dart';
import 'binding.dart';
import 'focus_manager.dart';
import 'framework.dart';

Expand Down Expand Up @@ -498,16 +499,19 @@ class WidgetOrderFocusTraversalPolicy extends FocusTraversalPolicy with Directio

// 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;
Expand Down Expand Up @@ -593,40 +597,22 @@ class ReadingOrderTraversalPolicy extends FocusTraversalPolicy with DirectionalF
@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;
Expand Down Expand Up @@ -691,8 +677,11 @@ class ReadingOrderTraversalPolicy extends FocusTraversalPolicy with DirectionalF
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) {
Expand Down
76 changes: 76 additions & 0 deletions packages/flutter/test/widgets/focus_traversal_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,44 @@ import 'package:flutter/widgets.dart';

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');
Expand Down Expand Up @@ -199,6 +237,44 @@ void main() {
});
});
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');
Expand Down

0 comments on commit 7c811b6

Please sign in to comment.