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

Synchronize modifier keys in RawKeyboard.keysPressed with modifier flags on events. #43948

Merged
merged 8 commits into from Nov 6, 2019

Conversation

@gspencergoog
Copy link
Contributor

gspencergoog commented Nov 1, 2019

Description

Currently, we listen to keyboard events to find out which keys should be represented in RawKeyboard.instance.keysPressed, but that's not sufficient to represent the physical state of the keys, since modifier keys could have been pressed when the overall app did not have keyboard focus (especially on desktop platforms).

This PR synchronizes the list of modifier keys in keysPressed with the modifier key flags that are present in the raw key event so that they can be relied upon to represent the current state of the keyboard. When synchronizing these states, we don't send any new key events, since they didn't happen when the app had keyboard focus, but if you ask "is this key down", we'll give the right answer.

Related Issues

Fixes #40496

Tests

  • Added tests for each of the platforms we support.
  • Modified the event simulation code in flutter_test so that the raw event data could be obtained, and wrapped it in a namespace class called KeyEventSimulator.

Breaking Change

  • No, this is not a breaking change.
@gspencergoog gspencergoog requested a review from franciscojma86 Nov 1, 2019
@googlebot googlebot added the cla: yes label Nov 1, 2019
@gspencergoog gspencergoog added this to In progress in Desktop Features Nov 1, 2019
@gspencergoog gspencergoog force-pushed the gspencergoog:modifier_sync branch 4 times, most recently from e96e3a3 to 28620e3 Nov 1, 2019
Copy link
Contributor

franciscojma86 left a comment

LGTM modulo avoiding "we" in comments

@gspencergoog

This comment has been minimized.

Copy link
Contributor Author

gspencergoog commented Nov 6, 2019

Thanks! Don't worry, we'll fix that. :-)

gspencergoog added 8 commits Nov 1, 2019
@gspencergoog gspencergoog force-pushed the gspencergoog:modifier_sync branch from 111ba31 to 756c2b4 Nov 6, 2019
@gspencergoog gspencergoog merged commit 028ed71 into flutter:master Nov 6, 2019
63 checks passed
63 checks passed
WIP Ready for review
Details
analyze-linux Task Summary
Details
analyze-linux
Details
build_tests-linux Task Summary
Details
build_tests-linux
Details
cla/google All necessary CLAs are signed
customer_testing-linux Task Summary
Details
customer_testing-linux
Details
customer_testing-macos Task Summary
Details
customer_testing-macos
Details
customer_testing-windows Task Summary
Details
customer_testing-windows
Details
deploy_gallery-linux Task Summary
Details
deploy_gallery-linux
Details
deploy_gallery-macos Task Summary
Details
deploy_gallery-macos
Details
docs-linux Task Summary
Details
docs-linux
Details
firebase_test_lab_tests-linux Task Summary
Details
firebase_test_lab_tests-linux
Details
flutter-build
Details
framework_tests-libraries-linux Task Summary
Details
framework_tests-libraries-linux
Details
framework_tests-libraries-macos Task Summary
Details
framework_tests-libraries-macos
Details
framework_tests-libraries-windows Task Summary
Details
framework_tests-libraries-windows
Details
framework_tests-misc-linux Task Summary
Details
framework_tests-misc-linux
Details
framework_tests-misc-macos Task Summary
Details
framework_tests-misc-macos
Details
framework_tests-misc-windows Task Summary
Details
framework_tests-misc-windows
Details
framework_tests-widgets-linux Task Summary
Details
framework_tests-widgets-linux
Details
framework_tests-widgets-macos Task Summary
Details
framework_tests-widgets-macos
Details
framework_tests-widgets-windows Task Summary
Details
framework_tests-widgets-windows
Details
hostonly_devicelab_tests-0-linux Task Summary
Details
hostonly_devicelab_tests-0-linux
Details
hostonly_devicelab_tests-1-linux Task Summary
Details
hostonly_devicelab_tests-1-linux
Details
hostonly_devicelab_tests-2-linux Task Summary
Details
hostonly_devicelab_tests-2-linux
Details
hostonly_devicelab_tests-3_last-linux Task Summary
Details
hostonly_devicelab_tests-3_last-linux
Details
web_tests-0-linux Task Summary
Details
web_tests-0-linux
Details
web_tests-1-linux Task Summary
Details
web_tests-1-linux
Details
web_tests-2-linux Task Summary
Details
web_tests-2-linux
Details
web_tests-3-linux Task Summary
Details
web_tests-3-linux
Details
web_tests-4-linux Task Summary
Details
web_tests-4-linux
Details
web_tests-5-linux Task Summary
Details
web_tests-5-linux
Details
web_tests-6-linux Task Summary
Details
web_tests-6-linux
Details
web_tests-7_last-linux Task Summary
Details
web_tests-7_last-linux
Details
@gspencergoog gspencergoog deleted the gspencergoog:modifier_sync branch Nov 13, 2019
@gspencergoog gspencergoog moved this from In progress to Done in Desktop Features Nov 15, 2019
sahandevs added a commit to sahandevs/flutter that referenced this pull request Nov 15, 2019
…ags on events. (flutter#43948)

Currently, we listen to keyboard events to find out which keys should be represented in RawKeyboard.instance.keysPressed, but that's not sufficient to represent the physical state of the keys, since modifier keys could have been pressed when the overall app did not have keyboard focus (especially on desktop platforms).

This PR synchronizes the list of modifier keys in keysPressed with the modifier key flags that are present in the raw key event so that they can be relied upon to represent the current state of the keyboard. When synchronizing these states, we don't send any new key events, since they didn't happen when the app had keyboard focus, but if you ask "is this key down", we'll give the right answer
sahandevs added a commit to sahandevs/flutter that referenced this pull request Nov 15, 2019
…ags on events. (flutter#43948)

Currently, we listen to keyboard events to find out which keys should be represented in RawKeyboard.instance.keysPressed, but that's not sufficient to represent the physical state of the keys, since modifier keys could have been pressed when the overall app did not have keyboard focus (especially on desktop platforms).

This PR synchronizes the list of modifier keys in keysPressed with the modifier key flags that are present in the raw key event so that they can be relied upon to represent the current state of the keyboard. When synchronizing these states, we don't send any new key events, since they didn't happen when the app had keyboard focus, but if you ask "is this key down", we'll give the right answer
sahandevs added a commit to sahandevs/flutter that referenced this pull request Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
3 participants
You can’t perform that action at this time.