This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Add focus support for platform view #33093
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hellohuanlin
force-pushed
the
huan_platform_view_focus_in_engine
branch
3 times, most recently
from
0df0bcf to
1519063
Compare
hellohuanlin
changed the title
hellohuanlin
changed the title
hellohuanlin
force-pushed
the
huan_platform_view_focus_in_engine
branch
from
1b606bd to
7372ec3
Compare
jmagman
reviewed
May 25, 2022
shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm
Outdated
Show resolved
Hide resolved
cyanglaz
reviewed
May 25, 2022
| // Caveat: | ||
| // 1. This detection logic does not cover the scenario when a platform view becomes | ||
| // first responder without any flutter text input resigning its first responder status | ||
| // (e.g. user tapping on platform view first). For now it works fine because there can only be |
There was a problem hiding this comment.
Just curious:
Are we expecting an UIKit update that will introduce multiple first responders?
There was a problem hiding this comment.
I should probably rephrase it clearly - I am not expecting UIKit change. What I meant to say is that right now the text input plugin does not need to keep track of the platform view focus state, but in the future we may need to track it (not because of UIKit change, but for potential other unknown reasons)
shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm
Outdated
Show resolved
Hide resolved
Comment on lines
2202
to
2212
| - (void)setPlatformViewTextInputClient:(long)platformViewID { | ||
| // No need to track the platformViewID for now (unlike in Android), because in iOS there can | ||
| // only be one single first responder. When a platform view becomes first responder, hide | ||
| // this dummy text input view (`_activeView`) for the previously focused widget. | ||
| [self removeEnableFlutterTextInputViewAccessibilityTimer]; | ||
| _activeView.accessibilityEnabled = NO; | ||
| [_activeView removeFromSuperview]; | ||
| [_inputHider removeFromSuperview]; | ||
| } | ||
|
|
There was a problem hiding this comment.
This looks fine to me.
cc @justinmc, do you mind take a quick look at this to see if it makes sense?
There was a problem hiding this comment.
@justinmc could you take a quick look? thanks
There was a problem hiding this comment.
im gonna land it first since i've tested it many times, and it's not used just yet (until the framework part is complete), but let me know if any thing comes up
shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm
Outdated
Show resolved
Hide resolved
shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm
Outdated
Show resolved
Hide resolved
hellohuanlin
force-pushed
the
huan_platform_view_focus_in_engine
branch
from
e66b307 to
26d1faf
Compare
hellohuanlin
added
the
waiting for tree to go green
|
This pull request is not suitable for automatic merging in its current state.
|
fluttergithubbot
removed
the
waiting for tree to go green
hellohuanlin
added
the
waiting for tree to go green
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
May 28, 2022
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
May 28, 2022
8 tasks
houhuayong
pushed a commit
to houhuayong/engine
that referenced
this pull request
Jun 21, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR continues the work to add Focus support for platform view. It mainly contains 2 parts:
I wrote up this proposal to provide more context such as different approaches to detect first responder events.
List which issues are fixed by this PR. You must list at least one issue.
flutter/flutter#34187
The previous PR in framework repo is here
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.