Skip to content

Conversation

@gspencergoog
Copy link
Contributor

@gspencergoog gspencergoog commented Mar 13, 2023

Description

This updates the directional traversal algorithm so that if you traverse down (or any other direction), and there isn't a widget below in the band created by the infinite height box the width of the currently focused widget, then we prefer the widget with one side closest to the band, and if there are two the same distance from the band, then we prefer the closest one vertically.

This layout used to do the wrong thing when you went down, preferring the one whose center was closest to the band, rather than the closest side:

before.mp4

And here is the new code:

after.mp4

Tests

  • Added tests for both horizontal and vertical versions of the issue.

@flutter-dashboard flutter-dashboard bot added f: focus Focus traversal, gaining or losing focus framework flutter/packages/flutter repository. See also f: labels. labels Mar 13, 2023
Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any user-facing documentation that discusses the navigation key traversal behaviour? If so, does it already document this behaviour as now implemented?

Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the accessibility traversal algorithm need to be updated too?

@gspencergoog
Copy link
Contributor Author

gspencergoog commented Mar 13, 2023

Does the accessibility traversal algorithm need to be updated too?

Accessibility traversal order in a direction is handled by the screen reader, we don't control that.

Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I'm not very familiar with this area :)

@gspencergoog
Copy link
Contributor Author

There is some discussion here, but it doesn't go into that much detail. I'll try and add some detail, but I worry a little that it might just be too much detail and their eyes will glaze over. :-)

@gspencergoog gspencergoog force-pushed the fix_directional_traversal branch from 5c3cd3b to b609428 Compare March 13, 2023 22:37
@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 13, 2023
@auto-submit auto-submit bot merged commit a05f21f into flutter:master Mar 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App f: focus Focus traversal, gaining or losing focus framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants