diff --git a/packages/flutter/lib/src/widgets/focus_traversal.dart b/packages/flutter/lib/src/widgets/focus_traversal.dart index e89189408784..003672d174d7 100644 --- a/packages/flutter/lib/src/widgets/focus_traversal.dart +++ b/packages/flutter/lib/src/widgets/focus_traversal.dart @@ -496,11 +496,11 @@ class _DirectionalPolicyData { /// only want to implement new next/previous policies. /// /// Since hysteresis in the navigation order is undesirable, this implementation -/// maintains a stack of previous locations that have been visited on the -/// policy data for the affected [FocusScopeNode]. If the previous direction -/// was the opposite of the current direction, then the this policy will request -/// focus on the previously focused node. Change to another direction other than -/// the current one or its opposite will clear the stack. +/// maintains a stack of previous locations that have been visited on the policy +/// data for the affected [FocusScopeNode]. If the previous direction was the +/// opposite of the current direction, then the this policy will request focus +/// on the previously focused node. Change to another direction other than the +/// current one or its opposite will clear the stack. /// /// For instance, if the focus moves down, down, down, and then up, up, up, it /// will follow the same path through the widgets in both directions. However, @@ -508,17 +508,31 @@ class _DirectionalPolicyData { /// follow the same path on the way up as it did on the way down, since changing /// the axis of motion resets the history. /// +/// This class implements an algorithm that considers an infinite band extending +/// along the direction of movement, the width or height (depending on +/// direction) of the currently focused widget, and finds the closest widget in +/// that band along the direction of movement. If nothing is found in that band, +/// then it picks the widget with an edge closest to the band in the +/// perpendicular direction. If two out-of-band widgets are the same distance +/// from the band, then it picks the one closest along the direction of +/// movement. +/// +/// The goal of this algorithm is to pick a widget that (to the user) doesn't +/// appear to traverse along the wrong axis, as it might if it only sorted +/// widgets by distance along one axis, but also jumps to the next logical +/// widget in a direction without skipping over widgets. +/// /// See also: /// -/// * [FocusNode], for a description of the focus system. -/// * [FocusTraversalGroup], a widget that groups together and imposes a -/// traversal policy on the [Focus] nodes below it in the widget hierarchy. -/// * [WidgetOrderTraversalPolicy], a policy that relies on the widget -/// creation order to describe the order of traversal. -/// * [ReadingOrderTraversalPolicy], a policy that describes the order as the -/// natural "reading order" for the current [Directionality]. -/// * [OrderedTraversalPolicy], a policy that describes the order -/// explicitly using [FocusTraversalOrder] widgets. +/// * [FocusNode], for a description of the focus system. +/// * [FocusTraversalGroup], a widget that groups together and imposes a +/// traversal policy on the [Focus] nodes below it in the widget hierarchy. +/// * [WidgetOrderTraversalPolicy], a policy that relies on the widget creation +/// order to describe the order of traversal. +/// * [ReadingOrderTraversalPolicy], a policy that describes the order as the +/// natural "reading order" for the current [Directionality]. +/// * [OrderedTraversalPolicy], a policy that describes the order explicitly +/// using [FocusTraversalOrder] widgets. mixin DirectionalFocusTraversalPolicyMixin on FocusTraversalPolicy { final Map _policyData = {}; @@ -622,6 +636,54 @@ mixin DirectionalFocusTraversalPolicyMixin on FocusTraversalPolicy { return sorted; } + static int _verticalCompareClosestEdge(Offset target, Rect a, Rect b) { + // Find which edge is closest to the target for each. + final double aCoord = (a.top - target.dy).abs() < (a.bottom - target.dy).abs() ? a.top : a.bottom; + final double bCoord = (b.top - target.dy).abs() < (b.bottom - target.dy).abs() ? b.top : b.bottom; + return (aCoord - target.dy).abs().compareTo((bCoord - target.dy).abs()); + } + + static int _horizontalCompareClosestEdge(Offset target, Rect a, Rect b) { + // Find which edge is closest to the target for each. + final double aCoord = (a.left - target.dx).abs() < (a.right - target.dx).abs() ? a.left : a.right; + final double bCoord = (b.left - target.dx).abs() < (b.right - target.dx).abs() ? b.left : b.right; + return (aCoord - target.dx).abs().compareTo((bCoord - target.dx).abs()); + } + + // Sort the ones that have edges that are closest horizontally first, and if + // two are the same horizontal distance, pick the one that is closest + // vertically. + static Iterable _sortClosestEdgesByDistancePreferHorizontal(Offset target, Iterable nodes) { + final List sorted = nodes.toList(); + mergeSort(sorted, compare: (FocusNode nodeA, FocusNode nodeB) { + final int horizontal = _horizontalCompareClosestEdge(target, nodeA.rect, nodeB.rect); + if (horizontal == 0) { + // If they're the same distance horizontally, pick the closest one + // vertically. + return _verticalCompare(target, nodeA.rect.center, nodeB.rect.center); + } + return horizontal; + }); + return sorted; + } + + // Sort the ones that have edges that are closest vertically first, and if + // two are the same vertical distance, pick the one that is closest + // horizontally. + static Iterable _sortClosestEdgesByDistancePreferVertical(Offset target, Iterable nodes) { + final List sorted = nodes.toList(); + mergeSort(sorted, compare: (FocusNode nodeA, FocusNode nodeB) { + final int vertical = _verticalCompareClosestEdge(target, nodeA.rect, nodeB.rect); + if (vertical == 0) { + // If they're the same distance vertically, pick the closest one + // horizontally. + return _horizontalCompare(target, nodeA.rect.center, nodeB.rect.center); + } + return vertical; + }); + return sorted; + } + // Sorts nodes from left to right horizontally, and removes nodes that are // either to the right of the left side of the target node if we're going // left, or to the left of the right side of the target node if we're going @@ -843,8 +905,9 @@ mixin DirectionalFocusTraversalPolicyMixin on FocusTraversalPolicy { break; } // Only out-of-band targets are eligible, so pick the one that is - // closest the to the center line horizontally. - found = _sortByDistancePreferHorizontal(focusedChild.rect.center, eligibleNodes).first; + // closest to the center line horizontally, and if any are the same + // distance horizontally, pick the closest one of those vertically. + found = _sortClosestEdgesByDistancePreferHorizontal(focusedChild.rect.center, eligibleNodes).first; break; case TraversalDirection.right: case TraversalDirection.left: @@ -869,8 +932,9 @@ mixin DirectionalFocusTraversalPolicyMixin on FocusTraversalPolicy { break; } // Only out-of-band targets are eligible, so pick the one that is - // to the center line vertically. - found = _sortByDistancePreferVertical(focusedChild.rect.center, eligibleNodes).first; + // closest to the center line vertically, and if any are the same + // distance vertically, pick the closest one of those horizontally. + found = _sortClosestEdgesByDistancePreferVertical(focusedChild.rect.center, eligibleNodes).first; break; } if (found != null) { diff --git a/packages/flutter/test/widgets/focus_traversal_test.dart b/packages/flutter/test/widgets/focus_traversal_test.dart index 2e04bec05edd..62efd05efd4d 100644 --- a/packages/flutter/test/widgets/focus_traversal_test.dart +++ b/packages/flutter/test/widgets/focus_traversal_test.dart @@ -1585,6 +1585,178 @@ void main() { clear(); }); + testWidgets('Closest vertical is picked when only out of band items are considered', (WidgetTester tester) async { + const int rows = 4; + List focus = List.generate(rows, (int _) => null); + final List nodes = List.generate(rows, (int index) => FocusNode(debugLabel: 'Node $index')); + + Widget makeFocus(int row) { + return Padding( + padding: EdgeInsetsDirectional.only(end: row != 0 ? 110.0 : 0), + child: Focus( + focusNode: nodes[row], + onFocusChange: (bool isFocused) => focus[row] = isFocused, + child: Container( + width: row == 1 ? 150 : 100, + height: 100, + color: Colors.primaries[row], + child: Text('[$row]'), + ), + ), + ); + } + + /// Layout is: + /// [0] + /// [ 1] + /// [ 2] + /// [ 3] + /// + /// The important feature is that nothing is in the vertical band defined + /// by widget [0]. We want it to traverse to 1, 2, 3 in order, even though + /// the center of [2] is horizontally closer to the vertical axis of [0]'s + /// center than [1]'s. + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: FocusTraversalGroup( + policy: WidgetOrderTraversalPolicy(), + child: FocusScope( + debugLabel: 'Scope', + child: Column( + crossAxisAlignment: CrossAxisAlignment.end, + children: [ + makeFocus(0), + makeFocus(1), + makeFocus(2), + makeFocus(3), + ], + ), + ), + ), + ), + ); + + void clear() { + focus = List.generate(focus.length, (int _) => null); + } + + final FocusNode scope = nodes[0].enclosingScope!; + + // Go down the column and make sure that the focus goes to the next + // closest one. + nodes[0].requestFocus(); + await tester.pump(); + expect(focus, orderedEquals([true, null, null, null])); + clear(); + + expect(scope.focusInDirection(TraversalDirection.down), isTrue); + await tester.pump(); + expect(focus, orderedEquals([false, true, null, null])); + clear(); + + expect(scope.focusInDirection(TraversalDirection.down), isTrue); + await tester.pump(); + expect(focus, orderedEquals([null, false, true, null])); + clear(); + + expect(scope.focusInDirection(TraversalDirection.down), isTrue); + await tester.pump(); + expect(focus, orderedEquals([null, null, false, true])); + clear(); + + expect(scope.focusInDirection(TraversalDirection.down), isFalse); + await tester.pump(); + expect(focus, orderedEquals([null, null, null, null])); + clear(); + }); + + testWidgets('Closest horizontal is picked when only out of band items are considered', (WidgetTester tester) async { + const int cols = 4; + List focus = List.generate(cols, (int _) => null); + final List nodes = List.generate(cols, (int index) => FocusNode(debugLabel: 'Node $index')); + + Widget makeFocus(int col) { + return Padding( + padding: EdgeInsetsDirectional.only(top: col != 0 ? 110.0 : 0), + child: Focus( + focusNode: nodes[col], + onFocusChange: (bool isFocused) => focus[col] = isFocused, + child: Container( + width: 100, + height: col == 1 ? 150 : 100, + color: Colors.primaries[col], + child: Text('[$col]'), + ), + ), + ); + } + + /// Layout is: + /// [0] + /// [ ][2][3] + /// [1] + /// ([ ] is part of [1], [1] is just taller than [2] and [3]). + /// + /// The important feature is that nothing is in the horizontal band + /// defined by widget [0]. We want it to traverse to 1, 2, 3 in order, + /// even though the center of [2] is vertically closer to the horizontal + /// axis of [0]'s center than [1]'s. + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: FocusTraversalGroup( + policy: WidgetOrderTraversalPolicy(), + child: FocusScope( + debugLabel: 'Scope', + child: Row( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + makeFocus(0), + makeFocus(1), + makeFocus(2), + makeFocus(3), + ], + ), + ), + ), + ), + ); + + void clear() { + focus = List.generate(focus.length, (int _) => null); + } + + final FocusNode scope = nodes[0].enclosingScope!; + + // Go down the row and make sure that the focus goes to the next + // closest one. + nodes[0].requestFocus(); + await tester.pump(); + expect(focus, orderedEquals([true, null, null, null])); + clear(); + + expect(scope.focusInDirection(TraversalDirection.right), isTrue); + await tester.pump(); + expect(focus, orderedEquals([false, true, null, null])); + clear(); + + expect(scope.focusInDirection(TraversalDirection.right), isTrue); + await tester.pump(); + expect(focus, orderedEquals([null, false, true, null])); + clear(); + + expect(scope.focusInDirection(TraversalDirection.right), isTrue); + await tester.pump(); + expect(focus, orderedEquals([null, null, false, true])); + clear(); + + expect(scope.focusInDirection(TraversalDirection.right), isFalse); + await tester.pump(); + expect(focus, orderedEquals([null, null, null, null])); + clear(); + }); + testWidgets('Can find first focus in all directions.', (WidgetTester tester) async { final GlobalKey upperLeftKey = GlobalKey(debugLabel: 'upperLeftKey'); final GlobalKey upperRightKey = GlobalKey(debugLabel: 'upperRightKey');