diff --git a/packages/flutter/lib/src/widgets/app.dart b/packages/flutter/lib/src/widgets/app.dart index d8d649c103..7a524639fb 100644 --- a/packages/flutter/lib/src/widgets/app.dart +++ b/packages/flutter/lib/src/widgets/app.dart @@ -1685,6 +1685,7 @@ class _WidgetsAppState extends State with WidgetsBindingObserver { }, 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 203e132529..f67080c71a 100644 --- a/packages/flutter/lib/src/widgets/focus_traversal.dart +++ b/packages/flutter/lib/src/widgets/focus_traversal.dart @@ -120,6 +120,16 @@ enum TraversalEdgeBehavior { /// 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]. @@ -186,6 +196,60 @@ abstract class FocusTraversalPolicy with Diagnosticable { ); } + /// 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 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. /// @@ -352,10 +416,21 @@ abstract class FocusTraversalPolicy with Diagnosticable { @protected Iterable sortDescendants(Iterable descendants, FocusNode currentNode); - Map _findGroups(FocusScopeNode scope, _FocusTraversalGroupNode? scopeGroupNode, FocusNode currentNode) { + static Iterable _getDescendantsWithoutExpandingScope(FocusNode node) { + final List result = []; + for (final FocusNode child in node.children) { + if (child is! FocusScopeNode) { + result.addAll(_getDescendantsWithoutExpandingScope(child)); + } + result.add(child); + } + return result; + } + + static Map _findGroups(FocusScopeNode scope, _FocusTraversalGroupNode? scopeGroupNode, FocusNode currentNode) { final FocusTraversalPolicy defaultPolicy = scopeGroupNode?.policy ?? ReadingOrderTraversalPolicy(); final Map groups = {}; - for (final FocusNode node in scope.descendants) { + for (final FocusNode node in _getDescendantsWithoutExpandingScope(scope)) { 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 @@ -388,7 +463,7 @@ abstract class FocusTraversalPolicy with Diagnosticable { // Sort all descendants, taking into account the FocusTraversalGroup // that they are each in, and filtering out non-traversable/focusable nodes. - List _sortAllDescendants(FocusScopeNode scope, FocusNode currentNode) { + static List _sortAllDescendants(FocusScopeNode scope, FocusNode currentNode) { final _FocusTraversalGroupNode? scopeGroupNode = FocusTraversalGroup._getGroupNode(scope); // Build the sorting data structure, separating descendants into groups. final Map groups = _findGroups(scope, scopeGroupNode, currentNode); @@ -473,30 +548,42 @@ abstract class FocusTraversalPolicy with Diagnosticable { if (focusedChild == null) { final FocusNode? firstFocus = forward ? findFirstFocus(currentNode) : findLastFocus(currentNode); if (firstFocus != null) { - requestFocusCallback( + return _requestTabTraversalFocus( firstFocus, alignmentPolicy: forward ? ScrollPositionAlignmentPolicy.keepVisibleAtEnd : ScrollPositionAlignmentPolicy.keepVisibleAtStart, + forward: forward, ); - return true; } } focusedChild ??= nearestScope; final List sortedNodes = _sortAllDescendants(nearestScope, 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: - requestFocusCallback(sortedNodes.first, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd); - return true; + return _requestTabTraversalFocus( + sortedNodes.first, + alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd, + forward: forward, + ); } } if (!forward && focusedChild == sortedNodes.first) { @@ -504,9 +591,26 @@ abstract class FocusTraversalPolicy with Diagnosticable { 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: - requestFocusCallback(sortedNodes.last, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart); - return true; + return _requestTabTraversalFocus( + sortedNodes.last, + alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart, + forward: forward, + ); } } @@ -514,11 +618,11 @@ abstract class FocusTraversalPolicy with Diagnosticable { FocusNode? previousNode; for (final FocusNode node in maybeFlipped) { if (previousNode == focusedChild) { - requestFocusCallback( + return _requestTabTraversalFocus( 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 ccf3fbfb56..bc57d8a391 100644 --- a/packages/flutter/lib/src/widgets/navigator.dart +++ b/packages/flutter/lib/src/widgets/navigator.dart @@ -1146,9 +1146,7 @@ class DefaultTransitionDelegate extends TransitionDelegate { /// The default value of [Navigator.routeTraversalEdgeBehavior]. /// /// {@macro flutter.widgets.navigator.routeTraversalEdgeBehavior} -const TraversalEdgeBehavior kDefaultRouteTraversalEdgeBehavior = kIsWeb - ? TraversalEdgeBehavior.leaveFlutterView - : TraversalEdgeBehavior.closedLoop; +const TraversalEdgeBehavior kDefaultRouteTraversalEdgeBehavior = TraversalEdgeBehavior.parentScope; /// 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 b51cb4f61c..fc954fa0a1 100644 --- a/packages/flutter/lib/src/widgets/routes.dart +++ b/packages/flutter/lib/src/widgets/routes.dart @@ -834,7 +834,9 @@ class _ModalScopeState extends State<_ModalScope> { 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 @@ -936,6 +938,8 @@ class _ModalScopeState extends State<_ModalScope> { 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 @@ -1704,11 +1708,26 @@ abstract class ModalRoute extends TransitionRoute with LocalHistoryRoute? nextRoute) { + super.didChangeNext(nextRoute); + changedInternalState(); + } + + @override + void didPopNext(Route nextRoute) { + super.didPopNext(nextRoute); + changedInternalState(); + } + @override void changedInternalState() { super.changedInternalState(); - setState(() { /* internal state already changed */ }); - _modalBarrier.markNeedsBuild(); + // 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(); + } _modalScope.maintainState = maintainState; } diff --git a/packages/flutter/test/material/icon_button_test.dart b/packages/flutter/test/material/icon_button_test.dart index fce7855170..02c6ad08d4 100644 --- a/packages/flutter/test/material/icon_button_test.dart +++ b/packages/flutter/test/material/icon_button_test.dart @@ -4,6 +4,7 @@ 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'; @@ -792,6 +793,10 @@ void main() { 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( @@ -821,11 +826,8 @@ void main() { expect(focusNode1.nextFocus(), isFalse); await tester.pump(); - expect(focusNode1.hasPrimaryFocus, isTrue); + expect(focusNode1.hasPrimaryFocus, !kIsWeb); 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 b59f17f8ea..6a246304e3 100644 --- a/packages/flutter/test/widgets/actions_test.dart +++ b/packages/flutter/test/widgets/actions_test.dart @@ -965,7 +965,7 @@ void main() { expect(buttonNode2.hasFocus, isFalse); primaryFocus!.nextFocus(); await tester.pump(); - expect(buttonNode1.hasFocus, isTrue); + expect(buttonNode1.hasFocus, isFalse); 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 ff0b8c9729..04cf497d08 100644 --- a/packages/flutter/test/widgets/focus_traversal_test.dart +++ b/packages/flutter/test/widgets/focus_traversal_test.dart @@ -430,6 +430,92 @@ void main() { }); + 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: [ + Focus( + focusNode: node1, + child: const SizedBox(width: 100, height: 100), + ), + SizedBox( + width: 100, + height: 100, + child: Navigator( + pages: >[ + MaterialPage( + 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 886de4a712..95111931e3 100644 --- a/packages/flutter/test/widgets/navigator_test.dart +++ b/packages/flutter/test/widgets/navigator_test.dart @@ -1487,29 +1487,34 @@ void main() { return result; }, )); - expect(log, ['building page 1 - false']); + final List expected = ['building page 1 - false']; + expect(log, expected); key.currentState!.pushReplacement(PageRouteBuilder( pageBuilder: (BuildContext context, Animation animation, Animation secondaryAnimation) { log.add('building page 2 - ${ModalRoute.of(context)!.canPop}'); return const Placeholder(); }, )); - expect(log, ['building page 1 - false']); + expect(log, expected); await tester.pump(); - expect(log, ['building page 1 - false', 'building page 2 - false']); + expected.add('building page 2 - false'); + expected.add('building page 1 - false'); // page 1 is rebuilt again because isCurrent changed. + expect(log, expected); await tester.pump(const Duration(milliseconds: 150)); - expect(log, ['building page 1 - false', 'building page 2 - false']); + expect(log, expected); key.currentState!.pushReplacement(PageRouteBuilder( pageBuilder: (BuildContext context, Animation animation, Animation secondaryAnimation) { log.add('building page 3 - ${ModalRoute.of(context)!.canPop}'); return const Placeholder(); }, )); - expect(log, ['building page 1 - false', 'building page 2 - false']); + expect(log, expected); await tester.pump(); - expect(log, ['building page 1 - false', 'building page 2 - false', 'building page 3 - false']); + expected.add('building page 3 - false'); + expected.add('building page 2 - false'); // page 2 is rebuilt again because isCurrent changed. + expect(log, expected); await tester.pump(const Duration(milliseconds: 200)); - expect(log, ['building page 1 - false', 'building page 2 - false', 'building page 3 - false']); + expect(log, expected); }); testWidgets('route semantics', (WidgetTester tester) async {