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

[Linux, Keyboard] Extract KeyboardManager's external dependencies as ViewDelegate; Rework testing framework#32305

Merged
fluttergithubbot merged 11 commits intoflutter:mainfrom
dkwingsmt:linux-key-delegate
Apr 12, 2022
Merged

[Linux, Keyboard] Extract KeyboardManager's external dependencies as ViewDelegate; Rework testing framework#32305
fluttergithubbot merged 11 commits intoflutter:mainfrom
dkwingsmt:linux-key-delegate

Conversation

@dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Mar 28, 2022

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

All of FlKeyboardManager's external dependencies are now extracted as FlKeyboardViewDelegate, which is implemented by FlView. 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, 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.

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.

A test passes

less changes

Redispatch uses unique_ptr

DisposeWithUnresolvedPends

first half of SingleDelegateWithAsyncResponds

Use idle to implement pending

Rename to flushChannelMessages

EmbedderCallRecord

SingleDelegateWithAsyncResponds

EXPECT_FALSE(during_redispatch_)

EXPECT_KEY_EVENT

CallRecord

All tests

Format
@dkwingsmt dkwingsmt force-pushed the linux-key-delegate branch from 3696bd8 to ba8573e Compare April 1, 2022 20:32
@dkwingsmt dkwingsmt changed the title Linux view delegate [Linux, Keyboard] Extract KeyboardManager's external dependencies as ViewDelegate Apr 1, 2022
@dkwingsmt dkwingsmt changed the title [Linux, Keyboard] Extract KeyboardManager's external dependencies as ViewDelegate [Linux, Keyboard] Extract KeyboardManager's external dependencies as ViewDelegate; Rework testing framework Apr 1, 2022
@dkwingsmt dkwingsmt marked this pull request as ready for review April 1, 2022 20:38
@dkwingsmt dkwingsmt requested a review from gspencergoog April 1, 2022 20:38
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

This is much nicer. Just some documentation nits.

@dkwingsmt dkwingsmt added affects: text input waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. labels Apr 12, 2022
@fluttergithubbot fluttergithubbot merged commit 36b2857 into flutter:main Apr 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 13, 2022
justinmc pushed a commit to justinmc/engine that referenced this pull request Apr 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

affects: text input platform-linux 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

Development

Successfully merging this pull request may close these issues.

3 participants