Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[macOS, Keyboard] Refactor: Clean up keyboard initialization, connection, and unit test framework #31940

Merged
merged 12 commits into from
Mar 11, 2022
Merged

[macOS, Keyboard] Refactor: Clean up keyboard initialization, connection, and unit test framework #31940

merged 12 commits into from
Mar 11, 2022

Conversation

@dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Mar 9, 2022 •
edited
Loading

This PR makes FlutterKeyboardManager an all-in-one handler for key events, including cooperation with TextInputPlugin, similar to Win32's KeyboardManagerWin32.

  • Responders are no longer created by FlutterViewController but by FlutterKeyboardManager.
  • Everything that FlutterKeyboardManager needs from FlutterViewController is abstracted as a new protocol,FlutterKeyboardViewDelegate.
  • FlutterKeySecondaryResponder is removed.
  • A new keyboard testing framework, KeyboardTester, is created. In the future more tests will be added to this framework.

All existing tests have been migrated with no semantic changes. No new tests should be needed since it's only a refactory.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@dkwingsmt dkwingsmt changed the title [macOS, Keyboard] Refactor: Move keyboard initialization into KeyboardManager, and connect to ViewController using ViewDelegate [macOS, Keyboard] Refactor: Clean up keyboard initialization, connection, and unit test framework Mar 9, 2022
@dkwingsmt dkwingsmt requested a review from gspencergoog March 10, 2022 00:51
@dkwingsmt
Copy link
Contributor Author

dkwingsmt commented Mar 10, 2022 •
edited
Loading

cc @cbracken @LongCatIsLooong

This is the refactory PR. I'm still debating whether to filter the events in FlutterKeyboardManager.handleEvent or during FlutterViewController's keydown/keyup/flagschange. I'm leaning on the former, and if so, what you'll do is to add duringCompose into FlutterKeyboardViewDelegate.

cbracken
cbracken approved these changes Mar 10, 2022
View reviewed changes
Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

LGTM stamp from a Japanese personal seal

@dkwingsmt dkwingsmt added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Mar 10, 2022
@fluttergithubbot fluttergithubbot merged commit e21a58d into main Mar 11, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 14, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 14, 2022
@engine-flutter-autoroll
e21a58d [macOS, Keyboard] Refactor: Clean up keyboard initialization,…
ca02217 
… connection, and unit test framework (flutter/engine#31940)
zanderso pushed a commit to flutter/flutter that referenced this pull request Mar 14, 2022
@engine-flutter-autoroll
Roll Engine from 6239bfb to 25acd77 (33 revisions) (#100061)
11deb15 
* ba6f4f5 Roll Skia from c88fff00c8fd to 02ea811ce869 (4 revisions) (flutter/engine#31955)

* 1a8033c Revert "Add a message when a key response takes too long. (#31945)" (flutter/engine#31962)

* ebcd86f68 Make sure the secondary vsync callback is called after the vsync callback (flutter/engine#31513)

* 2c94cc5 Generate a11y events for widgets that use kFlutterSemanticsFlagIsToggled (flutter/engine#31582)

* e21a58d [macOS, Keyboard] Refactor: Clean up keyboard initialization, connection, and unit test framework (flutter/engine#31940)

* 27e62d0 Set a11y roles for checks, toggles and sliders. (flutter/engine#31600)

* 5e760f6 Fix signals adding/removing a11y nodes in Linux. (flutter/engine#31601)

* 749dc3a Changed the default a11y node role from ATK_ROLE_FRAME to ATK_ROLE_PANEL (flutter/engine#31602)

* da7bae6 Animator stopped notifying delegate to render when pipeline is not empty (flutter/engine#31727)

* 77dd8af Roll Fuchsia Mac SDK from y2eJ9z95V... to j7jyFiU9e... (flutter/engine#31960)

* 3f803b2 Roll Skia from 02ea811ce869 to b141e485d248 (5 revisions) (flutter/engine#31961)

* 187a277 Roll Fuchsia Linux SDK from -9uFZIRSL... to k8Vq-HEG-... (flutter/engine#31971)

* 43975b2 Roll Skia from b141e485d248 to 1df655a42746 (13 revisions) (flutter/engine#31975)

* ab12f81 Roll Fuchsia Mac SDK from j7jyFiU9e... to g8opaxAq7... (flutter/engine#31978)

* 93b792f Roll Skia from 1df655a42746 to 38b9591b5a04 (1 revision) (flutter/engine#31979)

* d6964c0 Update jazzy to 0.14.1. (flutter/engine#31970)

* 31def4e Re-enable dart runner AOT builds (flutter/engine#31844)

* 887a193 Roll Fuchsia Linux SDK from k8Vq-HEG-... to mDjjSq8Im... (flutter/engine#31980)

* afd9ce1 Roll Fuchsia Mac SDK from g8opaxAq7... to f8IibBq3a... (flutter/engine#31983)

* f7d8314 Roll Fuchsia Linux SDK from mDjjSq8Im... to BEc_F0_Fp... (flutter/engine#31986)

* 503c02c Roll Fuchsia Mac SDK from f8IibBq3a... to oyZzinh6U... (flutter/engine#31987)

* caa3f3d Roll Fuchsia Linux SDK from BEc_F0_Fp... to obtypxiCA... (flutter/engine#31988)

* 4643016 Roll Dart SDK from 4fbe27c0f148 to 3e76a897013f (5 revisions) (flutter/engine#31989)

* 06d3d06 Roll Dart SDK from 3e76a897013f to 55b9ec4e382a (5 revisions) (flutter/engine#31991)

* cdf862b Roll Skia from 38b9591b5a04 to 6d19271fb148 (2 revisions) (flutter/engine#31992)

* bbebccd Roll Fuchsia Mac SDK from oyZzinh6U... to rmQ9yYUaF... (flutter/engine#31993)

* 56187fa Fix timestamp of touch events should use system startup time (flutter/engine#30422)

* 1dab008 Roll Fuchsia Linux SDK from obtypxiCA... to xYtoLAOvY... (flutter/engine#31995)

* fccde49 Add systemFontFamily to flutter/settings channel (flutter/engine#22981)

* 2a8c4f4 Roll Fuchsia Mac SDK from rmQ9yYUaF... to 84kG1vmHe... (flutter/engine#31996)

* e6cf464 Roll Skia from 6d19271fb148 to 7a61e5d65399 (1 revision) (flutter/engine#31997)

* 1cc349e Roll Fuchsia Linux SDK from xYtoLAOvY... to P8RdLi_Y_... (flutter/engine#31999)

* 25acd77 Roll Skia from 7a61e5d65399 to 02527b7182ea (1 revision) (flutter/engine#32000)
@dnfield dnfield deleted the mac-key-merge-manager branch March 16, 2022 05:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@gspencergoog gspencergoog Awaiting requested review from gspencergoog

1 more reviewer

@cbracken cbracken cbracken approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

platform-macos waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dkwingsmt @cbracken @fluttergithubbot