Skip to content

Commit

Permalink
Tweak directional focus traversal (flutter#116230)
Browse files Browse the repository at this point in the history 
* Use distance instead of coord

* Sort by distance prefer axis

* Switch initial sort back to sort by coordinate.

* revert test change

* Fix tests

* Simplify test case

* Add a test for irregular grids

* Review Changes
  • Loading branch information
@gspencergoog
gspencergoog committed Jan 19, 2023
1 parent 7a8dd2c commit 5c0d07e
Show file tree
Hide file tree
Showing 2 changed files with 252 additions and 112 deletions.
145 changes: 84 additions & 61 deletions packages/flutter/lib/src/widgets/focus_traversal.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,6 @@ enum TraversalDirection {
/// This direction is unaffected by the [Directionality] of the current
/// context.
left,

// TODO(gspencer): Add diagonal traversal directions used by TV remotes and
// game controllers when we support them.
}

/// An object used to specify a focus traversal policy used for configuring a
Expand Down Expand Up @@ -547,6 +544,46 @@ mixin DirectionalFocusTraversalPolicyMixin on FocusTraversalPolicy {
return null;
}

static int _verticalCompare(Offset target, Offset a, Offset b) {
return (a.dy - target.dy).abs().compareTo((b.dy - target.dy).abs());
}

static int _horizontalCompare(Offset target, Offset a, Offset b) {
return (a.dx - target.dx).abs().compareTo((b.dx - target.dx).abs());
}

// Sort the ones that are closest to target vertically first, and if two are
// the same vertical distance, pick the one that is closest horizontally.
static Iterable<FocusNode> _sortByDistancePreferVertical(Offset target, Iterable<FocusNode> nodes) {
final List<FocusNode> sorted = nodes.toList();
mergeSort<FocusNode>(sorted, compare: (FocusNode nodeA, FocusNode nodeB) {
final Offset a = nodeA.rect.center;
final Offset b = nodeB.rect.center;
final int vertical = _verticalCompare(target, a, b);
if (vertical == 0) {
return _horizontalCompare(target, a, b);
}
return vertical;
});
return sorted;
}

// Sort the ones that are closest horizontally first, and if two are the same
// horizontal distance, pick the one that is closest vertically.
static Iterable<FocusNode> _sortByDistancePreferHorizontal(Offset target, Iterable<FocusNode> nodes) {
final List<FocusNode> sorted = nodes.toList();
mergeSort<FocusNode>(sorted, compare: (FocusNode nodeA, FocusNode nodeB) {
final Offset a = nodeA.rect.center;
final Offset b = nodeB.rect.center;
final int horizontal = _horizontalCompare(target, a, b);
if (horizontal == 0) {
return _verticalCompare(target, a, b);
}
return horizontal;
});
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
Expand All @@ -555,52 +592,54 @@ mixin DirectionalFocusTraversalPolicyMixin on FocusTraversalPolicy {
// This doesn't need to take into account directionality because it is
// typically intending to actually go left or right, not in a reading
// direction.
Iterable<FocusNode>? _sortAndFilterHorizontally(
Iterable<FocusNode> _sortAndFilterHorizontally(
TraversalDirection direction,
Rect target,
FocusNode nearestScope,
Iterable<FocusNode> nodes,
) {
assert(direction == TraversalDirection.left || direction == TraversalDirection.right);
final Iterable<FocusNode> nodes = nearestScope.traversalDescendants;
assert(!nodes.contains(nearestScope));
final List<FocusNode> sorted = nodes.toList();
mergeSort<FocusNode>(sorted, compare: (FocusNode a, FocusNode b) => a.rect.center.dx.compareTo(b.rect.center.dx));
Iterable<FocusNode>? result;
final Iterable<FocusNode> filtered;
switch (direction) {
case TraversalDirection.left:
result = sorted.where((FocusNode node) => node.rect != target && node.rect.center.dx <= target.left);
filtered = nodes.where((FocusNode node) => node.rect != target && node.rect.center.dx <= target.left);
break;
case TraversalDirection.right:
result = sorted.where((FocusNode node) => node.rect != target && node.rect.center.dx >= target.right);
filtered = nodes.where((FocusNode node) => node.rect != target && node.rect.center.dx >= target.right);
break;
case TraversalDirection.up:
case TraversalDirection.down:
break;
throw ArgumentError('Invalid direction $direction');
}
return result;
final List<FocusNode> sorted = filtered.toList();
// Sort all nodes from left to right.
mergeSort<FocusNode>(sorted, compare: (FocusNode a, FocusNode b) => a.rect.center.dx.compareTo(b.rect.center.dx));
return sorted;
}

// Sorts nodes from top to bottom vertically, and removes nodes that are
// either below the top of the target node if we're going up, or above the
// bottom of the target node if we're going down.
Iterable<FocusNode>? _sortAndFilterVertically(
Iterable<FocusNode> _sortAndFilterVertically(
TraversalDirection direction,
Rect target,
Iterable<FocusNode> nodes,
) {
final List<FocusNode> sorted = nodes.toList();
mergeSort<FocusNode>(sorted, compare: (FocusNode a, FocusNode b) => a.rect.center.dy.compareTo(b.rect.center.dy));
assert(direction == TraversalDirection.up || direction == TraversalDirection.down);
final Iterable<FocusNode> filtered;
switch (direction) {
case TraversalDirection.up:
return sorted.where((FocusNode node) => node.rect != target && node.rect.center.dy <= target.top);
filtered = nodes.where((FocusNode node) => node.rect != target && node.rect.center.dy <= target.top);
break;
case TraversalDirection.down:
return sorted.where((FocusNode node) => node.rect != target && node.rect.center.dy >= target.bottom);
filtered = nodes.where((FocusNode node) => node.rect != target && node.rect.center.dy >= target.bottom);
break;
case TraversalDirection.left:
case TraversalDirection.right:
break;
throw ArgumentError('Invalid direction $direction');
}
assert(direction == TraversalDirection.up || direction == TraversalDirection.down);
return null;
final List<FocusNode> sorted = filtered.toList();
mergeSort<FocusNode>(sorted, compare: (FocusNode a, FocusNode b) => a.rect.center.dy.compareTo(b.rect.center.dy));
return sorted;
}

// Updates the policy data to keep the previously visited node so that we can
Expand Down Expand Up @@ -745,71 +784,55 @@ mixin DirectionalFocusTraversalPolicyMixin on FocusTraversalPolicy {
switch (direction) {
case TraversalDirection.down:
case TraversalDirection.up:
Iterable<FocusNode>? eligibleNodes = _sortAndFilterVertically(
direction,
focusedChild.rect,
nearestScope.traversalDescendants,
);
Iterable<FocusNode> eligibleNodes = _sortAndFilterVertically(direction, focusedChild.rect, nearestScope.traversalDescendants);
if (eligibleNodes.isEmpty) {
break;
}
if (focusedScrollable != null && !focusedScrollable.position.atEdge) {
final Iterable<FocusNode> filteredEligibleNodes = eligibleNodes!.where((FocusNode node) => Scrollable.maybeOf(node.context!) == focusedScrollable);
final Iterable<FocusNode> filteredEligibleNodes = eligibleNodes.where((FocusNode node) => Scrollable.maybeOf(node.context!) == focusedScrollable);
if (filteredEligibleNodes.isNotEmpty) {
eligibleNodes = filteredEligibleNodes;
}
}
if (eligibleNodes!.isEmpty) {
break;
}
List<FocusNode> sorted = eligibleNodes.toList();
if (direction == TraversalDirection.up) {
sorted = sorted.reversed.toList();
eligibleNodes = eligibleNodes.toList().reversed;
}
// Find any nodes that intersect the band of the focused child.
final Rect band = Rect.fromLTRB(focusedChild.rect.left, -double.infinity, focusedChild.rect.right, double.infinity);
final Iterable<FocusNode> inBand = sorted.where((FocusNode node) => !node.rect.intersect(band).isEmpty);
final Iterable<FocusNode> inBand = eligibleNodes.where((FocusNode node) => !node.rect.intersect(band).isEmpty);
if (inBand.isNotEmpty) {
// The inBand list is already sorted by horizontal distance, so pick
// the closest one.
found = inBand.first;
found = _sortByDistancePreferVertical(focusedChild.rect.center, inBand).first;
break;
}
// Only out-of-band targets remain, so pick the one that is closest the
// to the center line horizontally.
mergeSort<FocusNode>(sorted, compare: (FocusNode a, FocusNode b) {
return (a.rect.center.dx - focusedChild.rect.center.dx).abs().compareTo((b.rect.center.dx - focusedChild.rect.center.dx).abs());
});
found = sorted.first;
// 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;
break;
case TraversalDirection.right:
case TraversalDirection.left:
Iterable<FocusNode>? eligibleNodes = _sortAndFilterHorizontally(direction, focusedChild.rect, nearestScope);
Iterable<FocusNode> eligibleNodes = _sortAndFilterHorizontally(direction, focusedChild.rect, nearestScope.traversalDescendants);
if (eligibleNodes.isEmpty) {
break;
}
if (focusedScrollable != null && !focusedScrollable.position.atEdge) {
final Iterable<FocusNode> filteredEligibleNodes = eligibleNodes!.where((FocusNode node) => Scrollable.maybeOf(node.context!) == focusedScrollable);
final Iterable<FocusNode> filteredEligibleNodes = eligibleNodes.where((FocusNode node) => Scrollable.maybeOf(node.context!) == focusedScrollable);
if (filteredEligibleNodes.isNotEmpty) {
eligibleNodes = filteredEligibleNodes;
}
}
if (eligibleNodes!.isEmpty) {
break;
}
List<FocusNode> sorted = eligibleNodes.toList();
if (direction == TraversalDirection.left) {
sorted = sorted.reversed.toList();
eligibleNodes = eligibleNodes.toList().reversed;
}
// Find any nodes that intersect the band of the focused child.
final Rect band = Rect.fromLTRB(-double.infinity, focusedChild.rect.top, double.infinity, focusedChild.rect.bottom);
final Iterable<FocusNode> inBand = sorted.where((FocusNode node) => !node.rect.intersect(band).isEmpty);
final Iterable<FocusNode> inBand = eligibleNodes.where((FocusNode node) => !node.rect.intersect(band).isEmpty);
if (inBand.isNotEmpty) {
// The inBand list is already sorted by vertical distance, so pick the
// closest one.
found = inBand.first;
found = _sortByDistancePreferHorizontal(focusedChild.rect.center, inBand).first;
break;
}
// Only out-of-band targets remain, so pick the one that is closest the
// Only out-of-band targets are eligible, so pick the one that is
// to the center line vertically.
mergeSort<FocusNode>(sorted, compare: (FocusNode a, FocusNode b) {
return (a.rect.center.dy - focusedChild.rect.center.dy).abs().compareTo((b.rect.center.dy - focusedChild.rect.center.dy).abs());
});
found = sorted.first;
found = _sortByDistancePreferVertical(focusedChild.rect.center, eligibleNodes).first;
break;
}
if (found != null) {
Expand Down Expand Up @@ -892,8 +915,8 @@ class _ReadingOrderSortData with Diagnosticable {
}
if (common!.isEmpty) {
// If there is no common ancestor, then arbitrarily pick the
// directionality of the first group, which is the equivalent of the "first
// strongly typed" item in a bidi algorithm.
// directionality of the first group, which is the equivalent of the
// "first strongly typed" item in a bidirectional algorithm.
return list.first.directionality;
}
// Find the closest common ancestor. The memberAncestors list contains the
Expand Down
Loading

0 comments on commit 5c0d07e

Please sign in to comment.