Unverified Commit 4763be74 authored by chunhtai's avatar chunhtai Committed by GitHub

Can traverse if current focused node skips traversal (#130812)

Currently if the focus is on a focusnode that `skipTraversal = true`, the tab won't be able to traverse focus out of the node.

this pr fixes it
parent 9c8ee4fa
...@@ -352,7 +352,7 @@ abstract class FocusTraversalPolicy with Diagnosticable { ...@@ -352,7 +352,7 @@ abstract class FocusTraversalPolicy with Diagnosticable {
@protected @protected
Iterable<FocusNode> sortDescendants(Iterable<FocusNode> descendants, FocusNode currentNode); Iterable<FocusNode> sortDescendants(Iterable<FocusNode> descendants, FocusNode currentNode);
Map<FocusNode?, _FocusTraversalGroupInfo> _findGroups(FocusScopeNode scope, _FocusTraversalGroupNode? scopeGroupNode) { Map<FocusNode?, _FocusTraversalGroupInfo> _findGroups(FocusScopeNode scope, _FocusTraversalGroupNode? scopeGroupNode, FocusNode currentNode) {
final FocusTraversalPolicy defaultPolicy = scopeGroupNode?.policy ?? ReadingOrderTraversalPolicy(); final FocusTraversalPolicy defaultPolicy = scopeGroupNode?.policy ?? ReadingOrderTraversalPolicy();
final Map<FocusNode?, _FocusTraversalGroupInfo> groups = <FocusNode?, _FocusTraversalGroupInfo>{}; final Map<FocusNode?, _FocusTraversalGroupInfo> groups = <FocusNode?, _FocusTraversalGroupInfo>{};
for (final FocusNode node in scope.descendants) { for (final FocusNode node in scope.descendants) {
...@@ -374,7 +374,10 @@ abstract class FocusTraversalPolicy with Diagnosticable { ...@@ -374,7 +374,10 @@ abstract class FocusTraversalPolicy with Diagnosticable {
} }
// Skip non-focusable and non-traversable nodes in the same way that // Skip non-focusable and non-traversable nodes in the same way that
// FocusScopeNode.traversalDescendants would. // FocusScopeNode.traversalDescendants would.
if (node.canRequestFocus && !node.skipTraversal) { //
// Current focused node needs to be in the group so that the caller can
// find the next traversable node from the current focused node.
if (node == currentNode || (node.canRequestFocus && !node.skipTraversal)) {
groups[groupNode] ??= _FocusTraversalGroupInfo(groupNode, members: <FocusNode>[], defaultPolicy: defaultPolicy); groups[groupNode] ??= _FocusTraversalGroupInfo(groupNode, members: <FocusNode>[], defaultPolicy: defaultPolicy);
assert(!groups[groupNode]!.members.contains(node)); assert(!groups[groupNode]!.members.contains(node));
groups[groupNode]!.members.add(node); groups[groupNode]!.members.add(node);
...@@ -388,7 +391,7 @@ abstract class FocusTraversalPolicy with Diagnosticable { ...@@ -388,7 +391,7 @@ abstract class FocusTraversalPolicy with Diagnosticable {
List<FocusNode> _sortAllDescendants(FocusScopeNode scope, FocusNode currentNode) { List<FocusNode> _sortAllDescendants(FocusScopeNode scope, FocusNode currentNode) {
final _FocusTraversalGroupNode? scopeGroupNode = FocusTraversalGroup._getGroupNode(scope); final _FocusTraversalGroupNode? scopeGroupNode = FocusTraversalGroup._getGroupNode(scope);
// Build the sorting data structure, separating descendants into groups. // Build the sorting data structure, separating descendants into groups.
final Map<FocusNode?, _FocusTraversalGroupInfo> groups = _findGroups(scope, scopeGroupNode); final Map<FocusNode?, _FocusTraversalGroupInfo> groups = _findGroups(scope, scopeGroupNode, currentNode);
// Sort the member lists using the individual policy sorts. // Sort the member lists using the individual policy sorts.
for (final FocusNode? key in groups.keys) { for (final FocusNode? key in groups.keys) {
...@@ -397,6 +400,7 @@ abstract class FocusTraversalPolicy with Diagnosticable { ...@@ -397,6 +400,7 @@ abstract class FocusTraversalPolicy with Diagnosticable {
groups[key]!.members.addAll(sortedMembers); groups[key]!.members.addAll(sortedMembers);
} }
// Traverse the group tree, adding the children of members in the order they // Traverse the group tree, adding the children of members in the order they
// appear in the member lists. // appear in the member lists.
final List<FocusNode> sortedDescendants = <FocusNode>[]; final List<FocusNode> sortedDescendants = <FocusNode>[];
...@@ -421,17 +425,29 @@ abstract class FocusTraversalPolicy with Diagnosticable { ...@@ -421,17 +425,29 @@ abstract class FocusTraversalPolicy with Diagnosticable {
// They were left in above because they were needed to find their members // They were left in above because they were needed to find their members
// during sorting. // during sorting.
sortedDescendants.removeWhere((FocusNode node) { sortedDescendants.removeWhere((FocusNode node) {
return !node.canRequestFocus || node.skipTraversal; return node != currentNode && (!node.canRequestFocus || node.skipTraversal);
}); });
// Sanity check to make sure that the algorithm above doesn't diverge from // Sanity check to make sure that the algorithm above doesn't diverge from
// the one in FocusScopeNode.traversalDescendants in terms of which nodes it // the one in FocusScopeNode.traversalDescendants in terms of which nodes it
// finds. // finds.
assert((){
final Set<FocusNode> difference = sortedDescendants.toSet().difference(scope.traversalDescendants.toSet());
if (currentNode.skipTraversal) {
assert( assert(
sortedDescendants.length <= scope.traversalDescendants.length && sortedDescendants.toSet().difference(scope.traversalDescendants.toSet()).isEmpty, difference.length == 1 && difference.contains(currentNode),
'Sorted descendants contains different nodes than FocusScopeNode.traversalDescendants would. ' 'Sorted descendants contains different nodes than FocusScopeNode.traversalDescendants would. '
'These are the different nodes: ${sortedDescendants.toSet().difference(scope.traversalDescendants.toSet())}', 'These are the different nodes: ${difference.where((FocusNode node) => node != currentNode)}',
); );
return true;
}
assert(
difference.isEmpty,
'Sorted descendants contains different nodes than FocusScopeNode.traversalDescendants would. '
'These are the different nodes: $difference',
);
return true;
}());
return sortedDescendants; return sortedDescendants;
} }
...@@ -453,7 +469,7 @@ abstract class FocusTraversalPolicy with Diagnosticable { ...@@ -453,7 +469,7 @@ abstract class FocusTraversalPolicy with Diagnosticable {
bool _moveFocus(FocusNode currentNode, {required bool forward}) { bool _moveFocus(FocusNode currentNode, {required bool forward}) {
final FocusScopeNode nearestScope = currentNode.nearestScope!; final FocusScopeNode nearestScope = currentNode.nearestScope!;
invalidateScopeData(nearestScope); invalidateScopeData(nearestScope);
final FocusNode? focusedChild = nearestScope.focusedChild; FocusNode? focusedChild = nearestScope.focusedChild;
if (focusedChild == null) { if (focusedChild == null) {
final FocusNode? firstFocus = forward ? findFirstFocus(currentNode) : findLastFocus(currentNode); final FocusNode? firstFocus = forward ? findFirstFocus(currentNode) : findLastFocus(currentNode);
if (firstFocus != null) { if (firstFocus != null) {
...@@ -464,8 +480,11 @@ abstract class FocusTraversalPolicy with Diagnosticable { ...@@ -464,8 +480,11 @@ abstract class FocusTraversalPolicy with Diagnosticable {
return true; return true;
} }
} }
final List<FocusNode> sortedNodes = _sortAllDescendants(nearestScope, currentNode); focusedChild ??= nearestScope;
if (sortedNodes.isEmpty) { final List<FocusNode> sortedNodes = _sortAllDescendants(nearestScope, focusedChild);
assert(sortedNodes.contains(focusedChild));
if (sortedNodes.length < 2) {
// If there are no nodes to traverse to, like when descendantsAreTraversable // If there are no nodes to traverse to, like when descendantsAreTraversable
// is false or skipTraversal for all the nodes is true. // is false or skipTraversal for all the nodes is true.
return false; return false;
...@@ -473,7 +492,7 @@ abstract class FocusTraversalPolicy with Diagnosticable { ...@@ -473,7 +492,7 @@ abstract class FocusTraversalPolicy with Diagnosticable {
if (forward && focusedChild == sortedNodes.last) { if (forward && focusedChild == sortedNodes.last) {
switch (nearestScope.traversalEdgeBehavior) { switch (nearestScope.traversalEdgeBehavior) {
case TraversalEdgeBehavior.leaveFlutterView: case TraversalEdgeBehavior.leaveFlutterView:
focusedChild!.unfocus(); focusedChild.unfocus();
return false; return false;
case TraversalEdgeBehavior.closedLoop: case TraversalEdgeBehavior.closedLoop:
requestFocusCallback(sortedNodes.first, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd); requestFocusCallback(sortedNodes.first, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd);
...@@ -483,7 +502,7 @@ abstract class FocusTraversalPolicy with Diagnosticable { ...@@ -483,7 +502,7 @@ abstract class FocusTraversalPolicy with Diagnosticable {
if (!forward && focusedChild == sortedNodes.first) { if (!forward && focusedChild == sortedNodes.first) {
switch (nearestScope.traversalEdgeBehavior) { switch (nearestScope.traversalEdgeBehavior) {
case TraversalEdgeBehavior.leaveFlutterView: case TraversalEdgeBehavior.leaveFlutterView:
focusedChild!.unfocus(); focusedChild.unfocus();
return false; return false;
case TraversalEdgeBehavior.closedLoop: case TraversalEdgeBehavior.closedLoop:
requestFocusCallback(sortedNodes.last, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart); requestFocusCallback(sortedNodes.last, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart);
......
...@@ -812,7 +812,7 @@ void main() { ...@@ -812,7 +812,7 @@ void main() {
expect(focusNode1.hasPrimaryFocus, isTrue); expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse); expect(focusNode2.hasPrimaryFocus, isFalse);
expect(focusNode1.nextFocus(), isTrue); expect(focusNode1.nextFocus(), isFalse);
await tester.pump(); await tester.pump();
expect(focusNode1.hasPrimaryFocus, isTrue); expect(focusNode1.hasPrimaryFocus, isTrue);
......
...@@ -271,7 +271,7 @@ void main() { ...@@ -271,7 +271,7 @@ void main() {
expect(focusNode1.hasPrimaryFocus, isTrue); expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse); expect(focusNode2.hasPrimaryFocus, isFalse);
expect(focusNode1.nextFocus(), isTrue); expect(focusNode1.nextFocus(), isFalse);
await tester.pump(); await tester.pump();
expect(focusNode1.hasPrimaryFocus, isTrue); expect(focusNode1.hasPrimaryFocus, isTrue);
......
...@@ -414,7 +414,7 @@ void main() { ...@@ -414,7 +414,7 @@ void main() {
expect(focusNode1.hasPrimaryFocus, isTrue); expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse); expect(focusNode2.hasPrimaryFocus, isFalse);
expect(focusNode1.nextFocus(), isTrue); expect(focusNode1.nextFocus(), isFalse);
await tester.pump(); await tester.pump();
expect(focusNode1.hasPrimaryFocus, isTrue); expect(focusNode1.hasPrimaryFocus, isTrue);
......
...@@ -6690,7 +6690,7 @@ void main() { ...@@ -6690,7 +6690,7 @@ void main() {
expect(focusNode1.hasPrimaryFocus, isTrue); expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse); expect(focusNode2.hasPrimaryFocus, isFalse);
expect(focusNode1.nextFocus(), isTrue); expect(focusNode1.nextFocus(), isFalse);
await tester.pump(); await tester.pump();
expect(focusNode1.hasPrimaryFocus, isTrue); expect(focusNode1.hasPrimaryFocus, isTrue);
......
...@@ -908,6 +908,7 @@ void main() { ...@@ -908,6 +908,7 @@ void main() {
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
home: FocusableActionDetector( home: FocusableActionDetector(
focusNode: FocusNode(skipTraversal: true),
child: Column( child: Column(
children: <Widget>[ children: <Widget>[
ElevatedButton( ElevatedButton(
...@@ -938,6 +939,7 @@ void main() { ...@@ -938,6 +939,7 @@ void main() {
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
home: FocusableActionDetector( home: FocusableActionDetector(
focusNode: FocusNode(skipTraversal: true),
descendantsAreTraversable: false, descendantsAreTraversable: false,
child: Column( child: Column(
children: <Widget>[ children: <Widget>[
......
...@@ -2090,6 +2090,61 @@ void main() { ...@@ -2090,6 +2090,61 @@ void main() {
expect(Focus.of(upperLeftKey.currentContext!).hasPrimaryFocus, isTrue); expect(Focus.of(upperLeftKey.currentContext!).hasPrimaryFocus, isTrue);
}, skip: isBrowser, variant: KeySimulatorTransitModeVariant.all()); // https://github.com/flutter/flutter/issues/35347 }, skip: isBrowser, variant: KeySimulatorTransitModeVariant.all()); // https://github.com/flutter/flutter/issues/35347
testWidgets('Focus traversal actions works when current focus skip traversal', (WidgetTester tester) async {
final GlobalKey key1 = GlobalKey(debugLabel: 'key1');
final GlobalKey key2 = GlobalKey(debugLabel: 'key2');
final GlobalKey key3 = GlobalKey(debugLabel: 'key3');
await tester.pumpWidget(
WidgetsApp(
color: const Color(0xFFFFFFFF),
onGenerateRoute: (RouteSettings settings) {
return TestRoute(
child: Directionality(
textDirection: TextDirection.ltr,
child: FocusScope(
debugLabel: 'scope',
child: Column(
children: <Widget>[
Row(
children: <Widget>[
Focus(
autofocus: true,
skipTraversal: true,
debugLabel: '1',
child: SizedBox(width: 100, height: 100, key: key1),
),
Focus(
debugLabel: '2',
child: SizedBox(width: 100, height: 100, key: key2),
),
Focus(
debugLabel: '3',
child: SizedBox(width: 100, height: 100, key: key3),
),
],
),
],
),
),
),
);
},
),
);
expect(Focus.of(key1.currentContext!).hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.tab);
expect(Focus.of(key2.currentContext!).hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.tab);
expect(Focus.of(key3.currentContext!).hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.tab);
// Skips key 1 because it skips traversal.
expect(Focus.of(key2.currentContext!).hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.tab);
expect(Focus.of(key3.currentContext!).hasPrimaryFocus, isTrue);
}, skip: isBrowser, variant: KeySimulatorTransitModeVariant.all()); // https://github.com/flutter/flutter/issues/35347
testWidgets('Focus traversal inside a vertical scrollable scrolls to stay visible.', (WidgetTester tester) async { testWidgets('Focus traversal inside a vertical scrollable scrolls to stay visible.', (WidgetTester tester) async {
final List<int> items = List<int>.generate(11, (int index) => index).toList(); final List<int> items = List<int>.generate(11, (int index) => index).toList();
final List<FocusNode> nodes = List<FocusNode>.generate(11, (int index) => FocusNode(debugLabel: 'Item ${index + 1}')).toList(); final List<FocusNode> nodes = List<FocusNode>.generate(11, (int index) => FocusNode(debugLabel: 'Item ${index + 1}')).toList();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment