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.

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

Directional navigation key binding defaults should be limited to those platforms that use it. #54998

Open
gmoothart opened this issue Apr 16, 2020 · 16 comments
Labels
a: desktop Running on desktop d: devtools DevTools related - suite of performance and debugging tools framework flutter/packages/flutter repository. See also f: labels. fyi-macos For the attention of macOS platform team P2 Important issues not at the top of the work list platform-mac Building on or for macOS specifically team-text-input Owned by Text Input team

Comments

@gmoothart
Copy link
Contributor

When running the fluter devtools on macos, I'm seeing strange key handling behavior. There are extra key actions that I don't see on flutter web. Some discussion of the issue, with screenshots, is at flutter/devtools#1769. But essentially:

I'm adding row selection and keyboard navigation to a custom widget TreeTable (built here). I'm explicitly handling the arrow up/down keys to move the selection and scroll the view. But on macos I see scrolling that is not caused by my handler, and also a kind of ghost selected row (see the screenshots in flutter/devtools#1769) not caused by me (the color is different than the one I am using). Also, sometimes after clicking on a row the arrow keys will start cycling through the top-level tabs (Inspector, timeline) instead of the rows in the table.

In a nutshell there is some code handling the up/down arrow keys in a way that I don't understand, can't control, and can't predict. It also does not happen when I run via flutter web which leads me to believe it's a bug in the macos platform code.

The PR that introduced keyboard navigation is here: flutter/devtools#1810

Steps to Reproduce

$git clone git@github.com:flutter/devtools.git
$cd devtools/packages/devtools_app
$flutter run -d macos

  1. Enter the url of another running flutter app
  2. Click on the "info" tab
  3. Click "performance"
  4. Record performance data
  5. Click on the "call tree" tab
  6. Click inside the call tree table
  7. navigate up/down with the arrow keys.
  8. Compare with the behavior when running with -d web

Expected results:
I expect the arrow up/down keys to cause the window to scroll only when the selection reaches the bottom or top of the viewport. I expect there to be only 1 selected row. I expect that pressing the arrow up/down keys should not escape the treetable and start cycling the top-level tabs of the viewport.

Actual results:
See the examples at flutter/devtools#1769

Logs
I captured the logs, but they're really verbose (1600 lines). LMK if there is a better way to provide them besides pasting the whole thing into this issue.
gmoothart-macbookpro2:devtools_app gmoothart$ flutter analyze
Analyzing devtools_app...

warning • The include file ../../analysis_options.yaml in /Users/gmoothart/dev/devtools/packages/devtools_app/analysis_options.yaml cannot be found •
       analysis_options.yaml:1:11 • include_file_not_found
   info • Prefer declaring const constructors on `@immutable` classes • bug/lib/main.dart:35:3 • prefer_const_constructors_in_immutables
   info • Prefer const with constant constructors • bug/lib/main.dart:100:13 • prefer_const_constructors
   info • Prefer const with constant constructors • bug/lib/main.dart:113:16 • prefer_const_constructors

4 issues found. (ran in 20.1s)
gmoothart-macbookpro2:devtools_app gmoothart$ flutter doctor -v
[✓] Flutter (Channel master, v1.18.0-6.0.pre.51, on Mac OS X 10.15.3 19D76, locale en-US)
    • Flutter version 1.18.0-6.0.pre.51 at /Users/gmoothart/dev/flutter
    • Framework revision 20803507fd (3 minutes ago), 2020-04-16 14:36:42 -0700
    • Engine revision deef2663ac
    • Dart version 2.8.0 (build 2.8.0-dev.20.0 c9710e5059)

[✗] Android toolchain - develop for Android devices
    ✗ Unable to locate Android SDK.
      Install Android Studio from: https://developer.android.com/studio/index.html
      On first launch it will assist you in installing the Android SDK components.
      (or visit https://flutter.dev/docs/get-started/install/macos#android-setup for detailed instructions).
      If the Android SDK has been installed to a custom location, set ANDROID_SDK_ROOT to that location.
      You may also want to add it to your PATH environment variable.


[✓] Xcode - develop for iOS and macOS (Xcode 11.4)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 11.4, Build version 11E146
    • CocoaPods version 1.9.1

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[!] Android Studio (not installed)
    • Android Studio not found; download from https://developer.android.com/studio/index.html
      (or visit https://flutter.dev/docs/get-started/install/macos#android-setup for detailed instructions).

[✓] IntelliJ IDEA Community Edition (version 2019.3.4)
    • IntelliJ at /Applications/IntelliJ IDEA CE.app
    • Flutter plugin version 44.0.3
    • Dart plugin version 193.6911.31

[✓] VS Code (version 1.43.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.4.1

[✓] Connected device (3 available)
    • macOS      • macOS      • darwin-x64     • Mac OS X 10.15.3 19D76
    • Chrome     • chrome     • web-javascript • Google Chrome 80.0.3987.163
    • Web Server • web-server • web-javascript • Flutter Tools

! Doctor found issues in 2 categories.
@gmoothart gmoothart changed the title spurious arrow key handling in macos flutter app spurious arrow key handling in macos flutter app (devtools) Apr 16, 2020
@TahaTesser TahaTesser added platform-mac Building on or for macOS specifically d: devtools DevTools related - suite of performance and debugging tools tool Affects the "flutter" command-line tool. See also t: labels. labels Apr 17, 2020
@jonahwilliams jonahwilliams removed the tool Affects the "flutter" command-line tool. See also t: labels. label Apr 18, 2020
@gmoothart
Copy link
Contributor Author

I just noticed that a similar thing happens in the "flutter inspector" tab. This is without any specific key handling for the arrow keys. Focus travels through the inspector tree and other UI elements in an unpredictable way.

In this example I just mouse-click on the root element and then press the up/down/left/right arrow keys to see what happens.

inspector-arrow-keys

@devoncarew
Copy link
Member

cc @stuartmorgan

@gmoothart
Copy link
Contributor Author

I noticed that I can get similar behavior from the flutter gallery app.

In macos, the arrow keys move through the buttons:
gallery-arrow-keys

Similar to what "tab" does in flutter web; but while "tab" cycles through the UI elements, the macos arrow keys are "smart": left/right moves strictly back and forth and up/down moves focus vertically (example: with the "info" button highlighted, pressing down will focus "sign in").

In simple cases I could see this as desirable behavior, but with the kind of custom UI we have in the devtools it definitely is not. Maybe I just need to know how to turn it off?

@stuartmorgan
Copy link
Contributor

It also does not happen when I run via flutter web which leads me to believe it's a bug in the macos platform code.

Extremely unlikely; it's almost certainly this code. Note that macOS and web have different defaults.

@gspencergoog would have more context on this. Greg, I'm not sure why arrow keys are doing control navigation on macOS by default; I can't think of any native application that has that behavior so it seems very unexpected as a default.

@stuartmorgan
Copy link
Contributor

Maybe I just need to know how to turn it off?

I believe you can turn it off per #45102, but absent a strong argument in favor of having them, I think the right fix here is to have these not be part of the default set on desktop.

@stuartmorgan stuartmorgan added a: desktop Running on desktop framework flutter/packages/flutter repository. See also f: labels. labels Apr 21, 2020
@gspencergoog
Copy link
Contributor

You're right, I think these are probably not correct for macOS. I'll disable them.

I did look into the user interface guidelines for keyboard traversal on macOS at the time I defined those, but I didn't find guidance that was all that helpful there one way or another.

@stuartmorgan
Copy link
Contributor

When you disable them for macOS, maybe refactor a bit to have _defaultDesktopShortcuts, so that we don't have the same bug crop up for Linux and Windows? We can always re-specialize them later if we find places we want to diverge.

@gspencergoog
Copy link
Contributor

Linux (Debian) at least (I can't test windows at the moment) does do this kind of navigation, at least in their system settings app and a couple of others I tested.

@stuartmorgan
Copy link
Contributor

I'm seeing some control-specific things on Windows (e.g., if I'm in a list, or in a menu, then it navigates within that context, as on macOS), but nothing that jumps to other control areas.

@gspencergoog gspencergoog changed the title spurious arrow key handling in macos flutter app (devtools) Directional navigation key binding defaults should be limited to those platforms that use it. Apr 21, 2020
@gspencergoog
Copy link
Contributor

Actually, just disabling it won't work. I'm going to need to disable it at the top level, and then add it back in for each of the things that should be able to navigate in that way (e.g. for a list view, you should be able to navigate in the scroll axis).

@gmoothart
Copy link
Contributor Author

@gspencergoog Ah. This only partially helps me, since both my widgets are rendered in ListViews. Is there a way to control key navigation at that level? https://github.com/flutter/devtools/blob/6d9cd23f61c3bd98a32d84acbce0d25968b49021/packages/devtools_app/lib/src/flutter/table.dart#L661

@gspencergoog
Copy link
Contributor

You can always just turn it off, or bind it to another action.

To turn it off, you can put a Shortcuts widget around your view with those keys bound to "Intent.doNothing". Or bind them to your intent (and action) if you want to do something different.

@gspencergoog
Copy link
Contributor

gspencergoog commented Apr 24, 2020

Or, if you want to turn off directional navigation, but don't want to know the keys it is bound to, you can also bind a DirectionalFocusIntent to DoNothingAction() in an Actions widget around your view.

gmoothart added a commit to gmoothart/devtools that referenced this issue May 7, 2020
We want to handle key navigation ourselves, and Listbox expects arrow
key navigation to work on list elements, which creates a conflict. There
is some discussion of the behavior here:
flutter/flutter#54998

Also, since flutter#1899 autofocus is working which allowed me to remove a
couple tap-to-focus callbacks.
@stuartmorgan stuartmorgan added the P2 Important issues not at the top of the work list label Jun 10, 2020
@pedromassangocode
Copy link

Is this still valid?

@gspencergoog
Copy link
Contributor

Yes, this is still valid.

@cbracken cbracken added fyi-macos For the attention of macOS platform team team-text-input Owned by Text Input team and removed team-desktop labels Jun 5, 2024
@flutter-triage-bot
Copy link

The triaged-desktop label is irrelevant if there is no team-desktop label or fyi-desktop label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: desktop Running on desktop d: devtools DevTools related - suite of performance and debugging tools framework flutter/packages/flutter repository. See also f: labels. fyi-macos For the attention of macOS platform team P2 Important issues not at the top of the work list platform-mac Building on or for macOS specifically team-text-input Owned by Text Input team
Projects
None yet
Development

No branches or pull requests

9 participants