Skip to content

Conversation

@gspencergoog
Copy link
Contributor

@gspencergoog gspencergoog commented Sep 10, 2019

Description

This adds the default shortcuts and actions for keyboard-based focus traversal of apps.

This list of shortcuts includes shortcuts for TAB, SHIFT TAB, RIGHT_ARROW, LEFT_ARROW, UP_ARROW, DOWN_ARROW, and the four DPAD keys for game controllers (because the DPAD produces arrow key events).

It doesn't yet include functionality for triggering a control (e.g. SPACE, ENTER, or controller buttons), because that involves restructuring some of the Flutter controls to trigger animations differently, and so will be done in another PR (#41220)

Related Issues

Fixes #13264
Fixes #31957

Tests

  • Added tests to make sure that navigation works by simulating keystrokes. (not yet complete)

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.
  • I checked all the boxes!

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • No, this is not a breaking change.

@fluttergithubbot fluttergithubbot added framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Sep 10, 2019
@flutter flutter deleted a comment from fluttergithubbot Sep 10, 2019
@gspencergoog
Copy link
Contributor Author

This needs tests, so I'm not going to move it out of "draft" until I'm done with those.

@gspencergoog gspencergoog force-pushed the focus_shortcuts branch 5 times, most recently from cfb2a40 to 0fa0075 Compare September 24, 2019 15:20
@gspencergoog gspencergoog force-pushed the focus_shortcuts branch 3 times, most recently from 0a72c31 to ace31de Compare September 25, 2019 21:54
@goderbauer goderbauer added the a: desktop Running on desktop label Sep 25, 2019
@gspencergoog gspencergoog marked this pull request as ready for review September 25, 2019 22:30
@mdebbar mdebbar mentioned this pull request Sep 26, 2019
8 tasks
Copy link
Contributor

@darrenaustin darrenaustin left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a copy/paste oversight? How is the initial focus happening again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, for some reason I CAN'T READ. Removed all of the copies of that, and fixed up the first one (which was the only place that comment belonged).

@gspencergoog gspencergoog force-pushed the focus_shortcuts branch 2 times, most recently from 55f57c2 to a22fbaf Compare October 1, 2019 21:30
@gspencergoog gspencergoog merged commit bedf46d into flutter:master Oct 2, 2019
@gspencergoog gspencergoog deleted the focus_shortcuts branch October 9, 2019 18:12
gspencergoog added a commit that referenced this pull request Oct 10, 2019
This attempts to reland #40186 and #41220, that were reverted in #41945.

The main modifications from the original PRs are that I predefine the shortcuts and actions maps instead of defining them inline in the build function, and I use a new mapEquals to do a deep comparison so that we don't rebuild modified things if the contents of the map haven't changed.

I also eliminated an operator== and hashCode that were defined on the Actions widget, since widgets shouldn't have those. (it's too bad though: I get an 85% speedup if we leave this in! Too bad it prevents rebuilding of the children...)

Fixes #40101
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
This adds the default shortcuts and actions for keyboard-based focus traversal of apps.

This list of shortcuts includes shortcuts for TAB, SHIFT TAB, RIGHT_ARROW, LEFT_ARROW, UP_ARROW, DOWN_ARROW, and the four DPAD keys for game controllers (because the DPAD produces arrow key events).

It doesn't yet include functionality for triggering a control (e.g. SPACE, ENTER, or controller buttons), because that involves restructuring some of the Flutter controls to trigger animations differently, and so will be done in another PR (flutter#41220)
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
This attempts to reland flutter#40186 and flutter#41220, that were reverted in flutter#41945.

The main modifications from the original PRs are that I predefine the shortcuts and actions maps instead of defining them inline in the build function, and I use a new mapEquals to do a deep comparison so that we don't rebuild modified things if the contents of the map haven't changed.

I also eliminated an operator== and hashCode that were defined on the Actions widget, since widgets shouldn't have those. (it's too bad though: I get an 85% speedup if we leave this in! Too bad it prevents rebuilding of the children...)

Fixes flutter#40101
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a: desktop Running on desktop c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Game controller and DPAD navigation should work Users should be able to navigate around the app using keyboard

5 participants