Unverified Commit 5900c4ba authored by chunhtai's avatar chunhtai Committed by GitHub

Revert "Adds a parent scope TraversalEdgeBehavior and fixes modal rou… (#134550)

…te to no… (#130841)"

This reverts commit 0f3bd90d.

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
parent bdd96032
......@@ -1685,7 +1685,6 @@ class _WidgetsAppState extends State<WidgetsApp> with WidgetsBindingObserver {
},
onUnknownRoute: _onUnknownRoute,
observers: widget.navigatorObservers!,
routeTraversalEdgeBehavior: kIsWeb ? TraversalEdgeBehavior.leaveFlutterView : TraversalEdgeBehavior.parentScope,
reportsRouteUpdateToEngine: true,
),
);
......
......@@ -120,16 +120,6 @@ 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].
......@@ -196,60 +186,6 @@ 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<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 @@ abstract class FocusTraversalPolicy with Diagnosticable {
@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 @@ 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.
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 @@ abstract class FocusTraversalPolicy with Diagnosticable {
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 @@ 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:
return _requestTabTraversalFocus(
sortedNodes.last,
alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart,
forward: forward,
);
requestFocusCallback(sortedNodes.last, alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart);
return true;
}
}
......@@ -618,11 +514,11 @@ abstract class FocusTraversalPolicy with Diagnosticable {
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;
}
......
......@@ -1146,7 +1146,9 @@ class DefaultTransitionDelegate<T> extends TransitionDelegate<T> {
/// 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.
///
......
......@@ -834,9 +834,7 @@ class _ModalScopeState<T> extends State<_ModalScope<T>> {
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 @@ class _ModalScopeState<T> extends State<_ModalScope<T>> {
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
......@@ -1708,26 +1704,11 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
changedInternalState();
}
@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;
}
......
......@@ -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 @@ 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(
......@@ -826,8 +821,11 @@ void main() {
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', () {
......
......@@ -981,7 +981,7 @@ void main() {
expect(buttonNode2.hasFocus, isFalse);
primaryFocus!.nextFocus();
await tester.pump();
expect(buttonNode1.hasFocus, isFalse);
expect(buttonNode1.hasFocus, isTrue);
expect(buttonNode2.hasFocus, isFalse);
},
);
......
......@@ -441,96 +441,6 @@ void main() {
});
testWidgetsWithLeakTracking('Nested navigator does not trap focus', (WidgetTester tester) async {
final FocusNode node1 = FocusNode();
addTearDown(node1.dispose);
final FocusNode node2 = FocusNode();
addTearDown(node2.dispose);
final FocusNode node3 = FocusNode();
addTearDown(node3.dispose);
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, () {
testWidgetsWithLeakTracking('Find the initial focus if there is none yet.', (WidgetTester tester) async {
final GlobalKey key1 = GlobalKey(debugLabel: '1');
......
......@@ -1487,34 +1487,29 @@ void main() {
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 {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment