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

Fixes #6 Migrate RawKeyboardEvent and related implications according … #9

Merged

Conversation

aymswick
Copy link

@aymswick aymswick commented Feb 18, 2024

…to Flutter recommendations described here: flutter/flutter#136419

I swapped out the Raw version for the recommended Hardware version, and updated onKey to onKeyEvent in our keyboard listeners' FocusNodes as instructed by the migration guide: https://docs.flutter.dev/release/breaking-changes/key-event-migration . I believe merging this PR will get everyone on Flutter 3.19 compiling again without breaking existing functionality, but welcome further examination and manual testing.

I believe this change has caused 3 tests to fail in the pluto_key_manager_event_test.dart file, and have included a comment there to reflect what I believe to be the cause and best course of action for now. I have tested the 3 failing test cases manually (irl with a bluetooth keyboard on macOS and Chrome) and everything behaves as expected. I will paste the comment here as well:

While key combos still work in the real world, these 3 tests are failing due to what I suspect is an incomplete deprecation/migration from focusNode onKey to onKeyEvent. Flutter 3.19 does not trigger our event for sendKeyUpEvent only, and I prefer not to switch these tests to sendKeyDownEvent as that may cause unexpected behavior such as pasting multiple times due to repeating key presses. It might also be fine.

Additionally, I have reached out on the relevant Flutter issue (first link in this comment) to provide more clarity on if this test is failing due to an incomplete Flutter migration.

@doonfrs
Copy link
Owner

doonfrs commented Feb 18, 2024

Thank you @aymswick for your great effort I will merge it first to allow the build then I will check the failed tests.

@doonfrs doonfrs merged commit 29e2ada into doonfrs:master Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants