Skip to content

F
Front-End
Unverified Commit b1d8a4e4 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub
Browse files
Options

Modify focus traversal policy search to use focus tree instead of widget tree (#121186) 

* Modify focus traversal policy search to use focus tree instead of widget tree

* Eliminate unnecessary inherited widget

* Remove unintentional change

* Look for focus nodes without creating a dependency.

* Add test

* Review Changes

* Fix debug_test.dart

* Rebase onto master
parent 5d36cb77
Show whitespace changes
Inline Side-by-side
Showing with 213 additions and 126 deletions
packages/flutter/lib/src/widgets/focus_scope.dart
View file @ b1d8a4e4
......@@ -370,21 +370,17 @@ class Focus extends StatefulWidget {
/// given [BuildContext].
///
/// If no [Focus] node is found before reaching the nearest [FocusScope]
/// widget, or there is no [Focus] widget in scope, then this method will
/// throw an exception.
/// widget, or there is no [Focus] widget in the context, then this method
/// will throw an exception.
///
/// The `context` and `scopeOk` arguments must not be null.
///
/// Calling this function creates a dependency that will rebuild the given
/// context when the focus changes.
/// {@macro flutter.widgets.focus_scope.Focus.maybeOf}
///
/// See also:
///
/// * [maybeOf], which is similar to this function, but will return null
/// instead of throwing if it doesn't find a [Focus] node.
static FocusNode of(BuildContext context, { bool scopeOk = false }) {
final _FocusInheritedScope? marker = context.dependOnInheritedWidgetOfExactType<_FocusInheritedScope>();
final FocusNode? node = marker?.notifier;
static FocusNode of(BuildContext context, { bool scopeOk = false, bool createDependency = true }) {
final FocusNode? node = Focus.maybeOf(context, scopeOk: scopeOk, createDependency: createDependency);
assert(() {
if (node == null) {
throw FlutterError(
......@@ -423,18 +419,24 @@ class Focus extends StatefulWidget {
/// widget, or there is no [Focus] widget in scope, then this method will
/// return null.
///
/// The `context` and `scopeOk` arguments must not be null.
///
/// Calling this function creates a dependency that will rebuild the given
/// context when the focus changes.
/// {@template flutter.widgets.focus_scope.Focus.maybeOf}
/// If `createDependency` is true (which is the default), calling this
/// function creates a dependency that will rebuild the given context when the
/// focus node gains or loses focus.
/// {@endtemplate}
///
/// See also:
///
/// * [of], which is similar to this function, but will throw an exception if
/// it doesn't find a [Focus] node instead of returning null.
static FocusNode? maybeOf(BuildContext context, { bool scopeOk = false }) {
final _FocusInheritedScope? marker = context.dependOnInheritedWidgetOfExactType<_FocusInheritedScope>();
final FocusNode? node = marker?.notifier;
/// it doesn't find a [Focus] node, instead of returning null.
static FocusNode? maybeOf(BuildContext context, { bool scopeOk = false, bool createDependency = true }) {
final _FocusInheritedScope? scope;
if (createDependency) {
scope = context.dependOnInheritedWidgetOfExactType<_FocusInheritedScope>();
} else {
scope = context.getInheritedWidgetOfExactType<_FocusInheritedScope>();
}
final FocusNode? node = scope?.notifier;
if (node == null) {
return null;
}
......@@ -776,16 +778,16 @@ class FocusScope extends Focus {
ValueChanged<bool>? onFocusChange,
}) = _FocusScopeWithExternalFocusNode;
/// Returns the [FocusScopeNode] of the [FocusScope] that most tightly
/// encloses the given [context].
/// Returns the [FocusNode.nearestScope] of the [Focus] or [FocusScope] that
/// most tightly encloses the given [context].
///
/// If this node doesn't have a [Focus] widget ancestor, then the
/// [FocusManager.rootScope] is returned.
/// If this node doesn't have a [Focus] or [FocusScope] widget ancestor, then
/// the [FocusManager.rootScope] is returned.
///
/// The [context] argument must not be null.
static FocusScopeNode of(BuildContext context) {
final _FocusInheritedScope? marker = context.dependOnInheritedWidgetOfExactType<_FocusInheritedScope>();
return marker?.notifier?.nearestScope ?? context.owner!.focusManager.rootScope;
/// {@macro flutter.widgets.focus_scope.Focus.maybeOf}
static FocusScopeNode of(BuildContext context, { bool createDependency = true }) {
return Focus.maybeOf(context, scopeOk: true, createDependency: createDependency)?.nearestScope
?? context.owner!.focusManager.rootScope;
}
@override
......
packages/flutter/lib/src/widgets/focus_traversal.dart
View file @ b1d8a4e4
......@@ -47,11 +47,11 @@ void _focusAndEnsureVisible(
// sorting their contents.
class _FocusTraversalGroupInfo {
_FocusTraversalGroupInfo(
_FocusTraversalGroupScope? marker, {
_FocusTraversalGroupNode? group, {
FocusTraversalPolicy? defaultPolicy,
List<FocusNode>? members,
}) : groupNode = marker?.focusNode,
policy = marker?.policy ?? defaultPolicy ?? ReadingOrderTraversalPolicy(),
}) : groupNode = group,
policy = group?.policy ?? defaultPolicy ?? ReadingOrderTraversalPolicy(),
members = members ?? <FocusNode>[];
final FocusNode? groupNode;
......@@ -318,45 +318,43 @@ abstract class FocusTraversalPolicy with Diagnosticable {
@protected
Iterable<FocusNode> sortDescendants(Iterable<FocusNode> descendants, FocusNode currentNode);
_FocusTraversalGroupScope? _getMarker(BuildContext? context) {
return context?.getInheritedWidgetOfExactType<_FocusTraversalGroupScope>();
}
// Sort all descendants, taking into account the FocusTraversalGroup
// that they are each in, and filtering out non-traversable/focusable nodes.
List<FocusNode> _sortAllDescendants(FocusScopeNode scope, FocusNode currentNode) {
final _FocusTraversalGroupScope? scopeGroupMarker = _getMarker(scope.context);
final FocusTraversalPolicy defaultPolicy = scopeGroupMarker?.policy ?? ReadingOrderTraversalPolicy();
// Build the sorting data structure, separating descendants into groups.
Map<FocusNode?, _FocusTraversalGroupInfo> _findGroups(FocusScopeNode scope, _FocusTraversalGroupNode? scopeGroupNode) {
final FocusTraversalPolicy defaultPolicy = scopeGroupNode?.policy ?? ReadingOrderTraversalPolicy();
final Map<FocusNode?, _FocusTraversalGroupInfo> groups = <FocusNode?, _FocusTraversalGroupInfo>{};
for (final FocusNode node in scope.descendants) {
final _FocusTraversalGroupScope? groupMarker = _getMarker(node.context);
final FocusNode? groupNode = groupMarker?.focusNode;
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
// and makes it so the entire group is sorted along with the other members
// of the parent group.
if (node == groupNode) {
// To find the parent of the group node, we need to skip over the parent
// of the Focus node in _FocusTraversalGroupState.build, and start
// looking with that node's parent, since _getMarker will return the
// context it was called on if it matches the type.
final BuildContext? parentContext = _getAncestor(groupNode!.context!, count: 2);
final _FocusTraversalGroupScope? parentMarker = _getMarker(parentContext);
final FocusNode? parentNode = parentMarker?.focusNode;
groups[parentNode] ??= _FocusTraversalGroupInfo(parentMarker, members: <FocusNode>[], defaultPolicy: defaultPolicy);
assert(!groups[parentNode]!.members.contains(node));
groups[parentNode]!.members.add(groupNode);
// of the Focus node added in _FocusTraversalGroupState.build, and start
// looking with that node's parent, since _getGroupNode will return the
// node it was called on if it matches the type.
final _FocusTraversalGroupNode? parentGroup = FocusTraversalGroup._getGroupNode(groupNode!.parent!);
groups[parentGroup] ??= _FocusTraversalGroupInfo(parentGroup, members: <FocusNode>[], defaultPolicy: defaultPolicy);
assert(!groups[parentGroup]!.members.contains(node));
groups[parentGroup]!.members.add(groupNode);
continue;
}
// Skip non-focusable and non-traversable nodes in the same way that
// FocusScopeNode.traversalDescendants would.
if (node.canRequestFocus && !node.skipTraversal) {
groups[groupNode] ??= _FocusTraversalGroupInfo(groupMarker, members: <FocusNode>[], defaultPolicy: defaultPolicy);
groups[groupNode] ??= _FocusTraversalGroupInfo(groupNode, members: <FocusNode>[], defaultPolicy: defaultPolicy);
assert(!groups[groupNode]!.members.contains(node));
groups[groupNode]!.members.add(node);
}
}
return groups;
}
// Sort all descendants, taking into account the FocusTraversalGroup
// that they are each in, and filtering out non-traversable/focusable nodes.
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);
// Sort the member lists using the individual policy sorts.
for (final FocusNode? key in groups.keys) {
......@@ -381,8 +379,8 @@ abstract class FocusTraversalPolicy with Diagnosticable {
}
// Visit the children of the scope, if any.
if (groups.isNotEmpty && groups.containsKey(scopeGroupMarker?.focusNode)) {
visitGroups(groups[scopeGroupMarker?.focusNode]!);
if (groups.isNotEmpty && groups.containsKey(scopeGroupNode)) {
visitGroups(groups[scopeGroupNode]!);
}
// Remove the FocusTraversalGroup nodes themselves, which aren't focusable.
......@@ -1556,56 +1554,107 @@ class FocusTraversalGroup extends StatefulWidget {
/// {@macro flutter.widgets.ProxyWidget.child}
final Widget child;
/// Returns the focus policy set by the [FocusTraversalGroup] that most
/// tightly encloses the given [BuildContext].
/// Returns the [FocusTraversalPolicy] that applies to the nearest ancestor of
/// the given [FocusNode].
///
/// It does not create a rebuild dependency because changing the traversal
/// order doesn't change the widget tree, so nothing needs to be rebuilt as a
/// result of an order change.
/// Will return null if no [FocusTraversalPolicy] ancestor applies to the
/// given [FocusNode].
///
/// Will assert if no [FocusTraversalGroup] ancestor is found.
/// The [FocusTraversalPolicy] is set by introducing a [FocusTraversalGroup]
/// into the widget tree, which will associate a policy with the focus tree
/// under the nearest ancestor [Focus] widget.
///
/// This function differs from [maybeOf] in that it takes a [FocusNode] and
/// only traverses the focus tree to determine the policy in effect. Unlike
/// this function, the [maybeOf] function takes a [BuildContext] and first
/// walks up the widget tree to find the nearest ancestor [Focus] or
/// [FocusScope] widget, and then calls this function with the focus node
/// associated with that widget to determine the policy in effect.
static FocusTraversalPolicy? maybeOfNode(FocusNode node) {
return _getGroupNode(node)?.policy;
}
static _FocusTraversalGroupNode? _getGroupNode(FocusNode node) {
while (node.parent != null) {
if (node.context == null) {
return null;
}
if (node is _FocusTraversalGroupNode) {
return node;
}
node = node.parent!;
}
return null;
}
/// Returns the [FocusTraversalPolicy] that applies to the [FocusNode] of the
/// nearest ancestor [Focus] widget, given a [BuildContext].
///
/// Will throw a [FlutterError] in debug mode, and throw a null check
/// exception in release mode, if no [Focus] ancestor is found, or if no
/// [FocusTraversalPolicy] applies to the associated [FocusNode].
///
/// {@template flutter.widgets.focus_traversal.FocusTraversalGroup.of}
/// This function looks up the nearest ancestor [Focus] (or [FocusScope])
/// widget, and uses its [FocusNode] (or [FocusScopeNode]) to walk up the
/// focus tree to find the applicable [FocusTraversalPolicy] for that node.
///
/// Calling this function does not create a rebuild dependency because
/// changing the traversal order doesn't change the widget tree, so nothing
/// needs to be rebuilt as a result of an order change.
///
/// The [FocusTraversalPolicy] is set by introducing a [FocusTraversalGroup]
/// into the widget tree, which will associate a policy with the focus tree
/// under the nearest ancestor [Focus] widget.
/// {@endtemplate}
///
/// See also:
///
/// * [maybeOf] for a similar function that will return null if no
/// [FocusTraversalGroup] ancestor is found.
/// * [maybeOfNode] for a function that will look for a policy using a given
/// [FocusNode], and return null if no policy applies.
static FocusTraversalPolicy of(BuildContext context) {
final _FocusTraversalGroupScope? inherited = context.dependOnInheritedWidgetOfExactType<_FocusTraversalGroupScope>();
final FocusTraversalPolicy? policy = maybeOf(context);
assert(() {
if (inherited == null) {
if (policy == null) {
throw FlutterError(
'Unable to find a FocusTraversalGroup widget in the context.\n'
'Unable to find a Focus or FocusScope widget in the given context, or the FocusNode '
'from with the widget that was found is not associated with a FocusTraversalPolicy.\n'
'FocusTraversalGroup.of() was called with a context that does not contain a '
'FocusTraversalGroup.\n'
'No FocusTraversalGroup ancestor could be found starting from the context that was '
'passed to FocusTraversalGroup.of(). This can happen because there is not a '
'WidgetsApp or MaterialApp widget (those widgets introduce a FocusTraversalGroup), '
'or it can happen if the context comes from a widget above those widgets.\n'
'Focus or FocusScope widget, or there was no FocusTraversalPolicy in effect.\n'
'This can happen if there is not a FocusTraversalGroup that defines the policy, '
'or if the context comes from a widget that is above the WidgetsApp, MaterialApp, '
'or CupertinoApp widget (those widgets introduce an implicit default policy) \n'
'The context used was:\n'
' $context',
);
}
return true;
}());
return inherited!.policy;
return policy!;
}
/// Returns the focus policy set by the [FocusTraversalGroup] that most
/// tightly encloses the given [BuildContext].
/// Returns the [FocusTraversalPolicy] that applies to the [FocusNode] of the
/// nearest ancestor [Focus] widget, or null, given a [BuildContext].
///
/// It does not create a rebuild dependency because changing the traversal
/// order doesn't change the widget tree, so nothing needs to be rebuilt as a
/// result of an order change.
/// Will return null if it doesn't find an ancestor [Focus] or [FocusScope]
/// widget, or doesn't find a [FocusTraversalPolicy] that applies to the node.
///
/// Will return null if it doesn't find a [FocusTraversalGroup] ancestor.
/// {@macro flutter.widgets.focus_traversal.FocusTraversalGroup.of}
///
/// See also:
///
/// * [of] for a similar function that will throw if no [FocusTraversalGroup]
/// ancestor is found.
/// * [maybeOfNode] for a similar function that will look for a policy using a
/// given [FocusNode].
/// * [of] for a similar function that will throw if no [FocusTraversalPolicy]
/// applies.
static FocusTraversalPolicy? maybeOf(BuildContext context) {
final _FocusTraversalGroupScope? inherited = context.dependOnInheritedWidgetOfExactType<_FocusTraversalGroupScope>();
return inherited?.policy;
final FocusNode? node = Focus.maybeOf(context, scopeOk: true, createDependency: false);
if (node == null) {
return null;
}
return FocusTraversalGroup.maybeOfNode(node);
}
@override
......@@ -1618,21 +1667,28 @@ class FocusTraversalGroup extends StatefulWidget {
}
}
// A special focus node subclass that only FocusTraversalGroup uses so that it
// can be used to cache the policy in the focus tree, and so that the traversal
// code can find groups in the focus tree.
class _FocusTraversalGroupNode extends FocusNode {
_FocusTraversalGroupNode({
super.debugLabel,
required this.policy,
});
FocusTraversalPolicy policy;
}
class _FocusTraversalGroupState extends State<FocusTraversalGroup> {
// The internal focus node used to collect the children of this node into a
// group, and to provide a context for the traversal algorithm to sort the
// group with.
late final FocusNode focusNode;
@override
void initState() {
super.initState();
focusNode = FocusNode(
canRequestFocus: false,
skipTraversal: true,
// group with. It's a special subclass of FocusNode just so that it can be
// identified when walking the focus tree during traversal, and hold the
// current policy.
late final _FocusTraversalGroupNode focusNode = _FocusTraversalGroupNode(
debugLabel: 'FocusTraversalGroup',
policy: widget.policy,
);
}
@override
void dispose() {
......@@ -1640,12 +1696,17 @@ class _FocusTraversalGroupState extends State<FocusTraversalGroup> {
super.dispose();
}
@override
void didUpdateWidget (FocusTraversalGroup oldWidget) {
super.didUpdateWidget(oldWidget);
if (oldWidget.policy != widget.policy) {
focusNode.policy = widget.policy;
}
}
@override
Widget build(BuildContext context) {
return _FocusTraversalGroupScope(
policy: widget.policy,
focusNode: focusNode,
child: Focus(
return Focus(
focusNode: focusNode,
canRequestFocus: false,
skipTraversal: true,
......@@ -1653,26 +1714,10 @@ class _FocusTraversalGroupState extends State<FocusTraversalGroup> {
descendantsAreFocusable: widget.descendantsAreFocusable,
descendantsAreTraversable: widget.descendantsAreTraversable,
child: widget.child,
),
);
}
}
// A "marker" inherited widget to make the group faster to find.
class _FocusTraversalGroupScope extends InheritedWidget {
const _FocusTraversalGroupScope({
required this.policy,
required this.focusNode,
required super.child,
});
final FocusTraversalPolicy policy;
final FocusNode focusNode;
@override
bool updateShouldNotify(InheritedWidget oldWidget) => false;
}
/// An intent for use with the [RequestFocusAction], which supplies the
/// [FocusNode] that should be focused.
class RequestFocusIntent extends Intent {
......
packages/flutter/lib/src/widgets/framework.dart
View file @ b1d8a4e4
......@@ -2255,10 +2255,10 @@ abstract class BuildContext {
/// the widget or one of its ancestors is moved (for example, because an
/// ancestor is added or removed).
///
/// The [aspect] parameter is only used when `T` is an [InheritedWidget]
/// subclass that supports partial updates, like [InheritedModel]. It
/// specifies what "aspect" of the inherited widget this context depends on,
/// where the meaning of the aspect is determined by the specific subclass.
/// The [aspect] parameter is only used when `T` is an
/// [InheritedWidget] subclasses that supports partial updates, like
/// [InheritedModel]. It specifies what "aspect" of the inherited
/// widget this context depends on.
/// {@endtemplate}
T? dependOnInheritedWidgetOfExactType<T extends InheritedWidget>({ Object? aspect });
......
packages/flutter/test/material/debug_test.dart
View file @ b1d8a4e4
......@@ -162,7 +162,6 @@ void main() {
' Focus\n'
' _FocusInheritedScope\n'
' Focus\n'
' _FocusTraversalGroupScope\n'
' FocusTraversalGroup\n'
' AbsorbPointer\n'
' Listener\n'
......@@ -201,7 +200,6 @@ void main() {
' TapRegionSurface\n'
' _FocusInheritedScope\n'
' Focus\n'
' _FocusTraversalGroupScope\n'
' FocusTraversalGroup\n'
' _ActionsScope\n'
' Actions\n'
......
packages/flutter/test/widgets/focus_traversal_test.dart
View file @ b1d8a4e4
......@@ -2201,6 +2201,48 @@ void main() {
expect(unfocusableNode.hasFocus, isFalse);
});
testWidgets('Group applies correct policy if focus tree is different from widget tree.', (WidgetTester tester) async {
final GlobalKey key1 = GlobalKey(debugLabel: '1');
final GlobalKey key2 = GlobalKey(debugLabel: '2');
final GlobalKey key3 = GlobalKey(debugLabel: '3');
final GlobalKey key4 = GlobalKey(debugLabel: '4');
final FocusNode focusNode = FocusNode(debugLabel: 'child');
final FocusNode parentFocusNode = FocusNode(debugLabel: 'parent');
await tester.pumpWidget(
Column(
children: <Widget>[
FocusTraversalGroup(
policy: WidgetOrderTraversalPolicy(),
child: Focus(
child: Focus.withExternalFocusNode(
key: key1,
// This makes focusNode be a child of parentFocusNode instead
// of the surrounding Focus.
parentNode: parentFocusNode,
focusNode: focusNode,
child: Container(key: key2),
),
),
),
FocusTraversalGroup(
policy: SkipAllButFirstAndLastPolicy(),
child: FocusScope(
child: Focus.withExternalFocusNode(
key: key3,
focusNode: parentFocusNode,
child: Container(key: key4),
),
),
),
],
),
);
expect(focusNode.parent, equals(parentFocusNode));
expect(FocusTraversalGroup.maybeOf(key2.currentContext!), const TypeMatcher<SkipAllButFirstAndLastPolicy>());
expect(FocusTraversalGroup.of(key2.currentContext!), const TypeMatcher<SkipAllButFirstAndLastPolicy>());
});
testWidgets("Descendants of FocusTraversalGroup aren't traversable if descendantsAreTraversable is false.", (WidgetTester tester) async {
final FocusNode node1 = FocusNode();
final FocusNode node2 = FocusNode();
......
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

HIAST - GitLab