[text_input] prepare for custom text input sources #72803

jpnurmi · 2020-12-22T14:15:08Z

Description

The existing TextInputConnection class has been made abstract and method channel-agnostic. There is now a separate internal class, _TextInputChannelConnection, that implements the default type of text input connection that communicates with platform plugins via the text input system channel.

The main motivation of the separation is to allow custom text input sources that can replace the method channel-based implementation and for example, communicate with an in-app virtual keyboard on (embedded) platforms that do not have a system-provided keyboard.

To be able to eliminate direct method channel calls from the TextInput class, the following new methods have been introduced:

TextInput.detach()
TextInput.reset()
TextInputConnection.hide()
TextInputConnection.setClient()
TextInputConnection.clearClient()

Consequently, the following methods have been deprecated:

TextInputConnection.close() in favor of TextInput.detach()
TextInputConnection.connectionClosedReceived() in favor of TextInput.reset()

Related PRs

This PR was split off from #69146.

Related Issues

#68988

Tests

I added the following tests:

test/services/text_input_test.dart:
- TextInput message channels:
  - text input client is requested to hide on detach
  - old client is detached when a new client is attached
  - text input connection is reset

Checklist

Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.

jpnurmi · 2020-12-30T14:41:33Z

Rebased again in hopes to pass the CI (unrelated git fetch failures on macOS).

The existing TextInputConnection class has been made abstract and method channel-agnostic. There is now a separate internal class, _TextInputChannelConnection, that implements the default type of text input connection that communicates with platform plugins via the text input system channel. The main motivation of the separation is to allow custom text input sources that can replace the method channel-based implementation and for example, communicate with an in-app virtual keyboard on (embedded) platforms that do not have a keyboard provided by the system Ref: flutter#68988

justinmc

Thanks for splitting this out from #69146, should be easier to review and merge.

It looks like this could hypothetically break someone that was subclassing TextInputConnection. Do we know if anyone might be doing that? I've submitted this PR to have the internal Google tests run on it, and if it passes there then I'm more confident that it's not a problem.

CC @LongCatIsLooong since you reviewed #69146.

packages/flutter/lib/src/services/text_input.dart

…connection

jpnurmi · 2021-01-05T19:02:57Z

Thanks for the review!

It looks like this could hypothetically break someone that was subclassing TextInputConnection. Do we know if anyone might be doing that? I've submitted this PR to have the internal Google tests run on it, and if it passes there then I'm more confident that it's not a problem.

I believe it's not a breaking change because the constructor was private before, so it wasn't possible to create a TextInputConnection instance from the outside of text_input.dart.

justinmc · 2021-01-05T19:10:25Z

Ah I missed that, you're right! Should be fine then. I'm on board once all the tests pass.

packages/flutter/lib/src/services/text_input.dart

…connection

justinmc

LGTM but I'll wait for @LongCatIsLooong's final approval.

…connection

Later on, _TextInputSource will be made public, abstract and documented so that it can be used as a base class for custom input sources.

LongCatIsLooong

LGTM. Thank you!

LongCatIsLooong · 2021-01-14T18:34:32Z

packages/flutter/lib/src/services/text_input.dart

+  static MethodChannel? _channel;
+
+  static void setChannel(MethodChannel newChannel) {
+    _channel = newChannel..setMethodCallHandler(_handleTextInputInvocation);


should we call clean up on the previous channel?

This is only used in tests for setting up a testable environment via @visibleForTesting TextInput.setChannel(). This will be moved to the upcoming private default implementation of TextInputSource so it won't be visible to the outside.

packages/flutter/lib/src/services/text_input.dart

jpnurmi · 2021-01-14T21:59:01Z

Is the Google testing failure somehow related to this?

LongCatIsLooong · 2021-01-15T00:49:15Z

Doesn't seem to be the case. I have restarted the test.

LongCatIsLooong · 2021-01-15T05:39:03Z

The failure seems to be caused by something irrelevant. Could you rebase the branch to the latest commit and see if the failure goes away?

…connection

jpnurmi · 2021-01-15T09:06:34Z

Unrelated Mac build_tests failure. Let's try again.

This reverts commit b73722b.

…74349)

flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Dec 22, 2020

google-cla bot added the cla: yes label Dec 22, 2020

jpnurmi mentioned this pull request Dec 22, 2020

Add enablers for custom/in-app virtual keyboards #69146

Closed

10 tasks

jpnurmi force-pushed the text-input-channel-connection branch 2 times, most recently from 855ca1b to 4590fc3 Compare December 30, 2020 14:40

jpnurmi force-pushed the text-input-channel-connection branch from 4590fc3 to 138e868 Compare December 31, 2020 09:42

justinmc reviewed Jan 5, 2021

View reviewed changes

packages/flutter/lib/src/services/text_input.dart Outdated Show resolved Hide resolved

jpnurmi added 2 commits January 5, 2021 19:56

Merge remote-tracking branch 'origin/master' into text-input-channel-…

55a9118

…connection

[docs] fix spelling and deprecation version

ef9b289

goderbauer requested a review from justinmc January 6, 2021 22:54

LongCatIsLooong reviewed Jan 7, 2021

View reviewed changes

packages/flutter/lib/src/services/text_input.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/services/text_input.dart Outdated Show resolved Hide resolved

jpnurmi added 2 commits January 7, 2021 20:45

Merge remote-tracking branch 'origin/master' into text-input-channel-…

74c0024

…connection

[docs] restore original docs for deprecated methods

1aae859

justinmc approved these changes Jan 11, 2021

View reviewed changes

jpnurmi added 3 commits January 13, 2021 20:48

Merge remote-tracking branch 'origin/master' into text-input-channel-…

bd983d0

…connection

[text_input] eliminate remaining MethodChannel references in TextInput

f8ccbb1

Later on, _TextInputSource will be made public, abstract and documented so that it can be used as a base class for custom input sources.

Remove unused TextInput._setSource() for now to pass analysis

9e384ab

LongCatIsLooong approved these changes Jan 14, 2021

View reviewed changes

[docs] link TextInput.reset to TextInputClient.connectionClosed

77e188f

Merge remote-tracking branch 'origin/master' into text-input-channel-…

6edb588

…connection

jpnurmi force-pushed the text-input-channel-connection branch from d51a018 to 6edb588 Compare January 15, 2021 09:07

LongCatIsLooong added the waiting for tree to go green label Jan 15, 2021

fluttergithubbot merged commit b73722b into flutter:master Jan 15, 2021

jpnurmi deleted the text-input-channel-connection branch January 15, 2021 19:43

LongCatIsLooong mentioned this pull request Jan 20, 2021

App throws LateInitializationError when re-focusing text field #74347

Open

LongCatIsLooong added a commit that referenced this pull request Jan 20, 2021

Revert "[text_input] prepare for custom text input sources (#72803)"

429ad91

This reverts commit b73722b.

LongCatIsLooong mentioned this pull request Jan 20, 2021

Revert "[text_input] prepare for custom text input sources" #74349

Merged

fluttergithubbot pushed a commit that referenced this pull request Jan 20, 2021

Revert "[text_input] prepare for custom text input sources (#72803)" (#…

61299b1

…74349)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[text_input] prepare for custom text input sources #72803

[text_input] prepare for custom text input sources #72803

Uh oh!

jpnurmi commented Dec 22, 2020 •

edited

Loading

Uh oh!

jpnurmi commented Dec 30, 2020

Uh oh!

justinmc left a comment

Uh oh!

Uh oh!

jpnurmi commented Jan 5, 2021

Uh oh!

justinmc commented Jan 5, 2021

Uh oh!

Uh oh!

Uh oh!

justinmc left a comment

Uh oh!

LongCatIsLooong left a comment

Uh oh!

LongCatIsLooong Jan 14, 2021

Uh oh!

jpnurmi Jan 14, 2021

Uh oh!

Uh oh!

Uh oh!

jpnurmi commented Jan 14, 2021

Uh oh!

LongCatIsLooong commented Jan 15, 2021

Uh oh!

LongCatIsLooong commented Jan 15, 2021

Uh oh!

jpnurmi commented Jan 15, 2021

Uh oh!

Uh oh!

[text_input] prepare for custom text input sources #72803

[text_input] prepare for custom text input sources #72803

Uh oh!

Conversation

jpnurmi commented Dec 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related PRs

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

jpnurmi commented Dec 30, 2020

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jpnurmi commented Jan 5, 2021

Uh oh!

justinmc commented Jan 5, 2021

Uh oh!

Uh oh!

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong left a comment

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Jan 14, 2021

Choose a reason for hiding this comment

Uh oh!

jpnurmi Jan 14, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jpnurmi commented Jan 14, 2021

Uh oh!

LongCatIsLooong commented Jan 15, 2021

Uh oh!

LongCatIsLooong commented Jan 15, 2021

Uh oh!

jpnurmi commented Jan 15, 2021

Uh oh!

Uh oh!

jpnurmi commented Dec 22, 2020 •

edited

Loading