Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tweak directional focus traversal #116230

Merged
merged 8 commits into from Dec 1, 2022
Merged

Tweak directional focus traversal #116230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0a02963
Use distance instead of coord
gspencergoog Nov 29, 2022
5b3b71c
Sort by distance prefer axis
gspencergoog Nov 29, 2022
6d07dd4
Switch initial sort back to sort by coordinate.
gspencergoog Nov 29, 2022
9042958
revert test change
gspencergoog Nov 29, 2022
00f3192
Fix tests
gspencergoog Nov 29, 2022
f4743bd
Simplify test case
gspencergoog Nov 29, 2022
4f1f136
Add a test for irregular grids
gspencergoog Nov 29, 2022
d0f4fb4
Review Changes
gspencergoog Dec 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
145 changes: 84 additions & 61 deletions packages/flutter/lib/src/widgets/focus_traversal.dart
View file
Open in desktop
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