Skip to content

Commit 5900c4b

Browse files
authored
Revert "Adds a parent scope TraversalEdgeBehavior and fixes modal rou… (flutter#134550)
…te to no… (flutter#130841)" This reverts commit 0f3bd90. Internal test needs migration ## 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 bdd9603 commit 5900c4b

File tree

8 files changed

+35
-254
lines changed

8 files changed

+35
-254
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1685,7 +1685,6 @@ class _WidgetsAppState extends State<WidgetsApp> with WidgetsBindingObserver {
16851685
},
16861686
onUnknownRoute: _onUnknownRoute,
16871687
observers: widget.navigatorObservers!,
1688-
routeTraversalEdgeBehavior: kIsWeb ? TraversalEdgeBehavior.leaveFlutterView : TraversalEdgeBehavior.parentScope,
16891688
reportsRouteUpdateToEngine: true,
16901689
),
16911690
);

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

Lines changed: 17 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -120,16 +120,6 @@ 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,
133123
}
134124

135125
/// Determines how focusable widgets are traversed within a [FocusTraversalGroup].
@@ -196,60 +186,6 @@ abstract class FocusTraversalPolicy with Diagnosticable {
196186
);
197187
}
198188

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-
253189
/// Returns the node that should receive focus if focus is traversing
254190
/// forwards, and there is no current focus.
255191
///
@@ -416,21 +352,10 @@ abstract class FocusTraversalPolicy with Diagnosticable {
416352
@protected
417353
Iterable<FocusNode> sortDescendants(Iterable<FocusNode> descendants, FocusNode currentNode);
418354

419-
static Iterable<FocusNode> _getDescendantsWithoutExpandingScope(FocusNode node) {
420-
final List<FocusNode> result = <FocusNode>[];
421-
for (final FocusNode child in node.children) {
422-
if (child is! FocusScopeNode) {
423-
result.addAll(_getDescendantsWithoutExpandingScope(child));
424-
}
425-
result.add(child);
426-
}
427-
return result;
428-
}
429-
430-
static Map<FocusNode?, _FocusTraversalGroupInfo> _findGroups(FocusScopeNode scope, _FocusTraversalGroupNode? scopeGroupNode, FocusNode currentNode) {
355+
Map<FocusNode?, _FocusTraversalGroupInfo> _findGroups(FocusScopeNode scope, _FocusTraversalGroupNode? scopeGroupNode, FocusNode currentNode) {
431356
final FocusTraversalPolicy defaultPolicy = scopeGroupNode?.policy ?? ReadingOrderTraversalPolicy();
432357
final Map<FocusNode?, _FocusTraversalGroupInfo> groups = <FocusNode?, _FocusTraversalGroupInfo>{};
433-
for (final FocusNode node in _getDescendantsWithoutExpandingScope(scope)) {
358+
for (final FocusNode node in scope.descendants) {
434359
final _FocusTraversalGroupNode? groupNode = FocusTraversalGroup._getGroupNode(node);
435360
// Group nodes need to be added to their parent's node, or to the "null"
436361
// node if no parent is found. This creates the hierarchy of group nodes
@@ -463,7 +388,7 @@ abstract class FocusTraversalPolicy with Diagnosticable {
463388

464389
// Sort all descendants, taking into account the FocusTraversalGroup
465390
// that they are each in, and filtering out non-traversable/focusable nodes.
466-
static List<FocusNode> _sortAllDescendants(FocusScopeNode scope, FocusNode currentNode) {
391+
List<FocusNode> _sortAllDescendants(FocusScopeNode scope, FocusNode currentNode) {
467392
final _FocusTraversalGroupNode? scopeGroupNode = FocusTraversalGroup._getGroupNode(scope);
468393
// Build the sorting data structure, separating descendants into groups.
469394
final Map<FocusNode?, _FocusTraversalGroupInfo> groups = _findGroups(scope, scopeGroupNode, currentNode);
@@ -548,81 +473,52 @@ abstract class FocusTraversalPolicy with Diagnosticable {
548473
if (focusedChild == null) {
549474
final FocusNode? firstFocus = forward ? findFirstFocus(currentNode) : findLastFocus(currentNode);
550475
if (firstFocus != null) {
551-
return _requestTabTraversalFocus(
476+
requestFocusCallback(
552477
firstFocus,
553478
alignmentPolicy: forward ? ScrollPositionAlignmentPolicy.keepVisibleAtEnd : ScrollPositionAlignmentPolicy.keepVisibleAtStart,
554-
forward: forward,
555479
);
480+
return true;
556481
}
557482
}
558483
focusedChild ??= nearestScope;
559484
final List<FocusNode> sortedNodes = _sortAllDescendants(nearestScope, focusedChild);
560-
assert(sortedNodes.contains(focusedChild));
561485

486+
assert(sortedNodes.contains(focusedChild));
487+
if (sortedNodes.length < 2) {
488+
// If there are no nodes to traverse to, like when descendantsAreTraversable
489+
// is false or skipTraversal for all the nodes is true.
490+
return false;
491+
}
562492
if (forward && focusedChild == sortedNodes.last) {
563493
switch (nearestScope.traversalEdgeBehavior) {
564494
case TraversalEdgeBehavior.leaveFlutterView:
565495
focusedChild.unfocus();
566496
return false;
567-
case TraversalEdgeBehavior.parentScope:
568-
final FocusScopeNode? parentScope = nearestScope.enclosingScope;
569-
if (parentScope != null && parentScope != FocusManager.instance.rootScope) {
570-
focusedChild.unfocus();
571-
parentScope.nextFocus();
572-
// Verify the focus really has changed.
573-
return focusedChild.enclosingScope?.focusedChild != focusedChild;
574-
}
575-
// No valid parent scope. Fallback to closed loop behavior.
576-
return _requestTabTraversalFocus(
577-
sortedNodes.first,
578-
alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd,
579-
forward: forward,
580-
);
581497
case TraversalEdgeBehavior.closedLoop:
582-
return _requestTabTraversalFocus(
583-
sortedNodes.first,
584-
alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd,
585-
forward: forward,
586-
);
498+
requestFocusCallback(sortedNodes.first, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtEnd);
499+
return true;
587500
}
588501
}
589502
if (!forward && focusedChild == sortedNodes.first) {
590503
switch (nearestScope.traversalEdgeBehavior) {
591504
case TraversalEdgeBehavior.leaveFlutterView:
592505
focusedChild.unfocus();
593506
return false;
594-
case TraversalEdgeBehavior.parentScope:
595-
final FocusScopeNode? parentScope = nearestScope.enclosingScope;
596-
if (parentScope != null && parentScope != FocusManager.instance.rootScope) {
597-
focusedChild.unfocus();
598-
parentScope.previousFocus();
599-
// Verify the focus really has changed.
600-
return focusedChild.enclosingScope?.focusedChild != focusedChild;
601-
}
602-
// No valid parent scope. Fallback to closed loop behavior.
603-
return _requestTabTraversalFocus(
604-
sortedNodes.last,
605-
alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart,
606-
forward: forward,
607-
);
608507
case TraversalEdgeBehavior.closedLoop:
609-
return _requestTabTraversalFocus(
610-
sortedNodes.last,
611-
alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart,
612-
forward: forward,
613-
);
508+
requestFocusCallback(sortedNodes.last, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart);
509+
return true;
614510
}
615511
}
616512

617513
final Iterable<FocusNode> maybeFlipped = forward ? sortedNodes : sortedNodes.reversed;
618514
FocusNode? previousNode;
619515
for (final FocusNode node in maybeFlipped) {
620516
if (previousNode == focusedChild) {
621-
return _requestTabTraversalFocus(
517+
requestFocusCallback(
622518
node,
623519
alignmentPolicy: forward ? ScrollPositionAlignmentPolicy.keepVisibleAtEnd : ScrollPositionAlignmentPolicy.keepVisibleAtStart,
624-
forward: forward,
625520
);
521+
return true;
626522
}
627523
previousNode = node;
628524
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1146,7 +1146,9 @@ class DefaultTransitionDelegate<T> extends TransitionDelegate<T> {
11461146
/// The default value of [Navigator.routeTraversalEdgeBehavior].
11471147
///
11481148
/// {@macro flutter.widgets.navigator.routeTraversalEdgeBehavior}
1149-
const TraversalEdgeBehavior kDefaultRouteTraversalEdgeBehavior = TraversalEdgeBehavior.parentScope;
1149+
const TraversalEdgeBehavior kDefaultRouteTraversalEdgeBehavior = kIsWeb
1150+
? TraversalEdgeBehavior.leaveFlutterView
1151+
: TraversalEdgeBehavior.closedLoop;
11501152

11511153
/// A widget that manages a set of child widgets with a stack discipline.
11521154
///

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

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -834,9 +834,7 @@ 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(
838-
debugLabel: '$_ModalScopeState Focus Scope',
839-
);
837+
final FocusScopeNode focusScopeNode = FocusScopeNode(debugLabel: '$_ModalScopeState Focus Scope');
840838
final ScrollController primaryScrollController = ScrollController();
841839

842840
@override
@@ -938,8 +936,6 @@ class _ModalScopeState<T> extends State<_ModalScope<T>> {
938936
controller: primaryScrollController,
939937
child: FocusScope(
940938
node: focusScopeNode, // immutable
941-
// Only top most route can participate in focus traversal.
942-
skipTraversal: !widget.route.isCurrent,
943939
child: RepaintBoundary(
944940
child: AnimatedBuilder(
945941
animation: _listenable, // immutable
@@ -1708,26 +1704,11 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
17081704
changedInternalState();
17091705
}
17101706

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-
17231707
@override
17241708
void changedInternalState() {
17251709
super.changedInternalState();
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-
}
1710+
setState(() { /* internal state already changed */ });
1711+
_modalBarrier.markNeedsBuild();
17311712
_modalScope.maintainState = maintainState;
17321713
}
17331714

packages/flutter/test/material/icon_button_test.dart

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

55
import 'dart:ui';
66

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

801796
await tester.pumpWidget(
802797
wrap(
@@ -826,8 +821,11 @@ void main() {
826821
expect(focusNode1.nextFocus(), isFalse);
827822
await tester.pump();
828823

829-
expect(focusNode1.hasPrimaryFocus, !kIsWeb);
824+
expect(focusNode1.hasPrimaryFocus, isTrue);
830825
expect(focusNode2.hasPrimaryFocus, isFalse);
826+
827+
focusNode1.dispose();
828+
focusNode2.dispose();
831829
});
832830

833831
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, isFalse);
984+
expect(buttonNode1.hasFocus, isTrue);
985985
expect(buttonNode2.hasFocus, isFalse);
986986
},
987987
);

0 commit comments

Comments
 (0)