Skip to content

Commit ff73448

Browse files
chunhtaichunhtai
authored
Reland "Adds a parent scope TraversalEdgeBehavior and fixes modal rou… (#134554)
…… (#134550)" fixes flutter/flutter#112567 This reverts commit 5900c4b. The internal test needs migration. cl/564746935 This is the same of original pr, no new change ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [Features we expect every widget to implement]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
1 parent 51f1a46 commit ff73448

File tree

8 files changed

+254
-35
lines changed

8 files changed

+254
-35
lines changed

‎packages/flutter/lib/src/widgets/app.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1683,6 +1683,7 @@ class _WidgetsAppState extends State<WidgetsApp> with WidgetsBindingObserver {
16831683
},
16841684
onUnknownRoute: _onUnknownRoute,
16851685
observers: widget.navigatorObservers!,
1686+
routeTraversalEdgeBehavior: kIsWeb ? TraversalEdgeBehavior.leaveFlutterView : TraversalEdgeBehavior.parentScope,
16861687
reportsRouteUpdateToEngine: true,
16871688
),
16881689
);

‎packages/flutter/lib/src/widgets/focus_traversal.dart‎

Lines changed: 121 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,16 @@ enum TraversalEdgeBehavior {
120120
/// address bar, escape an `iframe`, or focus on HTML elements other than
121121
/// those managed by Flutter.
122122
leaveFlutterView,
123+
124+
/// Allows focus to traverse up to parent scope.
125+
///
126+
/// When reaching the edge of the current scope, requesting the next focus
127+
/// will look up to the parent scope of the current scope and focus the focus
128+
/// node next to the current scope.
129+
///
130+
/// If there is no parent scope above the current scope, fallback to
131+
/// [closedLoop] behavior.
132+
parentScope,
123133
}
124134

125135
/// Determines how focusable widgets are traversed within a [FocusTraversalGroup].
@@ -186,6 +196,60 @@ abstract class FocusTraversalPolicy with Diagnosticable {
186196
);
187197
}
188198

199+
/// Request focus on a focus node as a result of a tab traversal.
200+
///
201+
/// If the `node` is a [FocusScopeNode], this method will recursively find
202+
/// the next focus from its descendants until it find a regular [FocusNode].
203+
///
204+
/// Returns true if this method focused a new focus node.
205+
bool _requestTabTraversalFocus(
206+
FocusNode node, {
207+
ScrollPositionAlignmentPolicy? alignmentPolicy,
208+
double? alignment,
209+
Duration? duration,
210+
Curve? curve,
211+
required bool forward,
212+
}) {
213+
if (node is FocusScopeNode) {
214+
if (node.focusedChild != null) {
215+
// Can't stop here as the `focusedChild` may be a focus scope node
216+
// without a first focus. The first focus will be picked in the
217+
// next iteration.
218+
return _requestTabTraversalFocus(
219+
node.focusedChild!,
220+
alignmentPolicy: alignmentPolicy,
221+
alignment: alignment,
222+
duration: duration,
223+
curve: curve,
224+
forward: forward,
225+
);
226+
}
227+
final List<FocusNode> sortedChildren = _sortAllDescendants(node, node);
228+
if (sortedChildren.isNotEmpty) {
229+
_requestTabTraversalFocus(
230+
forward ? sortedChildren.first : sortedChildren.last,
231+
alignmentPolicy: alignmentPolicy,
232+
alignment: alignment,
233+
duration: duration,
234+
curve: curve,
235+
forward: forward,
236+
);
237+
// Regardless if _requestTabTraversalFocus return true or false, a first
238+
// focus has been picked.
239+
return true;
240+
}
241+
}
242+
final bool nodeHadPrimaryFocus = node.hasPrimaryFocus;
243+
requestFocusCallback(
244+
node,
245+
alignmentPolicy: alignmentPolicy,
246+
alignment: alignment,
247+
duration: duration,
248+
curve: curve,
249+
);
250+
return !nodeHadPrimaryFocus;
251+
}
252+
189253
/// Returns the node that should receive focus if focus is traversing
190254
/// forwards, and there is no current focus.
191255
///
@@ -340,10 +404,21 @@ abstract class FocusTraversalPolicy with Diagnosticable {
340404
@protected
341405
Iterable<FocusNode> sortDescendants(Iterable<FocusNode> descendants, FocusNode currentNode);
342406

343-
Map<FocusNode?, _FocusTraversalGroupInfo> _findGroups(FocusScopeNode scope, _FocusTraversalGroupNode? scopeGroupNode, FocusNode currentNode) {
407+
static Iterable<FocusNode> _getDescendantsWithoutExpandingScope(FocusNode node) {
408+
final List<FocusNode> result = <FocusNode>[];
409+
for (final FocusNode child in node.children) {
410+
if (child is! FocusScopeNode) {
411+
result.addAll(_getDescendantsWithoutExpandingScope(child));
412+
}
413+
result.add(child);
414+
}
415+
return result;
416+
}
417+
418+
static Map<FocusNode?, _FocusTraversalGroupInfo> _findGroups(FocusScopeNode scope, _FocusTraversalGroupNode? scopeGroupNode, FocusNode currentNode) {
344419
final FocusTraversalPolicy defaultPolicy = scopeGroupNode?.policy ?? ReadingOrderTraversalPolicy();
345420
final Map<FocusNode?, _FocusTraversalGroupInfo> groups = <FocusNode?, _FocusTraversalGroupInfo>{};
346-
for (final FocusNode node in scope.descendants) {
421+
for (final FocusNode node in _getDescendantsWithoutExpandingScope(scope)) {
347422
final _FocusTraversalGroupNode? groupNode = FocusTraversalGroup._getGroupNode(node);
348423
// Group nodes need to be added to their parent's node, or to the "null"
349424
// node if no parent is found. This creates the hierarchy of group nodes
@@ -376,7 +451,7 @@ abstract class FocusTraversalPolicy with Diagnosticable {
376451

377452
// Sort all descendants, taking into account the FocusTraversalGroup
378453
// that they are each in, and filtering out non-traversable/focusable nodes.
379-
List<FocusNode> _sortAllDescendants(FocusScopeNode scope, FocusNode currentNode) {
454+
static List<FocusNode> _sortAllDescendants(FocusScopeNode scope, FocusNode currentNode) {
380455
final _FocusTraversalGroupNode? scopeGroupNode = FocusTraversalGroup._getGroupNode(scope);
381456
// Build the sorting data structure, separating descendants into groups.
382457
final Map<FocusNode?, _FocusTraversalGroupInfo> groups = _findGroups(scope, scopeGroupNode, currentNode);
@@ -463,52 +538,81 @@ abstract class FocusTraversalPolicy with Diagnosticable {
463538
if (focusedChild == null) {
464539
final FocusNode? firstFocus = forward ? findFirstFocus(currentNode) : findLastFocus(currentNode);
465540
if (firstFocus != null) {
466-
requestFocusCallback(
541+
return _requestTabTraversalFocus(
467542
firstFocus,
468543
alignmentPolicy: forward ? ScrollPositionAlignmentPolicy.keepVisibleAtEnd : ScrollPositionAlignmentPolicy.keepVisibleAtStart,
544+
forward: forward,
469545
);
470-
return true;
471546
}
472547
}
473548
focusedChild ??= nearestScope;
474549
final List<FocusNode> sortedNodes = _sortAllDescendants(nearestScope, focusedChild);
475-
476550
assert(sortedNodes.contains(focusedChild));
477-
if (sortedNodes.length < 2) {
478-
// If there are no nodes to traverse to, like when descendantsAreTraversable
479-
// is false or skipTraversal for all the nodes is true.
480-
return false;
481-
}
551+
482552
if (forward && focusedChild == sortedNodes.last) {
483553
switch (nearestScope.traversalEdgeBehavior) {
484554
case TraversalEdgeBehavior.leaveFlutterView:
485555
focusedChild.unfocus();
486556
return false;
557+
case TraversalEdgeBehavior.parentScope:
558+
final FocusScopeNode? parentScope = nearestScope.enclosingScope;
559+
if (parentScope != null && parentScope != FocusManager.instance.rootScope) {
560+
focusedChild.unfocus();
561+
parentScope.nextFocus();
562+
// Verify the focus really has changed.
563+
return focusedChild.enclosingScope?.focusedChild != focusedChild;
564+
}
565+
// No valid parent scope. Fallback to closed loop behavior.
566+
return _requestTabTraversalFocus(
567+
sortedNodes.first,
568+
alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd,
569+
forward: forward,
570+
);
487571
case TraversalEdgeBehavior.closedLoop:
488-
requestFocusCallback(sortedNodes.first, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd);
489-
return true;
572+
return _requestTabTraversalFocus(
573+
sortedNodes.first,
574+
alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd,
575+
forward: forward,
576+
);
490577
}
491578
}
492579
if (!forward && focusedChild == sortedNodes.first) {
493580
switch (nearestScope.traversalEdgeBehavior) {
494581
case TraversalEdgeBehavior.leaveFlutterView:
495582
focusedChild.unfocus();
496583
return false;
584+
case TraversalEdgeBehavior.parentScope:
585+
final FocusScopeNode? parentScope = nearestScope.enclosingScope;
586+
if (parentScope != null && parentScope != FocusManager.instance.rootScope) {
587+
focusedChild.unfocus();
588+
parentScope.previousFocus();
589+
// Verify the focus really has changed.
590+
return focusedChild.enclosingScope?.focusedChild != focusedChild;
591+
}
592+
// No valid parent scope. Fallback to closed loop behavior.
593+
return _requestTabTraversalFocus(
594+
sortedNodes.last,
595+
alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart,
596+
forward: forward,
597+
);
497598
case TraversalEdgeBehavior.closedLoop:
498-
requestFocusCallback(sortedNodes.last, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart);
499-
return true;
599+
return _requestTabTraversalFocus(
600+
sortedNodes.last,
601+
alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart,
602+
forward: forward,
603+
);
500604
}
501605
}
502606

503607
final Iterable<FocusNode> maybeFlipped = forward ? sortedNodes : sortedNodes.reversed;
504608
FocusNode? previousNode;
505609
for (final FocusNode node in maybeFlipped) {
506610
if (previousNode == focusedChild) {
507-
requestFocusCallback(
611+
return _requestTabTraversalFocus(
508612
node,
509613
alignmentPolicy: forward ? ScrollPositionAlignmentPolicy.keepVisibleAtEnd : ScrollPositionAlignmentPolicy.keepVisibleAtStart,
614+
forward: forward,
510615
);
511-
return true;
512616
}
513617
previousNode = node;
514618
}

‎packages/flutter/lib/src/widgets/navigator.dart‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,9 +1144,7 @@ class DefaultTransitionDelegate<T> extends TransitionDelegate<T> {
11441144
/// The default value of [Navigator.routeTraversalEdgeBehavior].
11451145
///
11461146
/// {@macro flutter.widgets.navigator.routeTraversalEdgeBehavior}
1147-
const TraversalEdgeBehavior kDefaultRouteTraversalEdgeBehavior = kIsWeb
1148-
? TraversalEdgeBehavior.leaveFlutterView
1149-
: TraversalEdgeBehavior.closedLoop;
1147+
const TraversalEdgeBehavior kDefaultRouteTraversalEdgeBehavior = TraversalEdgeBehavior.parentScope;
11501148

11511149
/// A widget that manages a set of child widgets with a stack discipline.
11521150
///

‎packages/flutter/lib/src/widgets/routes.dart‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -834,7 +834,9 @@ class _ModalScopeState<T> extends State<_ModalScope<T>> {
834834
late Listenable _listenable;
835835

836836
/// The node this scope will use for its root [FocusScope] widget.
837-
final FocusScopeNode focusScopeNode = FocusScopeNode(debugLabel: '$_ModalScopeState Focus Scope');
837+
final FocusScopeNode focusScopeNode = FocusScopeNode(
838+
debugLabel: '$_ModalScopeState Focus Scope',
839+
);
838840
final ScrollController primaryScrollController = ScrollController();
839841

840842
@override
@@ -936,6 +938,8 @@ class _ModalScopeState<T> extends State<_ModalScope<T>> {
936938
controller: primaryScrollController,
937939
child: FocusScope(
938940
node: focusScopeNode, // immutable
941+
// Only top most route can participate in focus traversal.
942+
skipTraversal: !widget.route.isCurrent,
939943
child: RepaintBoundary(
940944
child: AnimatedBuilder(
941945
animation: _listenable, // immutable
@@ -1704,11 +1708,26 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
17041708
changedInternalState();
17051709
}
17061710

1711+
@override
1712+
void didChangeNext(Route<dynamic>? nextRoute) {
1713+
super.didChangeNext(nextRoute);
1714+
changedInternalState();
1715+
}
1716+
1717+
@override
1718+
void didPopNext(Route<dynamic> nextRoute) {
1719+
super.didPopNext(nextRoute);
1720+
changedInternalState();
1721+
}
1722+
17071723
@override
17081724
void changedInternalState() {
17091725
super.changedInternalState();
1710-
setState(() { /* internal state already changed */ });
1711-
_modalBarrier.markNeedsBuild();
1726+
// No need to mark dirty if this method is called during build phase.
1727+
if (SchedulerBinding.instance.schedulerPhase != SchedulerPhase.persistentCallbacks) {
1728+
setState(() { /* internal state already changed */ });
1729+
_modalBarrier.markNeedsBuild();
1730+
}
17121731
_modalScope.maintainState = maintainState;
17131732
}
17141733

‎packages/flutter/test/material/icon_button_test.dart‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'dart:ui';
66

7+
import 'package:flutter/foundation.dart';
78
import 'package:flutter/material.dart';
89
import 'package:flutter/rendering.dart';
910
import 'package:flutter_test/flutter_test.dart';
@@ -792,6 +793,10 @@ void main() {
792793
testWidgetsWithLeakTracking("Disabled IconButton can't be traversed to when disabled.", (WidgetTester tester) async {
793794
final FocusNode focusNode1 = FocusNode(debugLabel: 'IconButton 1');
794795
final FocusNode focusNode2 = FocusNode(debugLabel: 'IconButton 2');
796+
addTearDown(() {
797+
focusNode1.dispose();
798+
focusNode2.dispose();
799+
});
795800

796801
await tester.pumpWidget(
797802
wrap(
@@ -821,11 +826,8 @@ void main() {
821826
expect(focusNode1.nextFocus(), isFalse);
822827
await tester.pump();
823828

824-
expect(focusNode1.hasPrimaryFocus, isTrue);
829+
expect(focusNode1.hasPrimaryFocus, !kIsWeb);
825830
expect(focusNode2.hasPrimaryFocus, isFalse);
826-
827-
focusNode1.dispose();
828-
focusNode2.dispose();
829831
});
830832

831833
group('feedback', () {

‎packages/flutter/test/widgets/actions_test.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -981,7 +981,7 @@ void main() {
981981
expect(buttonNode2.hasFocus, isFalse);
982982
primaryFocus!.nextFocus();
983983
await tester.pump();
984-
expect(buttonNode1.hasFocus, isTrue);
984+
expect(buttonNode1.hasFocus, isFalse);
985985
expect(buttonNode2.hasFocus, isFalse);
986986
},
987987
);

0 commit comments

Comments
 (0)