Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respond to AndroidView focus events. #33901

Merged
merged 2 commits into from Jun 6, 2019

Conversation

@amirh
Copy link
Contributor

commented Jun 5, 2019

Description

When an AndroidView gains focus we invoke the(newly introduced)
'TextInput.setPlatformViewClient' text_input system channel method
which sets the platform view as the text input target.

When the AndroidView loses focus we send a clearFocus message to
platform views system channel(so the engine can clear the focus from
the platform view).

This PR is going to land before the engine implementation is rolled to
the framework, we swallow MissingPluginException for the newly
introduced method channel methods so this is a no-op before the engine
is ready(after the engine is rolled with the corresponding change I'll
remove the logic to swallow the exceptions).

The engine counterpart is in: flutter/engine#9203

Related Issues

#19718

Tests

I added the following tests:

  • AndroidView sets a platform view text input client when focused
  • AndroidView clears platform focus when unfocused

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 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

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read [Handling breaking changes]). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.
Respond to AndroidView focus events.
When an AndroidView gains focus we invoke the(newly introduced)
'TextInput.setPlatformViewClient' text_input system channel method
which sets the platform view as the text input target.

When the AndroidView loses focus we send a `clearFocus` message to
platform views system channel(so the engine can clear the focus from
the platform view).

This PR is going to land before the engine implementation is rolled to
the framework, we swallow MissingPluginException for the newly
introduced methdo channel methods so this is a no-op before the engine
is ready(after the engine is rolled with the corresponding change I'll
remove the logic to swallow the exceptions).

@amirh amirh requested a review from gspencergoog Jun 5, 2019

@googlebot googlebot added the cla: yes label Jun 5, 2019

@amirh

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2019

cc @jason-simmons (for context when reviewing the corresponding engine PR)

@phanirithvij

This comment has been minimized.

Copy link

commented Jun 5, 2019

Please merge this as soon as possible.
I've been waiting for this to get fixed as we're planning on using Webviews for login and signup in our application.
And this is what's stopping us from using flutter.
I don't think we can wait until October.

@gspencergoog
Copy link
Contributor

left a comment

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

Update packages/flutter/test/services/fake_platform_views.dart
Co-Authored-By: Greg Spencer <gspencergoog@users.noreply.github.com>
@amirh

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2019

Waiting for flutter/engine#9203 to be approved before landing this one.
(This PR will land before the engine PR, but I'm waiting to see the method channel protocol is approved on both sides)

@sakina1403

This comment has been minimized.

Copy link

commented Jun 6, 2019

Waiting for flutter/engine#9203 to be approved before landing this one.
(This PR will land before the engine PR, but I'm waiting to see the method channel protocol is approved on both sides)

Is there an ETA??? Waiting desperately for this since March. Tired of using workarounds. We really love Flutter but this has been holding me back since a long time. Our App has heavy use of WebView and a lot of JS communication as well and this is the only plugin perfect for postMessage Communication. We have 100K+ downloads but usability is affecting largely because of this issue.

Thank you for all your efforts @amirh . Unless and until it's reaally necessary to wait please please merge this ASAP.

@amirh amirh merged commit 7d27550 into flutter:master Jun 6, 2019

35 checks passed

WIP Ready for review
Details
add2app-macos Task Summary
Details
add2app-macos
Details
analyze Task Summary
Details
analyze
Details
build_tests-linux Task Summary
Details
build_tests-linux
Details
build_tests-macos Task Summary
Details
build_tests-macos
Details
build_tests-windows Task Summary
Details
build_tests-windows
Details
cla/google All necessary CLAs are signed
deploy_gallery Task Summary
Details
deploy_gallery
Details
deploy_gallery-macos Task Summary
Details
deploy_gallery-macos
Details
docs Task Summary
Details
docs
Details
flutter-build
Details
integration_tests-linux Task Summary
Details
integration_tests-linux
Details
integration_tests-windows Task Summary
Details
integration_tests-windows
Details
tests-linux Task Summary
Details
tests-linux
Details
tests-macos Task Summary
Details
tests-macos
Details
tests-windows Task Summary
Details
tests-windows
Details
tool_tests-linux Task Summary
Details
tool_tests-linux
Details
tool_tests-macos Task Summary
Details
tool_tests-macos
Details
tool_tests-windows Task Summary
Details
tool_tests-windows
Details

@amirh amirh deleted the amirh:android_view_keyboard branch Jun 6, 2019

amirh added a commit to flutter/engine that referenced this pull request Jun 7, 2019

Keyboard support for embedded Android views. (#9203)
Generally what this PR is doing is setting up a delegation mechanism
for Android's onCreateInputConnection.

It works by letting the framework know when an embedded view gets loses
focus(within the virtual display), the framework maintains a focus node
for each Android view that is kept in sync with the focus state of the
embedded view.

The TextInputPlugin is extended to allow for 2 type of text clients a
"framework client"(what we had before) and a "platform view client".
When the AndroidView's focus node in the framework is focused the
framework sets a "platform view text client" for the TextInputPlugin,
which will result in the TextInputPlugin delegating
createInputConnection to the platform view.

When a platform view is resized, we are detaching it from a virtual
display and attaching it to a new one, as a side affect a platform view
might lose an active input connection, to workaround that we "lock" the
connection when resizing(by caching it and forcing the cached copy until
the resize is done).

Additional things worth calling out in this PR:

To properly answer which views are allowed for input connection
proxying we compare a candidate view's root view to the set of root
views of all virtual displays.
We also preserve a view's focus state across resizes.
Note that this PR only wires text for the io.flutter.view.FlutterView
For the new Android embedding some additional plumbing is necessary.

Corresponding framework PR: flutter/flutter#33901

flutter/flutter#19718

tango5614 added a commit to tango5614/flutter that referenced this pull request Jun 7, 2019

Merge branch 'master' into fix28096
* master: (25 commits)
  Increase daemon protocol version for getSupportedPlatforms (flutter#33980)
  skip web test on crazy import (flutter#34017)
  Compatibility pass on flutter/foundation tests for JavaScript compilation. (1) (flutter#33349)
  0602dbb27 Roll src/third_party/dart 9dcb026b26..6e0d978505 (72 commits) (flutter#34027)
  Add chrome stable to dockerfile and web shard (flutter#33787)
  Codegen an entrypoint for flutter web applications (flutter#33956)
  Revert "Reland "Text inline widgets, TextSpan rework" (flutter#33946)" (flutter#34002)
  Revert "Re-add deprecated method for plugin migration compatibility. (flutter#34006)" (flutter#34022)
  Remove print (flutter#34004)
  Manual roll the engine to land the timing API (flutter#33989)
  Make plugins Swift-first on macOS (flutter#33997)
  Re-add deprecated method for plugin migration compatibility. (flutter#34006)
  make sure version check includes hotfixes (flutter#33459)
  Respond to AndroidView focus events. (flutter#33901)
  Add 'doctor' support for Windows (flutter#33872)
  Add use_frameworks to macOS Podfile template (flutter#33987)
  [Material] Create a themable Range Slider (continuous and discrete) (flutter#31681)
  Updating names to correct versioning convention (flutter#33865)
  Whitelist adb.exe heap corruption exit code. (flutter#33951)
  [flutter_tool] Fix 'q' for Fuchsia profile/debug mode (flutter#33846)
  ...

huqiuser pushed a commit to huqiuser/engine that referenced this pull request Jun 12, 2019

Keyboard support for embedded Android views. (flutter#9203)
Generally what this PR is doing is setting up a delegation mechanism
for Android's onCreateInputConnection.

It works by letting the framework know when an embedded view gets loses
focus(within the virtual display), the framework maintains a focus node
for each Android view that is kept in sync with the focus state of the
embedded view.

The TextInputPlugin is extended to allow for 2 type of text clients a
"framework client"(what we had before) and a "platform view client".
When the AndroidView's focus node in the framework is focused the
framework sets a "platform view text client" for the TextInputPlugin,
which will result in the TextInputPlugin delegating
createInputConnection to the platform view.

When a platform view is resized, we are detaching it from a virtual
display and attaching it to a new one, as a side affect a platform view
might lose an active input connection, to workaround that we "lock" the
connection when resizing(by caching it and forcing the cached copy until
the resize is done).

Additional things worth calling out in this PR:

To properly answer which views are allowed for input connection
proxying we compare a candidate view's root view to the set of root
views of all virtual displays.
We also preserve a view's focus state across resizes.
Note that this PR only wires text for the io.flutter.view.FlutterView
For the new Android embedding some additional plumbing is necessary.

Corresponding framework PR: flutter/flutter#33901

flutter/flutter#19718

Kiku-Reise added a commit to Kiku-Reise/flutter that referenced this pull request Jun 14, 2019

Respond to AndroidView focus events. (flutter#33901)
When an AndroidView gains focus we invoke the(newly introduced)
'TextInput.setPlatformViewClient' text_input system channel method
which sets the platform view as the text input target.

When the AndroidView loses focus we send a clearFocus message to
platform views system channel(so the engine can clear the focus from
the platform view).

This PR is going to land before the engine implementation is rolled to
the framework, we swallow MissingPluginException for the newly
introduced method channel methods so this is a no-op before the engine
is ready(after the engine is rolled with the corresponding change I'll
remove the logic to swallow the exceptions).

The engine counterpart is in: flutter/engine#9203

goderbauer added a commit to goderbauer/flutter that referenced this pull request Jul 3, 2019

Respond to AndroidView focus events. (flutter#33901)
When an AndroidView gains focus we invoke the(newly introduced)
'TextInput.setPlatformViewClient' text_input system channel method
which sets the platform view as the text input target.

When the AndroidView loses focus we send a clearFocus message to
platform views system channel(so the engine can clear the focus from
the platform view).

This PR is going to land before the engine implementation is rolled to
the framework, we swallow MissingPluginException for the newly
introduced method channel methods so this is a no-op before the engine
is ready(after the engine is rolled with the corresponding change I'll
remove the logic to swallow the exceptions).

The engine counterpart is in: flutter/engine#9203

wxCUHK added a commit to wxCUHK/engine that referenced this pull request Jul 4, 2019

Keyboard support for embedded Android views. (flutter#9203)
Generally what this PR is doing is setting up a delegation mechanism
for Android's onCreateInputConnection.

It works by letting the framework know when an embedded view gets loses
focus(within the virtual display), the framework maintains a focus node
for each Android view that is kept in sync with the focus state of the
embedded view.

The TextInputPlugin is extended to allow for 2 type of text clients a
"framework client"(what we had before) and a "platform view client".
When the AndroidView's focus node in the framework is focused the
framework sets a "platform view text client" for the TextInputPlugin,
which will result in the TextInputPlugin delegating
createInputConnection to the platform view.

When a platform view is resized, we are detaching it from a virtual
display and attaching it to a new one, as a side affect a platform view
might lose an active input connection, to workaround that we "lock" the
connection when resizing(by caching it and forcing the cached copy until
the resize is done).

Additional things worth calling out in this PR:

To properly answer which views are allowed for input connection
proxying we compare a candidate view's root view to the set of root
views of all virtual displays.
We also preserve a view's focus state across resizes.
Note that this PR only wires text for the io.flutter.view.FlutterView
For the new Android embedding some additional plumbing is necessary.

Corresponding framework PR: flutter/flutter#33901

flutter/flutter#19718
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.