Revert "Adds a parent scope TraversalEdgeBehavior and fixes modal rou… (#134565)
Reverts
https://github.com/flutter/flutter/commit/0f3bd90d9b108904cc0390e899c2b3c6ee2e76e0
to unblock the flutter roll.
Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com>
diff --git a/packages/flutter/lib/src/widgets/app.dart b/packages/flutter/lib/src/widgets/app.dart
index 7a52463..d8d649c 100644
--- a/packages/flutter/lib/src/widgets/app.dart
+++ b/packages/flutter/lib/src/widgets/app.dart
@@ -1685,7 +1685,6 @@
},
onUnknownRoute: _onUnknownRoute,
observers: widget.navigatorObservers!,
- routeTraversalEdgeBehavior: kIsWeb ? TraversalEdgeBehavior.leaveFlutterView : TraversalEdgeBehavior.parentScope,
reportsRouteUpdateToEngine: true,
),
);
diff --git a/packages/flutter/lib/src/widgets/focus_traversal.dart b/packages/flutter/lib/src/widgets/focus_traversal.dart
index f67080c..203e132 100644
--- a/packages/flutter/lib/src/widgets/focus_traversal.dart
+++ b/packages/flutter/lib/src/widgets/focus_traversal.dart
@@ -120,16 +120,6 @@
/// address bar, escape an `iframe`, or focus on HTML elements other than
/// those managed by Flutter.
leaveFlutterView,
-
- /// Allows focus to traverse up to parent scope.
- ///
- /// When reaching the edge of the current scope, requesting the next focus
- /// will look up to the parent scope of the current scope and focus the focus
- /// node next to the current scope.
- ///
- /// If there is no parent scope above the current scope, fallback to
- /// [closedLoop] behavior.
- parentScope,
}
/// Determines how focusable widgets are traversed within a [FocusTraversalGroup].
@@ -196,60 +186,6 @@
);
}
- /// Request focus on a focus node as a result of a tab traversal.
- ///
- /// If the `node` is a [FocusScopeNode], this method will recursively find
- /// the next focus from its descendants until it find a regular [FocusNode].
- ///
- /// Returns true if this method focused a new focus node.
- bool _requestTabTraversalFocus(
- FocusNode node, {
- ScrollPositionAlignmentPolicy? alignmentPolicy,
- double? alignment,
- Duration? duration,
- Curve? curve,
- required bool forward,
- }) {
- if (node is FocusScopeNode) {
- if (node.focusedChild != null) {
- // Can't stop here as the `focusedChild` may be a focus scope node
- // without a first focus. The first focus will be picked in the
- // next iteration.
- return _requestTabTraversalFocus(
- node.focusedChild!,
- alignmentPolicy: alignmentPolicy,
- alignment: alignment,
- duration: duration,
- curve: curve,
- forward: forward,
- );
- }
- final List<FocusNode> sortedChildren = _sortAllDescendants(node, node);
- if (sortedChildren.isNotEmpty) {
- _requestTabTraversalFocus(
- forward ? sortedChildren.first : sortedChildren.last,
- alignmentPolicy: alignmentPolicy,
- alignment: alignment,
- duration: duration,
- curve: curve,
- forward: forward,
- );
- // Regardless if _requestTabTraversalFocus return true or false, a first
- // focus has been picked.
- return true;
- }
- }
- final bool nodeHadPrimaryFocus = node.hasPrimaryFocus;
- requestFocusCallback(
- node,
- alignmentPolicy: alignmentPolicy,
- alignment: alignment,
- duration: duration,
- curve: curve,
- );
- return !nodeHadPrimaryFocus;
- }
-
/// Returns the node that should receive focus if focus is traversing
/// forwards, and there is no current focus.
///
@@ -416,21 +352,10 @@
@protected
Iterable<FocusNode> sortDescendants(Iterable<FocusNode> descendants, FocusNode currentNode);
- static Iterable<FocusNode> _getDescendantsWithoutExpandingScope(FocusNode node) {
- final List<FocusNode> result = <FocusNode>[];
- for (final FocusNode child in node.children) {
- if (child is! FocusScopeNode) {
- result.addAll(_getDescendantsWithoutExpandingScope(child));
- }
- result.add(child);
- }
- return result;
- }
-
- static Map<FocusNode?, _FocusTraversalGroupInfo> _findGroups(FocusScopeNode scope, _FocusTraversalGroupNode? scopeGroupNode, FocusNode currentNode) {
+ Map<FocusNode?, _FocusTraversalGroupInfo> _findGroups(FocusScopeNode scope, _FocusTraversalGroupNode? scopeGroupNode, FocusNode currentNode) {
final FocusTraversalPolicy defaultPolicy = scopeGroupNode?.policy ?? ReadingOrderTraversalPolicy();
final Map<FocusNode?, _FocusTraversalGroupInfo> groups = <FocusNode?, _FocusTraversalGroupInfo>{};
- for (final FocusNode node in _getDescendantsWithoutExpandingScope(scope)) {
+ for (final FocusNode node in scope.descendants) {
final _FocusTraversalGroupNode? groupNode = FocusTraversalGroup._getGroupNode(node);
// Group nodes need to be added to their parent's node, or to the "null"
// node if no parent is found. This creates the hierarchy of group nodes
@@ -463,7 +388,7 @@
// Sort all descendants, taking into account the FocusTraversalGroup
// that they are each in, and filtering out non-traversable/focusable nodes.
- static List<FocusNode> _sortAllDescendants(FocusScopeNode scope, FocusNode currentNode) {
+ List<FocusNode> _sortAllDescendants(FocusScopeNode scope, FocusNode currentNode) {
final _FocusTraversalGroupNode? scopeGroupNode = FocusTraversalGroup._getGroupNode(scope);
// Build the sorting data structure, separating descendants into groups.
final Map<FocusNode?, _FocusTraversalGroupInfo> groups = _findGroups(scope, scopeGroupNode, currentNode);
@@ -548,42 +473,30 @@
if (focusedChild == null) {
final FocusNode? firstFocus = forward ? findFirstFocus(currentNode) : findLastFocus(currentNode);
if (firstFocus != null) {
- return _requestTabTraversalFocus(
+ requestFocusCallback(
firstFocus,
alignmentPolicy: forward ? ScrollPositionAlignmentPolicy.keepVisibleAtEnd : ScrollPositionAlignmentPolicy.keepVisibleAtStart,
- forward: forward,
);
+ return true;
}
}
focusedChild ??= nearestScope;
final List<FocusNode> sortedNodes = _sortAllDescendants(nearestScope, focusedChild);
- assert(sortedNodes.contains(focusedChild));
+ assert(sortedNodes.contains(focusedChild));
+ if (sortedNodes.length < 2) {
+ // If there are no nodes to traverse to, like when descendantsAreTraversable
+ // is false or skipTraversal for all the nodes is true.
+ return false;
+ }
if (forward && focusedChild == sortedNodes.last) {
switch (nearestScope.traversalEdgeBehavior) {
case TraversalEdgeBehavior.leaveFlutterView:
focusedChild.unfocus();
return false;
- case TraversalEdgeBehavior.parentScope:
- final FocusScopeNode? parentScope = nearestScope.enclosingScope;
- if (parentScope != null && parentScope != FocusManager.instance.rootScope) {
- focusedChild.unfocus();
- parentScope.nextFocus();
- // Verify the focus really has changed.
- return focusedChild.enclosingScope?.focusedChild != focusedChild;
- }
- // No valid parent scope. Fallback to closed loop behavior.
- return _requestTabTraversalFocus(
- sortedNodes.first,
- alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd,
- forward: forward,
- );
case TraversalEdgeBehavior.closedLoop:
- return _requestTabTraversalFocus(
- sortedNodes.first,
- alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd,
- forward: forward,
- );
+ requestFocusCallback(sortedNodes.first, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd);
+ return true;
}
}
if (!forward && focusedChild == sortedNodes.first) {
@@ -591,26 +504,9 @@
case TraversalEdgeBehavior.leaveFlutterView:
focusedChild.unfocus();
return false;
- case TraversalEdgeBehavior.parentScope:
- final FocusScopeNode? parentScope = nearestScope.enclosingScope;
- if (parentScope != null && parentScope != FocusManager.instance.rootScope) {
- focusedChild.unfocus();
- parentScope.previousFocus();
- // Verify the focus really has changed.
- return focusedChild.enclosingScope?.focusedChild != focusedChild;
- }
- // No valid parent scope. Fallback to closed loop behavior.
- return _requestTabTraversalFocus(
- sortedNodes.last,
- alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart,
- forward: forward,
- );
case TraversalEdgeBehavior.closedLoop:
- return _requestTabTraversalFocus(
- sortedNodes.last,
- alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart,
- forward: forward,
- );
+ requestFocusCallback(sortedNodes.last, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart);
+ return true;
}
}
@@ -618,11 +514,11 @@
FocusNode? previousNode;
for (final FocusNode node in maybeFlipped) {
if (previousNode == focusedChild) {
- return _requestTabTraversalFocus(
+ requestFocusCallback(
node,
alignmentPolicy: forward ? ScrollPositionAlignmentPolicy.keepVisibleAtEnd : ScrollPositionAlignmentPolicy.keepVisibleAtStart,
- forward: forward,
);
+ return true;
}
previousNode = node;
}
diff --git a/packages/flutter/lib/src/widgets/navigator.dart b/packages/flutter/lib/src/widgets/navigator.dart
index bc57d8a..ccf3fbf 100644
--- a/packages/flutter/lib/src/widgets/navigator.dart
+++ b/packages/flutter/lib/src/widgets/navigator.dart
@@ -1146,7 +1146,9 @@
/// The default value of [Navigator.routeTraversalEdgeBehavior].
///
/// {@macro flutter.widgets.navigator.routeTraversalEdgeBehavior}
-const TraversalEdgeBehavior kDefaultRouteTraversalEdgeBehavior = TraversalEdgeBehavior.parentScope;
+const TraversalEdgeBehavior kDefaultRouteTraversalEdgeBehavior = kIsWeb
+ ? TraversalEdgeBehavior.leaveFlutterView
+ : TraversalEdgeBehavior.closedLoop;
/// A widget that manages a set of child widgets with a stack discipline.
///
diff --git a/packages/flutter/lib/src/widgets/routes.dart b/packages/flutter/lib/src/widgets/routes.dart
index fc954fa..b51cb4f 100644
--- a/packages/flutter/lib/src/widgets/routes.dart
+++ b/packages/flutter/lib/src/widgets/routes.dart
@@ -834,9 +834,7 @@
late Listenable _listenable;
/// The node this scope will use for its root [FocusScope] widget.
- final FocusScopeNode focusScopeNode = FocusScopeNode(
- debugLabel: '$_ModalScopeState Focus Scope',
- );
+ final FocusScopeNode focusScopeNode = FocusScopeNode(debugLabel: '$_ModalScopeState Focus Scope');
final ScrollController primaryScrollController = ScrollController();
@override
@@ -938,8 +936,6 @@
controller: primaryScrollController,
child: FocusScope(
node: focusScopeNode, // immutable
- // Only top most route can participate in focus traversal.
- skipTraversal: !widget.route.isCurrent,
child: RepaintBoundary(
child: AnimatedBuilder(
animation: _listenable, // immutable
@@ -1709,25 +1705,10 @@
}
@override
- void didChangeNext(Route<dynamic>? nextRoute) {
- super.didChangeNext(nextRoute);
- changedInternalState();
- }
-
- @override
- void didPopNext(Route<dynamic> nextRoute) {
- super.didPopNext(nextRoute);
- changedInternalState();
- }
-
- @override
void changedInternalState() {
super.changedInternalState();
- // No need to mark dirty if this method is called during build phase.
- if (SchedulerBinding.instance.schedulerPhase != SchedulerPhase.persistentCallbacks) {
- setState(() { /* internal state already changed */ });
- _modalBarrier.markNeedsBuild();
- }
+ setState(() { /* internal state already changed */ });
+ _modalBarrier.markNeedsBuild();
_modalScope.maintainState = maintainState;
}
diff --git a/packages/flutter/test/material/icon_button_test.dart b/packages/flutter/test/material/icon_button_test.dart
index 02c6ad0..fce7855 100644
--- a/packages/flutter/test/material/icon_button_test.dart
+++ b/packages/flutter/test/material/icon_button_test.dart
@@ -4,7 +4,6 @@
import 'dart:ui';
-import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart';
@@ -793,10 +792,6 @@
testWidgetsWithLeakTracking("Disabled IconButton can't be traversed to when disabled.", (WidgetTester tester) async {
final FocusNode focusNode1 = FocusNode(debugLabel: 'IconButton 1');
final FocusNode focusNode2 = FocusNode(debugLabel: 'IconButton 2');
- addTearDown(() {
- focusNode1.dispose();
- focusNode2.dispose();
- });
await tester.pumpWidget(
wrap(
@@ -826,8 +821,11 @@
expect(focusNode1.nextFocus(), isFalse);
await tester.pump();
- expect(focusNode1.hasPrimaryFocus, !kIsWeb);
+ expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse);
+
+ focusNode1.dispose();
+ focusNode2.dispose();
});
group('feedback', () {
diff --git a/packages/flutter/test/widgets/actions_test.dart b/packages/flutter/test/widgets/actions_test.dart
index 6a24630..b59f17f 100644
--- a/packages/flutter/test/widgets/actions_test.dart
+++ b/packages/flutter/test/widgets/actions_test.dart
@@ -965,7 +965,7 @@
expect(buttonNode2.hasFocus, isFalse);
primaryFocus!.nextFocus();
await tester.pump();
- expect(buttonNode1.hasFocus, isFalse);
+ expect(buttonNode1.hasFocus, isTrue);
expect(buttonNode2.hasFocus, isFalse);
},
);
diff --git a/packages/flutter/test/widgets/focus_traversal_test.dart b/packages/flutter/test/widgets/focus_traversal_test.dart
index 04cf497..ff0b8c9 100644
--- a/packages/flutter/test/widgets/focus_traversal_test.dart
+++ b/packages/flutter/test/widgets/focus_traversal_test.dart
@@ -430,92 +430,6 @@
});
- testWidgets('Nested navigator does not trap focus', (WidgetTester tester) async {
- final FocusNode node1 = FocusNode();
- final FocusNode node2 = FocusNode();
- final FocusNode node3 = FocusNode();
- await tester.pumpWidget(
- Directionality(
- textDirection: TextDirection.ltr,
- child: FocusTraversalGroup(
- policy: ReadingOrderTraversalPolicy(),
- child: FocusScope(
- child: Column(
- children: <Widget>[
- Focus(
- focusNode: node1,
- child: const SizedBox(width: 100, height: 100),
- ),
- SizedBox(
- width: 100,
- height: 100,
- child: Navigator(
- pages: <Page<void>>[
- MaterialPage<void>(
- child: Focus(
- focusNode: node2,
- child: const SizedBox(width: 100, height: 100),
- ),
- ),
- ],
- onPopPage: (_, __) => false,
- ),
- ),
- Focus(
- focusNode: node3,
- child: const SizedBox(width: 100, height: 100),
- ),
- ],
- ),
- ),
- ),
- ),
- );
-
- node1.requestFocus();
- await tester.pump();
-
- expect(node1.hasFocus, isTrue);
- expect(node2.hasFocus, isFalse);
- expect(node3.hasFocus, isFalse);
-
- node1.nextFocus();
- await tester.pump();
- expect(node1.hasFocus, isFalse);
- expect(node2.hasFocus, isTrue);
- expect(node3.hasFocus, isFalse);
-
- node2.nextFocus();
- await tester.pump();
- expect(node1.hasFocus, isFalse);
- expect(node2.hasFocus, isFalse);
- expect(node3.hasFocus, isTrue);
-
- node3.nextFocus();
- await tester.pump();
- expect(node1.hasFocus, isTrue);
- expect(node2.hasFocus, isFalse);
- expect(node3.hasFocus, isFalse);
-
- node1.previousFocus();
- await tester.pump();
- expect(node1.hasFocus, isFalse);
- expect(node2.hasFocus, isFalse);
- expect(node3.hasFocus, isTrue);
-
- node3.previousFocus();
- await tester.pump();
- expect(node1.hasFocus, isFalse);
- expect(node2.hasFocus, isTrue);
- expect(node3.hasFocus, isFalse);
-
- node2.previousFocus();
- await tester.pump();
- expect(node1.hasFocus, isTrue);
- expect(node2.hasFocus, isFalse);
- expect(node3.hasFocus, isFalse);
- });
-
group(ReadingOrderTraversalPolicy, () {
testWidgets('Find the initial focus if there is none yet.', (WidgetTester tester) async {
final GlobalKey key1 = GlobalKey(debugLabel: '1');
diff --git a/packages/flutter/test/widgets/navigator_test.dart b/packages/flutter/test/widgets/navigator_test.dart
index 9511193..886de4a 100644
--- a/packages/flutter/test/widgets/navigator_test.dart
+++ b/packages/flutter/test/widgets/navigator_test.dart
@@ -1487,34 +1487,29 @@
return result;
},
));
- final List<String> expected = <String>['building page 1 - false'];
- expect(log, expected);
+ expect(log, <String>['building page 1 - false']);
key.currentState!.pushReplacement(PageRouteBuilder<int>(
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
log.add('building page 2 - ${ModalRoute.of(context)!.canPop}');
return const Placeholder();
},
));
- expect(log, expected);
+ expect(log, <String>['building page 1 - false']);
await tester.pump();
- expected.add('building page 2 - false');
- expected.add('building page 1 - false'); // page 1 is rebuilt again because isCurrent changed.
- expect(log, expected);
+ expect(log, <String>['building page 1 - false', 'building page 2 - false']);
await tester.pump(const Duration(milliseconds: 150));
- expect(log, expected);
+ expect(log, <String>['building page 1 - false', 'building page 2 - false']);
key.currentState!.pushReplacement(PageRouteBuilder<int>(
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
log.add('building page 3 - ${ModalRoute.of(context)!.canPop}');
return const Placeholder();
},
));
- expect(log, expected);
+ expect(log, <String>['building page 1 - false', 'building page 2 - false']);
await tester.pump();
- expected.add('building page 3 - false');
- expected.add('building page 2 - false'); // page 2 is rebuilt again because isCurrent changed.
- expect(log, expected);
+ expect(log, <String>['building page 1 - false', 'building page 2 - false', 'building page 3 - false']);
await tester.pump(const Duration(milliseconds: 200));
- expect(log, expected);
+ expect(log, <String>['building page 1 - false', 'building page 2 - false', 'building page 3 - false']);
});
testWidgets('route semantics', (WidgetTester tester) async {
