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

Conversation

@dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Apr 27, 2022

This PR makes KeyboardManager an all-in-one component that encapsulates the entire keyboard system, and reworks the testing framework to support such changes.

All of KeyboardManager's external dependencies are now extracted as KeyboardManager.ViewDelegate, which is implemented by FlutterView. The keyboard responders are now created and managed by the manager, instead of the view. In this way, the view is no longer aware of any implementation details, but only what the manager needs from outside. Moreover, what's needed for KeyboardManager by InputConnectionAdaptor has also been extracted as a delegate.

Moreover, the testing framework has been reworked. The new framework mocks and compares the end result of the manager's effects, i.e. the engine and the messenger. It also adopts the latest "class-based" design.

As a non-trivial change, the channel message no longer provides vendorId and productId. Mocking them is a little harder under the new test framework, and they're likely not used much, and are expected to be removed in KeyEvent anyway.

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.

@zanderso
Copy link
Member

From PR triage: @dkwingsmt is this PR ready for review or is it still WIP?

@dkwingsmt
Copy link
Contributor Author

@zanderso This PR is almost ready for review, after I add a few documentations. But the reviewer is out this week so I'll have to wait until the next week.

Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

Much simpler. I like it!

@dkwingsmt dkwingsmt merged commit a406112 into flutter:main May 4, 2022
@dkwingsmt dkwingsmt deleted the android-key-view-delegate branch May 4, 2022 20:37
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 4, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 5, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 5, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants