google / flutter-desktop-embedding

Experimental plugins for Flutter for Desktop
Apache License 2.0
7.1k stars 604 forks source link

[menubar] Bug: Shortcuts using alphabetical keys have an extra SHIFT modifier key registered #865

Closed guibarrette closed 3 years ago

guibarrette commented 3 years ago

Describe the bug I'm not sure when this bug appeared, it may be because of the new scheme for keyboard logical key ID, but I'm now seeing a bug regarding the registered shortcuts from the menubar plugin where a SHIFT modifier key is always attached to the shortcut when using a key binding that use an alphabetical key. It is not present with numerical keys.

For example, here I've added comments to the shortcuts to specify how they are registered:

// Tested on macOS
setApplicationMenu([
  Submenu(label: 'File', children: [
    MenuItem(
      label: 'Open…',
      onClicked: (){ print("Opening!"); },
      shortcut: LogicalKeySet(LogicalKeyboardKey.keyO)   // BUG:  Appears as SHIFT+O
    ),
    MenuDivider(),
    MenuItem(
      label: 'Save',
      onClicked: (){ print("Saved!"); },
      shortcut: LogicalKeySet(LogicalKeyboardKey.meta, LogicalKeyboardKey.keyS)   // BUG: Appears as CMD+SHIFT+S
    ),
  ]),
  Submenu(label: 'Counter', children: [
    MenuItem(
      label: 'Increment',
      onClicked: (){ print("incrementing"); },
      shortcut: LogicalKeySet(LogicalKeyboardKey.meta, LogicalKeyboardKey.digit1)   // OK: Appears as CMD+1
    ),
  ])
]);

Here are the screenshots showing the output of this code:

I'm using the plugin on macOS, so I can't say if this bug is also present on Windows and Linux

Log ``` $ flutter doctor -v [✓] Flutter (Channel master, 2.4.0-5.0.pre.122, on Mac OS X 10.14.6 18G9216 darwin-x64, locale fr-CA) • Flutter version 2.4.0-5.0.pre.122 at /Volumes/SPCC-SSD-GB/Programmation/Flutter/_SDKs/_NowUsing/flutter • Upstream repository https://github.com/flutter/flutter.git • Framework revision 5acf7e98f1 (il y a 23 heures), 2021-07-20 19:58:58 -0700 • Engine revision 9b270621e4 • Dart version 2.14.0 (build 2.14.0-338.0.dev) [!] Android toolchain - develop for Android devices (Android SDK version 28.0.3) ✗ cmdline-tools component is missing Run `path/to/sdkmanager --install "cmdline-tools;latest"` See https://developer.android.com/studio/command-line for more details. ✗ Android license status unknown. Run `flutter doctor --android-licenses` to accept the SDK licenses. See https://flutter.dev/docs/get-started/install/macos#android-setup for more details. [!] Xcode - develop for iOS and macOS • Xcode at /Applications/Xcode.app/Contents/Developer • Xcode 11.3.1, Build version 11C505 ✗ Xcode 11.3.1 out of date (12.0.1 is recommended). Download the latest version or update via the Mac App Store. • CocoaPods version 1.10.1 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 3.5) • Android Studio at /Applications/Android Studio.app/Contents • Flutter plugin version 43.0.1 • Dart plugin version 191.8593 • Java version OpenJDK Runtime Environment (build 1.8.0_202-release-1483-b49-5587405) [✓] VS Code (version 1.58.2) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.24.0 [✓] Connected device (2 available) • macOS (desktop) • macos • darwin-x64 • Mac OS X 10.14.6 18G9216 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 91.0.4472.164 ! Doctor found issues in 2 categories. ```
guibarrette commented 3 years ago

I just tested with the Flutter Stable release as of today (2.2.3), this bug is also present there. Therefore, it shouldn't be caused by the new scheme for keyboard logical key ID as I first thought, but something else.

Log ``` $ flutter doctor -v [✓] Flutter (Channel stable, 2.2.3, on Mac OS X 10.14.6 18G9216 darwin-x64, locale fr-CA) • Flutter version 2.2.3 at /Volumes/SPCC-SSD-GB/Programmation/Flutter/_SDKs/_NowUsing/flutter • Framework revision f4abaa0735 (il y a 3 semaines), 2021-07-01 12:46:11 -0700 • Engine revision 241c87ad80 • Dart version 2.13.4 [✓] Android toolchain - develop for Android devices (Android SDK version 28.0.3) • Android SDK at /Users/GuillaumeBarrette/Library/Android/sdk • Platform android-29, build-tools 28.0.3 • Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 1.8.0_202-release-1483-b49-5587405) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS • Xcode at /Applications/Xcode.app/Contents/Developer • Xcode 11.3.1, Build version 11C505 • CocoaPods version 1.10.1 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 3.5) • Android Studio at /Applications/Android Studio.app/Contents • Flutter plugin version 43.0.1 • Dart plugin version 191.8593 • Java version OpenJDK Runtime Environment (build 1.8.0_202-release-1483-b49-5587405) [✓] VS Code (version 1.58.2) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.24.0 [✓] Connected device (2 available) • macOS (desktop) • macos • darwin-x64 • Mac OS X 10.14.6 18G9216 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 91.0.4472.164 • No issues found! ```
stuartmorgan commented 3 years ago

Sounds like the labels for the logic keys became upper case at some point; the conversion to a channel representation probably just needs to lower-case the label.

guibarrette commented 3 years ago

Hi @stuartmorgan , thanks for your help, that was it! I just did a pull request to fix it. Thanks for your work!