Unverified Commit 8ef5e2f0 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Add OrderedFocusTraversalPolicy and FocusTraversalGroup to all… (#49235)

This change adds a way to provide explicit focus order for a part of the widget tree.

It adds FocusTraversalPolicyGroup, which in many ways is similar to DefaultFocusTraversal, except that it groups a widget subtree together so that those nodes are traversed as a group. DefaultFocusTraversal doesn't work as one would expect: If there is more than one DefaultFocusTraversal inside of a focus scope, the policy can change depending on which node was asked to move "next", which can cause unexpected behavior. The new grouping mechanism doesn't have that problem. I deprecate DefaultFocusTraversal in this PR.

It also adds OrderedFocusTraversalPolicy, which is a policy that can be supplied to FocusTraversalPolicyGroup to set the policy for a sub-tree. It looks for FocusTraversalOrder inherited widgets, which use a FocusOrder to do the sorting. FocusOrder has two subclasses: NumericalFocusOrder (which sorts based on a double), and LexicalFocusOrder, which sorts based on a String.

As part of doing this, I refactored the way FocusTraversalPolicy is implemented so that it has more default implementation methods, and exposes a new protected member: sortDescendants, which makes it easier for developers to make their own policy subclasses: they only need to implement sortDescendants to get a new ordering behavior, but can also still override any of the default implementation behaviors if they need different behavior.

I was able to do this without breaking the API (AFAICT).
parent f769bcc5
......@@ -435,7 +435,7 @@ class _FocusDemoState extends State<FocusDemo> {
kUndoActionKey: () => kUndoAction,
kRedoActionKey: () => kRedoAction,
},
child: DefaultFocusTraversal(
child: FocusTraversalGroup(
policy: ReadingOrderTraversalPolicy(),
child: Shortcuts(
shortcuts: <LogicalKeySet, Intent>{
......
......@@ -142,7 +142,7 @@ class _FocusDemoState extends State<FocusDemo> {
Widget build(BuildContext context) {
final TextTheme textTheme = Theme.of(context).textTheme;
return DefaultFocusTraversal(
return FocusTraversalGroup(
policy: ReadingOrderTraversalPolicy(),
child: FocusScope(
debugLabel: 'Scope',
......
......@@ -1395,7 +1395,7 @@ class _WidgetsAppState extends State<WidgetsApp> with WidgetsBindingObserver {
debugLabel: '<Default WidgetsApp Shortcuts>',
child: Actions(
actions: widget.actions ?? WidgetsApp.defaultActions,
child: DefaultFocusTraversal(
child: FocusTraversalGroup(
policy: ReadingOrderTraversalPolicy(),
child: _MediaQueryFromWindow(
child: Localizations(
......
......@@ -230,12 +230,12 @@ class FocusAttachment {
/// particular direction, is determined by the [FocusTraversalPolicy] in force.
///
/// The ambient policy is determined by looking up the widget hierarchy for a
/// [DefaultFocusTraversal] widget, and obtaining the focus traversal policy
/// [FocusTraversalGroup] widget, and obtaining the focus traversal policy
/// from it. Different focus nodes can inherit difference policies, so part of
/// the app can go in widget order, and part can go in reading order, depending
/// upon the use case.
///
/// Predefined policies include [WidgetOrderFocusTraversalPolicy],
/// Predefined policies include [WidgetOrderTraversalPolicy],
/// [ReadingOrderTraversalPolicy], and [DirectionalFocusTraversalPolicyMixin],
/// but custom policies can be built based upon these policies.
///
......@@ -361,8 +361,8 @@ class FocusAttachment {
/// events to focused nodes.
/// * [FocusTraversalPolicy], a class used to determine how to move the focus
/// to other nodes.
/// * [DefaultFocusTraversal], a widget used to configure the default focus
/// traversal policy for a widget subtree.
/// * [FocusTraversalGroup], a widget used to group together and configure the
/// focus traversal policy for a widget subtree.
class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
/// Creates a focus node.
///
......@@ -426,8 +426,8 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
///
/// See also:
///
/// * [DefaultFocusTraversal], a widget that sets the traversal policy for
/// its descendants.
/// * [FocusTraversalGroup], a widget used to group together and configure the
/// focus traversal policy for a widget subtree.
/// * [FocusTraversalPolicy], a class that can be extended to describe a
/// traversal policy.
bool get canRequestFocus {
......@@ -518,7 +518,8 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
return _descendants;
}
/// Returns all descendants which do not have the [skipTraversal] flag set.
/// Returns all descendants which do not have the [skipTraversal] and do have
/// the [canRequestFocus] flag set.
Iterable<FocusNode> get traversalDescendants => descendants.where((FocusNode node) => !node.skipTraversal && node.canRequestFocus);
/// An [Iterable] over the ancestors of this node.
......@@ -776,7 +777,7 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
_manager?.primaryFocus?._setAsFocusedChild();
}
if (oldScope != null && child.context != null && child.enclosingScope != oldScope) {
DefaultFocusTraversal.of(child.context, nullOk: true)?.changedScope(node: child, oldScope: oldScope);
FocusTraversalGroup.of(child.context, nullOk: true)?.changedScope(node: child, oldScope: oldScope);
}
if (child._requestFocusWhenReparented) {
child._doRequestFocus();
......@@ -915,19 +916,19 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
/// [FocusTraversalPolicy.next] method.
///
/// Returns true if it successfully found a node and requested focus.
bool nextFocus() => DefaultFocusTraversal.of(context).next(this);
bool nextFocus() => FocusTraversalGroup.of(context).next(this);
/// Request to move the focus to the previous focus node, by calling the
/// [FocusTraversalPolicy.previous] method.
///
/// Returns true if it successfully found a node and requested focus.
bool previousFocus() => DefaultFocusTraversal.of(context).previous(this);
bool previousFocus() => FocusTraversalGroup.of(context).previous(this);
/// Request to move the focus to the nearest focus node in the given
/// direction, by calling the [FocusTraversalPolicy.inDirection] method.
///
/// Returns true if it successfully found a node and requested focus.
bool focusInDirection(TraversalDirection direction) => DefaultFocusTraversal.of(context).inDirection(this, direction);
bool focusInDirection(TraversalDirection direction) => FocusTraversalGroup.of(context).inDirection(this, direction);
@override
void debugFillProperties(DiagnosticPropertiesBuilder properties) {
......
......@@ -462,8 +462,10 @@ class _FocusState extends State<Focus> {
return _FocusMarker(
node: focusNode,
child: Semantics(
focusable: _canRequestFocus,
focused: _hasPrimaryFocus,
// If these values are false, then just don't set them, so they don't
// eclipse values set by children.
focusable: _canRequestFocus ? true : null,
focused: _hasPrimaryFocus ? true : null,
child: widget.child,
),
);
......
......@@ -2191,6 +2191,8 @@ abstract class BuildContext {
/// Obtains the element corresponding to the nearest widget of the given type [T],
/// which must be the type of a concrete [InheritedWidget] subclass.
///
/// Returns null if no such element is found.
///
/// Calling this method is O(1) with a small constant factor.
///
/// This method does not establish a relationship with the target in the way
......
......@@ -68,7 +68,7 @@ void main() {
),
));
expect(tester.getSemantics(find.byType(Focus).last), matchesSemantics(
expect(tester.getSemantics(find.byType(Focus)), matchesSemantics(
hasCheckedState: true,
hasEnabledState: true,
isEnabled: true,
......@@ -83,7 +83,7 @@ void main() {
),
));
expect(tester.getSemantics(find.byType(Focus).last), matchesSemantics(
expect(tester.getSemantics(find.byType(Focus)), matchesSemantics(
hasCheckedState: true,
hasEnabledState: true,
isChecked: true,
......@@ -99,7 +99,7 @@ void main() {
),
));
expect(tester.getSemantics(find.byType(Focus).last), matchesSemantics(
expect(tester.getSemantics(find.byWidgetPredicate((Widget widget) => widget.runtimeType.toString() == '_CheckboxRenderObjectWidget')), matchesSemantics(
hasCheckedState: true,
hasEnabledState: true,
));
......@@ -111,7 +111,7 @@ void main() {
),
));
expect(tester.getSemantics(find.byType(Focus).last), matchesSemantics(
expect(tester.getSemantics(find.byWidgetPredicate((Widget widget) => widget.runtimeType.toString() == '_CheckboxRenderObjectWidget')), matchesSemantics(
hasCheckedState: true,
hasEnabledState: true,
isChecked: true,
......
......@@ -116,7 +116,7 @@ void main() {
' The ancestors of this widget were:\n'
' Semantics\n'
' Builder\n'
' RepaintBoundary-[GlobalKey#2d465]\n'
' RepaintBoundary-[GlobalKey#00000]\n'
' IgnorePointer\n'
' AnimatedBuilder\n'
' FadeTransition\n'
......@@ -131,19 +131,19 @@ void main() {
' PageStorage\n'
' Offstage\n'
' _ModalScopeStatus\n'
' _ModalScope<dynamic>-[LabeledGlobalKey<_ModalScopeState<dynamic>>#969b7]\n'
' _ModalScope<dynamic>-[LabeledGlobalKey<_ModalScopeState<dynamic>>#00000]\n'
' _EffectiveTickerMode\n'
' TickerMode\n'
' _OverlayEntryWidget-[LabeledGlobalKey<_OverlayEntryWidgetState>#545d0]\n'
' _OverlayEntryWidget-[LabeledGlobalKey<_OverlayEntryWidgetState>#00000]\n'
' _Theatre\n'
' Overlay-[LabeledGlobalKey<OverlayState>#31a52]\n'
' Overlay-[LabeledGlobalKey<OverlayState>#00000]\n'
' _FocusMarker\n'
' Semantics\n'
' FocusScope\n'
' AbsorbPointer\n'
' _PointerListener\n'
' Listener\n'
' Navigator-[GlobalObjectKey<NavigatorState> _WidgetsAppState#10579]\n'
' Navigator-[GlobalObjectKey<NavigatorState> _WidgetsAppState#00000]\n'
' IconTheme\n'
' IconTheme\n'
' _InheritedCupertinoTheme\n'
......@@ -158,19 +158,23 @@ void main() {
' CheckedModeBanner\n'
' Title\n'
' Directionality\n'
' _LocalizationsScope-[GlobalKey#a51e3]\n'
' _LocalizationsScope-[GlobalKey#00000]\n'
' Semantics\n'
' Localizations\n'
' MediaQuery\n'
' _MediaQueryFromWindow\n'
' DefaultFocusTraversal\n'
' Semantics\n'
' _FocusMarker\n'
' Focus\n'
' _FocusTraversalGroupMarker\n'
' FocusTraversalGroup\n'
' Actions\n'
' _ShortcutsMarker\n'
' Semantics\n'
' _FocusMarker\n'
' Focus\n'
' Shortcuts\n'
' WidgetsApp-[GlobalObjectKey _MaterialAppState#38e79]\n'
' WidgetsApp-[GlobalObjectKey _MaterialAppState#00000]\n'
' ScrollConfiguration\n'
' MaterialApp\n'
' [root]\n'
......
......@@ -570,7 +570,7 @@ void main() {
}
Widget wrap({ Widget child }) {
return DefaultFocusTraversal(
return FocusTraversalGroup(
policy: ReadingOrderTraversalPolicy(),
child: Directionality(
textDirection: TextDirection.ltr,
......
......@@ -598,7 +598,7 @@ void main() {
// This checks both FocusScopes that have their own nodes, as well as those
// that use external nodes.
await tester.pumpWidget(
DefaultFocusTraversal(
FocusTraversalGroup(
child: Column(
children: <Widget>[
FocusScope(
......@@ -661,7 +661,7 @@ void main() {
final FocusScopeNode parentFocusScope2 = FocusScopeNode(debugLabel: 'Parent Scope 2');
await tester.pumpWidget(
DefaultFocusTraversal(
FocusTraversalGroup(
child: Column(
children: <Widget>[
FocusScope(
......@@ -711,7 +711,7 @@ void main() {
expect(find.text('b'), findsOneWidget);
await tester.pumpWidget(
DefaultFocusTraversal(
FocusTraversalGroup(
child: Column(
children: <Widget>[
FocusScope(
......@@ -746,7 +746,7 @@ void main() {
final FocusScopeNode parentFocusScope2 = FocusScopeNode(debugLabel: 'Parent Scope 2');
await tester.pumpWidget(
DefaultFocusTraversal(
FocusTraversalGroup(
child: Column(
children: <Widget>[
FocusScope(
......@@ -794,7 +794,7 @@ void main() {
expect(find.text('b'), findsOneWidget);
await tester.pumpWidget(
DefaultFocusTraversal(
FocusTraversalGroup(
child: Column(
children: <Widget>[
FocusScope(
......
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