Skip to content

Prepare ShortcutActivator and ShortcutManager to migrate to KeyEvent from RawKeyEvent. - #136854

Merged
gspencergoog merged 2 commits into
flutter:masterfrom
gspencergoog:activator_accepts
Nov 15, 2023
Merged

gspencergoog merged 2 commits into
flutter:masterfrom
gspencergoog:activator_accepts

Conversation

@gspencergoog

@gspencergoog gspencergoog commented Oct 19, 2023 •

Copy link
Copy Markdown
Contributor

Description

This modifies methods in ShortcutActivator and ShortcutManager to take KeyEvent events instead of RawKeyEvent events, introducing a breaking change.

Changes in this PR:

  1. Convert ShortcutActivator.accepts so it takes KeyEvents instead of RawKeyEvents.
  2. Deprecate ShortcutActivator.isActivatedBy.
  3. Convert ShortcutManager.handleKeypress to handle KeyEvents instead of RawKeyEvents.
  4. Modify Shortcuts widget to call Focus.onKeyEvent instead of Focus.onKey internally.
  5. Modify CallbackShortcuts to use KeyEvents instead of RawKeyEvents.

Breaking Change Migration Guide: https://docs.flutter.dev/release/breaking-changes/shortcut-key-event-migration

Related Issues

Tests

  • Update tests for the new reality.

@github-actions github-actions Bot added the framework flutter/packages/flutter repository. See also f: labels. label Oct 19, 2023
Comment thread packages/flutter/lib/src/services/hardware_keyboard.dart Outdated
Comment thread packages/flutter/lib/src/widgets/shortcuts.dart Outdated
Comment thread packages/flutter/lib/src/widgets/shortcuts.dart Outdated
Comment thread packages/flutter/lib/src/widgets/shortcuts.dart Outdated
@gspencergoog
gspencergoog force-pushed the activator_accepts branch 2 times, most recently from 0907ccc to f1162dd Compare October 23, 2023 19:39
@gspencergoog

Copy link
Copy Markdown
Contributor Author

Okay, I updated this so that it just breaks things. I have to say that it seems cleaner than the other way.

I didn't implement the accepts on CharacterActivator, etc. to take either kind of event, although I could do that for more backward compatibility. I wanted to see what breaks first.

Comment thread packages/flutter/lib/src/widgets/shortcuts.dart Outdated
Comment thread packages/flutter/lib/src/widgets/shortcuts.dart Outdated
@dkwingsmt

dkwingsmt commented Oct 25, 2023 •

Copy link
Copy Markdown
Contributor

Thanks for changing accepts to taking into account of triggers. However, now that this PR contains a logical change, it might not allow auto fixing again.

If we really want to enable auto fixing, we can:

  • Create a new method such as acceptEvent.
  • The acceptEvent defaults to accept(event, state) && trigger.contains(event.key)
  • Flutter's activator classes will override acceptEvent.
  • Deprecate accept.

@gspencergoog

Copy link
Copy Markdown
Contributor Author

Thanks for changing accepts to taking into account of triggers. However, now that this PR contains a logical change, it might not allow auto fixing again.

Since the two functions have the same signature, and only differ in implementation, any "quick fix" applied would just rename the call, and wouldn't be able to address any difference in functionality anyhow.

For instance, if I have a subclass that always returns true from accepts, and relies on the triggers to filter it to the keys I care about, just renaming it to acceptsEvent will be the only fix that can be applied, but that doesn't take the difference in functionality into account. I suppose renaming it would force people to look at all the instances where it is used, but in that case we wouldn't want to supply a flutter fix because that would just "fix" them all by renaming them, hiding the implementation difference again.

@gspencergoog

Copy link
Copy Markdown
Contributor Author

I'm a little worried that the triggers change might be too subtle for people to understand easily and make it hard to migrate. I mean, it's not too complicated, but I find it hard to justify the churn for the benefit.

It is nice to make the triggers optional, and I like that it opens up the algorithm possibilities in the accepts function, but it's a small change that will take a lot of explaining.

@gspencergoog

gspencergoog commented Nov 14, 2023 •

Copy link
Copy Markdown
Contributor Author

@goderbauer and @dkwingsmt can you take another look here: I think this is ready to be submitted. I'll be commenting out the super_editor customer tests and passing the presubmit before I submit, followed by submitting the fix to super_editor before I turn them back on. I wanted to make sure you're OK with this change before I started that process, however, to keep them commented out for as short a time as possible.

@goderbauer goderbauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

gspencergoog added a commit to flutter/tests that referenced this pull request Nov 15, 2023
## Description

This temporarily disables the super_editor tests while
flutter/flutter#136854 and
https://github.com/superlistapp/super_editor/pull/1593 are landed, to
avoid a lot of churn in the super_editor repo.

## Related Issues
 - flutter/flutter#136419
@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 15, 2023
@gspencergoog
gspencergoog merged commit f796e62 into flutter:master Nov 15, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 16, 2023
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Nov 16, 2023
Roll Flutter from e8c2bb1 to 53a57ad (39 revisions)

flutter/flutter@e8c2bb1...53a57ad

2023-11-16 [email protected] Roll Flutter Engine from 8ab189b77b8d to 2e9f0df868b3 (1 revision) (flutter/flutter#138543)
2023-11-16 [email protected] Roll Flutter Engine from 622fa0614412 to 8ab189b77b8d (1 revision) (flutter/flutter#138533)
2023-11-16 [email protected] [flutter_tools] - Add `queries` section to Android manifest file (flutter/flutter#137207)
2023-11-16 [email protected] Roll Flutter Engine from 8aff9c134b8f to 622fa0614412 (1 revision) (flutter/flutter#138532)
2023-11-16 [email protected] Roll Flutter Engine from 3cfcdebe8623 to 8aff9c134b8f (18 revisions) (flutter/flutter#138529)
2023-11-16 [email protected] Roll Flutter Engine from 30327eae0802 to 3cfcdebe8623 (1 revision) (flutter/flutter#138515)
2023-11-15 [email protected] Catch error for missing directory in `FontConfigManager` (flutter/flutter#138496)
2023-11-15 [email protected] Make `UnderlineInputBorder` consistent (flutter/flutter#124153)
2023-11-15 [email protected] Prepare `ShortcutActivator` and `ShortcutManager` to migrate to `KeyEvent` from `RawKeyEvent`. (flutter/flutter#136854)
2023-11-15 [email protected] Pin package:web 0.4.0 (flutter/flutter#138428)
2023-11-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland VelocityTracker update (#132291)" (flutter/flutter#138512)
2023-11-15 [email protected] [web] skip flaky overflow_clipbehavior_none.cupertino.0.png golden check (flutter/flutter#138498)
2023-11-15 [email protected] Roll Flutter Engine from 7c2b8d520b3d to 30327eae0802 (2 revisions) (flutter/flutter#138502)
2023-11-15 [email protected] Reland VelocityTracker update (#132291) (flutter/flutter#137381)
2023-11-15 [email protected] Roll Flutter Engine from f58dac64ad1a to 7c2b8d520b3d (1 revision) (flutter/flutter#138499)
2023-11-15 [email protected] Fix 2D tap to stop scrolling (flutter/flutter#138442)
2023-11-15 [email protected] Roll Flutter Engine from d22d063ac9f6 to f58dac64ad1a (2 revisions) (flutter/flutter#138494)
2023-11-15 [email protected] SemanticOwner should dispatch creation and disposal events (flutter/flutter#138388)
2023-11-15 [email protected] Roll Flutter Engine from ecaf9442034d to d22d063ac9f6 (5 revisions) (flutter/flutter#138489)
2023-11-15 [email protected] Roll Packages from 428ba3e to 0cd2378 (1 revision) (flutter/flutter#138482)
2023-11-15 [email protected] Marks Mac_android hot_mode_dev_cycle__benchmark to be unflaky (flutter/flutter#138472)
2023-11-15 [email protected] Roll Flutter Engine from a7a48a68e6f8 to ecaf9442034d (1 revision) (flutter/flutter#138468)
2023-11-15 [email protected] Roll Flutter Engine from 976edd5192d1 to a7a48a68e6f8 (3 revisions) (flutter/flutter#138463)
2023-11-15 [email protected] Roll Flutter Engine from a7f2267dd1f4 to 976edd5192d1 (1 revision) (flutter/flutter#138462)
2023-11-15 [email protected] Roll Flutter Engine from bc5bbd3b9ebe to a7f2267dd1f4 (1 revision) (flutter/flutter#138459)
2023-11-15 [email protected] Roll Flutter Engine from d7ca057b891f to bc5bbd3b9ebe (2 revisions) (flutter/flutter#138457)
2023-11-15 [email protected] Roll Flutter Engine from 1347413470b7 to d7ca057b891f (1 revision) (flutter/flutter#138456)
2023-11-15 [email protected] Roll Flutter Engine from c5a067b637f4 to 1347413470b7 (5 revisions) (flutter/flutter#138452)
2023-11-15 [email protected] Reland [SingleChildScrollView] Correct the offset pixels if it is out of range during layout (flutter/flutter#136871)
2023-11-14 [email protected] Add to TableCell docs (flutter/flutter#138258)
2023-11-14 [email protected] Roll Flutter Engine from f15b259fe98c to c5a067b637f4 (4 revisions) (flutter/flutter#138441)
2023-11-14 49699333+dependabot[bot]@users.noreply.github.com Bump dessant/lock-threads from 4.0.1 to 5.0.0 (flutter/flutter#138437)
2023-11-14 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.22.5 to 2.22.6 (flutter/flutter#138438)
2023-11-14 [email protected] Roll Flutter Engine from eba757803a6f to f15b259fe98c (1 revision) (flutter/flutter#138429)
2023-11-14 [email protected] Roll Flutter Engine from 603bdd48df8a to eba757803a6f (3 revisions) (flutter/flutter#138425)
2023-11-14 [email protected] Unified analytics migration for `CodeSizeAnalysis` (flutter/flutter#138351)
2023-11-14 [email protected] Roll Flutter Engine from 777dcb99f6e0 to 603bdd48df8a (1 revision) (flutter/flutter#138421)
2023-11-14 [email protected] Run all tests in examples/ (flutter/flutter#138374)
2023-11-14 [email protected] Roll Flutter Engine from 1b3fd80812c3 to 777dcb99f6e0 (2 revisions) (flutter/flutter#138420)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

...
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
Roll Flutter from e8c2bb1 to 53a57ad (39 revisions)

flutter/flutter@e8c2bb1...53a57ad

2023-11-16 [email protected] Roll Flutter Engine from 8ab189b77b8d to 2e9f0df868b3 (1 revision) (flutter/flutter#138543)
2023-11-16 [email protected] Roll Flutter Engine from 622fa0614412 to 8ab189b77b8d (1 revision) (flutter/flutter#138533)
2023-11-16 [email protected] [flutter_tools] - Add `queries` section to Android manifest file (flutter/flutter#137207)
2023-11-16 [email protected] Roll Flutter Engine from 8aff9c134b8f to 622fa0614412 (1 revision) (flutter/flutter#138532)
2023-11-16 [email protected] Roll Flutter Engine from 3cfcdebe8623 to 8aff9c134b8f (18 revisions) (flutter/flutter#138529)
2023-11-16 [email protected] Roll Flutter Engine from 30327eae0802 to 3cfcdebe8623 (1 revision) (flutter/flutter#138515)
2023-11-15 [email protected] Catch error for missing directory in `FontConfigManager` (flutter/flutter#138496)
2023-11-15 [email protected] Make `UnderlineInputBorder` consistent (flutter/flutter#124153)
2023-11-15 [email protected] Prepare `ShortcutActivator` and `ShortcutManager` to migrate to `KeyEvent` from `RawKeyEvent`. (flutter/flutter#136854)
2023-11-15 [email protected] Pin package:web 0.4.0 (flutter/flutter#138428)
2023-11-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland VelocityTracker update (#132291)" (flutter/flutter#138512)
2023-11-15 [email protected] [web] skip flaky overflow_clipbehavior_none.cupertino.0.png golden check (flutter/flutter#138498)
2023-11-15 [email protected] Roll Flutter Engine from 7c2b8d520b3d to 30327eae0802 (2 revisions) (flutter/flutter#138502)
2023-11-15 [email protected] Reland VelocityTracker update (#132291) (flutter/flutter#137381)
2023-11-15 [email protected] Roll Flutter Engine from f58dac64ad1a to 7c2b8d520b3d (1 revision) (flutter/flutter#138499)
2023-11-15 [email protected] Fix 2D tap to stop scrolling (flutter/flutter#138442)
2023-11-15 [email protected] Roll Flutter Engine from d22d063ac9f6 to f58dac64ad1a (2 revisions) (flutter/flutter#138494)
2023-11-15 [email protected] SemanticOwner should dispatch creation and disposal events (flutter/flutter#138388)
2023-11-15 [email protected] Roll Flutter Engine from ecaf9442034d to d22d063ac9f6 (5 revisions) (flutter/flutter#138489)
2023-11-15 [email protected] Roll Packages from 428ba3e to 0cd2378 (1 revision) (flutter/flutter#138482)
2023-11-15 [email protected] Marks Mac_android hot_mode_dev_cycle__benchmark to be unflaky (flutter/flutter#138472)
2023-11-15 [email protected] Roll Flutter Engine from a7a48a68e6f8 to ecaf9442034d (1 revision) (flutter/flutter#138468)
2023-11-15 [email protected] Roll Flutter Engine from 976edd5192d1 to a7a48a68e6f8 (3 revisions) (flutter/flutter#138463)
2023-11-15 [email protected] Roll Flutter Engine from a7f2267dd1f4 to 976edd5192d1 (1 revision) (flutter/flutter#138462)
2023-11-15 [email protected] Roll Flutter Engine from bc5bbd3b9ebe to a7f2267dd1f4 (1 revision) (flutter/flutter#138459)
2023-11-15 [email protected] Roll Flutter Engine from d7ca057b891f to bc5bbd3b9ebe (2 revisions) (flutter/flutter#138457)
2023-11-15 [email protected] Roll Flutter Engine from 1347413470b7 to d7ca057b891f (1 revision) (flutter/flutter#138456)
2023-11-15 [email protected] Roll Flutter Engine from c5a067b637f4 to 1347413470b7 (5 revisions) (flutter/flutter#138452)
2023-11-15 [email protected] Reland [SingleChildScrollView] Correct the offset pixels if it is out of range during layout (flutter/flutter#136871)
2023-11-14 [email protected] Add to TableCell docs (flutter/flutter#138258)
2023-11-14 [email protected] Roll Flutter Engine from f15b259fe98c to c5a067b637f4 (4 revisions) (flutter/flutter#138441)
2023-11-14 49699333+dependabot[bot]@users.noreply.github.com Bump dessant/lock-threads from 4.0.1 to 5.0.0 (flutter/flutter#138437)
2023-11-14 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.22.5 to 2.22.6 (flutter/flutter#138438)
2023-11-14 [email protected] Roll Flutter Engine from eba757803a6f to f15b259fe98c (1 revision) (flutter/flutter#138429)
2023-11-14 [email protected] Roll Flutter Engine from 603bdd48df8a to eba757803a6f (3 revisions) (flutter/flutter#138425)
2023-11-14 [email protected] Unified analytics migration for `CodeSizeAnalysis` (flutter/flutter#138351)
2023-11-14 [email protected] Roll Flutter Engine from 777dcb99f6e0 to 603bdd48df8a (1 revision) (flutter/flutter#138421)
2023-11-14 [email protected] Run all tests in examples/ (flutter/flutter#138374)
2023-11-14 [email protected] Roll Flutter Engine from 1b3fd80812c3 to 777dcb99f6e0 (2 revisions) (flutter/flutter#138420)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

...
bisor0627 pushed a commit to bisor0627/packages that referenced this pull request Jun 19, 2026
Roll Flutter from e8c2bb1 to 53a57ad (39 revisions)

flutter/flutter@e8c2bb1...53a57ad

2023-11-16 [email protected] Roll Flutter Engine from 8ab189b77b8d to 2e9f0df868b3 (1 revision) (flutter/flutter#138543)
2023-11-16 [email protected] Roll Flutter Engine from 622fa0614412 to 8ab189b77b8d (1 revision) (flutter/flutter#138533)
2023-11-16 [email protected] [flutter_tools] - Add `queries` section to Android manifest file (flutter/flutter#137207)
2023-11-16 [email protected] Roll Flutter Engine from 8aff9c134b8f to 622fa0614412 (1 revision) (flutter/flutter#138532)
2023-11-16 [email protected] Roll Flutter Engine from 3cfcdebe8623 to 8aff9c134b8f (18 revisions) (flutter/flutter#138529)
2023-11-16 [email protected] Roll Flutter Engine from 30327eae0802 to 3cfcdebe8623 (1 revision) (flutter/flutter#138515)
2023-11-15 [email protected] Catch error for missing directory in `FontConfigManager` (flutter/flutter#138496)
2023-11-15 [email protected] Make `UnderlineInputBorder` consistent (flutter/flutter#124153)
2023-11-15 [email protected] Prepare `ShortcutActivator` and `ShortcutManager` to migrate to `KeyEvent` from `RawKeyEvent`. (flutter/flutter#136854)
2023-11-15 [email protected] Pin package:web 0.4.0 (flutter/flutter#138428)
2023-11-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland VelocityTracker update (#132291)" (flutter/flutter#138512)
2023-11-15 [email protected] [web] skip flaky overflow_clipbehavior_none.cupertino.0.png golden check (flutter/flutter#138498)
2023-11-15 [email protected] Roll Flutter Engine from 7c2b8d520b3d to 30327eae0802 (2 revisions) (flutter/flutter#138502)
2023-11-15 [email protected] Reland VelocityTracker update (#132291) (flutter/flutter#137381)
2023-11-15 [email protected] Roll Flutter Engine from f58dac64ad1a to 7c2b8d520b3d (1 revision) (flutter/flutter#138499)
2023-11-15 [email protected] Fix 2D tap to stop scrolling (flutter/flutter#138442)
2023-11-15 [email protected] Roll Flutter Engine from d22d063ac9f6 to f58dac64ad1a (2 revisions) (flutter/flutter#138494)
2023-11-15 [email protected] SemanticOwner should dispatch creation and disposal events (flutter/flutter#138388)
2023-11-15 [email protected] Roll Flutter Engine from ecaf9442034d to d22d063ac9f6 (5 revisions) (flutter/flutter#138489)
2023-11-15 [email protected] Roll Packages from 1b93fed to f23e52e (1 revision) (flutter/flutter#138482)
2023-11-15 [email protected] Marks Mac_android hot_mode_dev_cycle__benchmark to be unflaky (flutter/flutter#138472)
2023-11-15 [email protected] Roll Flutter Engine from a7a48a68e6f8 to ecaf9442034d (1 revision) (flutter/flutter#138468)
2023-11-15 [email protected] Roll Flutter Engine from 976edd5192d1 to a7a48a68e6f8 (3 revisions) (flutter/flutter#138463)
2023-11-15 [email protected] Roll Flutter Engine from a7f2267dd1f4 to 976edd5192d1 (1 revision) (flutter/flutter#138462)
2023-11-15 [email protected] Roll Flutter Engine from bc5bbd3b9ebe to a7f2267dd1f4 (1 revision) (flutter/flutter#138459)
2023-11-15 [email protected] Roll Flutter Engine from d7ca057b891f to bc5bbd3b9ebe (2 revisions) (flutter/flutter#138457)
2023-11-15 [email protected] Roll Flutter Engine from 1347413470b7 to d7ca057b891f (1 revision) (flutter/flutter#138456)
2023-11-15 [email protected] Roll Flutter Engine from c5a067b637f4 to 1347413470b7 (5 revisions) (flutter/flutter#138452)
2023-11-15 [email protected] Reland [SingleChildScrollView] Correct the offset pixels if it is out of range during layout (flutter/flutter#136871)
2023-11-14 [email protected] Add to TableCell docs (flutter/flutter#138258)
2023-11-14 [email protected] Roll Flutter Engine from f15b259fe98c to c5a067b637f4 (4 revisions) (flutter/flutter#138441)
2023-11-14 49699333+dependabot[bot]@users.noreply.github.com Bump dessant/lock-threads from 4.0.1 to 5.0.0 (flutter/flutter#138437)
2023-11-14 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.22.5 to 2.22.6 (flutter/flutter#138438)
2023-11-14 [email protected] Roll Flutter Engine from eba757803a6f to f15b259fe98c (1 revision) (flutter/flutter#138429)
2023-11-14 [email protected] Roll Flutter Engine from 603bdd48df8a to eba757803a6f (3 revisions) (flutter/flutter#138425)
2023-11-14 [email protected] Unified analytics migration for `CodeSizeAnalysis` (flutter/flutter#138351)
2023-11-14 [email protected] Roll Flutter Engine from 777dcb99f6e0 to 603bdd48df8a (1 revision) (flutter/flutter#138421)
2023-11-14 [email protected] Run all tests in examples/ (flutter/flutter#138374)
2023-11-14 [email protected] Roll Flutter Engine from 1b3fd80812c3 to 777dcb99f6e0 (2 revisions) (flutter/flutter#138420)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants