Reland "Fixes ability to call nextFocus() on a node to focus its desc…

…… (#136898) â�¦endant" (#136894)" This reverts commit c2bd2c1. fixes flutter/flutter#134854 This is a straight reland, the internal test is testing a wrong behave. https://critique.corp.google.com/cl/575028981
moneybird · Oct 23, 2023 · e869144 · e869144
1 parent af129b6
commit e869144
Show file tree

Hide file tree

Showing 2 changed files with 122 additions and 6 deletions.
diff --git a/packages/flutter/lib/src/widgets/focus_traversal.dart b/packages/flutter/lib/src/widgets/focus_traversal.dart
@@ -306,7 +306,7 @@ abstract class FocusTraversalPolicy with Diagnosticable {
     final FocusScopeNode scope = currentNode.nearestScope!;
     FocusNode? candidate = scope.focusedChild;
     if (ignoreCurrentFocus || candidate == null && scope.descendants.isNotEmpty) {
-      final Iterable<FocusNode> sorted = _sortAllDescendants(scope, currentNode);
+      final Iterable<FocusNode> sorted = _sortAllDescendants(scope, currentNode).where((FocusNode node) => _canRequestTraversalFocus(node));
       if (sorted.isEmpty) {
         candidate = null;
       } else {
@@ -405,13 +405,17 @@ abstract class FocusTraversalPolicy with Diagnosticable {
   @protected
   Iterable<FocusNode> sortDescendants(Iterable<FocusNode> descendants, FocusNode currentNode);
 
+  static bool _canRequestTraversalFocus(FocusNode node) {
+    return node.canRequestFocus && !node.skipTraversal;
+  }
+
   static Iterable<FocusNode> _getDescendantsWithoutExpandingScope(FocusNode node) {
     final List<FocusNode> result = <FocusNode>[];
     for (final FocusNode child in node.children) {
+      result.add(child);
       if (child is! FocusScopeNode) {
         result.addAll(_getDescendantsWithoutExpandingScope(child));
       }
-      result.add(child);
     }
     return result;
   }
@@ -488,15 +492,15 @@ abstract class FocusTraversalPolicy with Diagnosticable {
     // They were left in above because they were needed to find their members
     // during sorting.
     sortedDescendants.removeWhere((FocusNode node) {
-      return node != currentNode && (!node.canRequestFocus || node.skipTraversal);
+      return node != currentNode && !_canRequestTraversalFocus(node);
     });
 
     // Sanity check to make sure that the algorithm above doesn't diverge from
     // the one in FocusScopeNode.traversalDescendants in terms of which nodes it
     // finds.
     assert((){
       final Set<FocusNode> difference = sortedDescendants.toSet().difference(scope.traversalDescendants.toSet());
-      if (currentNode.skipTraversal || !currentNode.canRequestFocus) {
+      if (!_canRequestTraversalFocus(currentNode)) {
         // The scope.traversalDescendants will not contain currentNode if it
         // skips traversal or not focusable.
         assert(

diff --git a/packages/flutter/test/widgets/focus_traversal_test.dart b/packages/flutter/test/widgets/focus_traversal_test.dart
@@ -96,6 +96,113 @@ void main() {
       expect(scope.hasFocus, isTrue);
     });
 
+    testWidgetsWithLeakTracking('focus traversal should work case 1', (WidgetTester tester) async {
+      final FocusNode outer1 = FocusNode(debugLabel: 'outer1', skipTraversal: true);
+      final FocusNode outer2 = FocusNode(debugLabel: 'outer2', skipTraversal: true);
+      final FocusNode inner1 = FocusNode(debugLabel: 'inner1', );
+      final FocusNode inner2 = FocusNode(debugLabel: 'inner2', );
+      addTearDown(() {
+        outer1.dispose();
+        outer2.dispose();
+        inner1.dispose();
+        inner2.dispose();
+      });
+
+      await tester.pumpWidget(
+        Directionality(
+          textDirection: TextDirection.ltr,
+          child: FocusTraversalGroup(
+            child: Row(
+              children: <Widget>[
+                FocusScope(
+                  child: Focus(
+                    focusNode: outer1,
+                    child: Focus(
+                      focusNode: inner1,
+                      child: const SizedBox(width: 10, height: 10),
+                    ),
+                  ),
+                ),
+                FocusScope(
+                  child: Focus(
+                    focusNode: outer2,
+                    // Add a padding to ensure both Focus widgets have different
+                    // sizes.
+                    child: Padding(
+                      padding: const EdgeInsets.all(5),
+                      child: Focus(
+                        focusNode: inner2,
+                        child: const SizedBox(width: 10, height: 10),
+                      ),
+                    ),
+                  ),
+                ),
+              ],
+            ),
+          ),
+        ),
+      );
+
+      expect(FocusManager.instance.primaryFocus, isNull);
+      inner1.requestFocus();
+      await tester.pump();
+      expect(FocusManager.instance.primaryFocus, inner1);
+      outer2.nextFocus();
+      await tester.pump();
+      expect(FocusManager.instance.primaryFocus, inner2);
+    });
+
+    testWidgetsWithLeakTracking('focus traversal should work case 2', (WidgetTester tester) async {
+      final FocusNode outer1 = FocusNode(debugLabel: 'outer1', skipTraversal: true);
+      final FocusNode outer2 = FocusNode(debugLabel: 'outer2', skipTraversal: true);
+      final FocusNode inner1 = FocusNode(debugLabel: 'inner1', );
+      final FocusNode inner2 = FocusNode(debugLabel: 'inner2', );
+      addTearDown(() {
+        outer1.dispose();
+        outer2.dispose();
+        inner1.dispose();
+        inner2.dispose();
+      });
+
+      await tester.pumpWidget(
+        Directionality(
+          textDirection: TextDirection.ltr,
+          child: FocusTraversalGroup(
+            child: Row(
+              children: <Widget>[
+                FocusScope(
+                  child: Focus(
+                    focusNode: outer1,
+                    child: Focus(
+                      focusNode: inner1,
+                      child: const SizedBox(width: 10, height: 10),
+                    ),
+                  ),
+                ),
+                FocusScope(
+                  child: Focus(
+                    focusNode: outer2,
+                    child: Focus(
+                      focusNode: inner2,
+                      child: const SizedBox(width: 10, height: 10),
+                    ),
+                  ),
+                ),
+              ],
+            ),
+          ),
+        ),
+      );
+
+      expect(FocusManager.instance.primaryFocus, isNull);
+      inner1.requestFocus();
+      await tester.pump();
+      expect(FocusManager.instance.primaryFocus, inner1);
+      outer2.nextFocus();
+      await tester.pump();
+      expect(FocusManager.instance.primaryFocus, inner2);
+    });
+
     testWidgetsWithLeakTracking('Move focus to next node.', (WidgetTester tester) async {
       final GlobalKey key1 = GlobalKey(debugLabel: '1');
       final GlobalKey key2 = GlobalKey(debugLabel: '2');
@@ -716,8 +823,13 @@ void main() {
         final bool didFindNode = node1.nextFocus();
         await tester.pump();
         expect(didFindNode, isTrue);
-        expect(node1.hasPrimaryFocus, isFalse);
-        expect(node2.hasPrimaryFocus, isTrue);
+        if (canRequestFocus) {
+          expect(node1.hasPrimaryFocus, isTrue);
+          expect(node2.hasPrimaryFocus, isFalse);
+        } else {
+          expect(node1.hasPrimaryFocus, isFalse);
+          expect(node2.hasPrimaryFocus, isTrue);
+        }
       }
     });