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

[iOS] Add iOS live text input engine side support #34751

Merged
merged 6 commits into from
Jul 27, 2022

Conversation

luckysmg
Copy link
Contributor

@luckysmg luckysmg commented Jul 19, 2022

Preview:

normal.video.mp4

Related PR in flutter framework (WIP)
flutter/flutter#96637

List which issues are fixed by this PR. You must list at least one issue.
flutter/flutter#91896

This PR will let the framework side query the device live text input ability.(See framework PR classLiveText)

If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].

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.

@luckysmg luckysmg changed the title Add iOS live text input support [iOS] Add iOS live text input support Jul 19, 2022
@luckysmg luckysmg changed the title [iOS] Add iOS live text input support [iOS] Add iOS live text input engine side support Jul 20, 2022
@luckysmg luckysmg requested a review from jmagman July 25, 2022 11:47
Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

LGTM!
cc @LongCatIsLooong re: #29229

@luckysmg luckysmg requested a review from cyanglaz July 25, 2022 22:25
Copy link
Contributor

@LongCatIsLooong LongCatIsLooong left a comment

Choose a reason for hiding this comment

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

LGTM. Any reason for implementing isLiveTextInputAvailable in the platform plugin instead of the text input plugin?

@luckysmg
Copy link
Contributor Author

luckysmg commented Jul 25, 2022

Hi @LongCatIsLooong
In my op

  1. LiveText is a kind of platform ability (just like clipboard).Maybe there are more functions will be added in the future
    (eg:Maybe like recognizing the text in images on iOS16, that is not much related with text input). So Adding it in FlutterTextInputPlugin may be not so appropriate.

  2. Developers may query this value any time even user is not inputing in any textField (So activeView in FlutterTextInputPlugin is nil and we will still create another UITextField to get this value), which is not so elegant.

So I think it is more appropriate to put it in platform plugin.
You can See the framework interface LiveText I wrote in framework PR:
packages/flutter/lib/src/services/live_text.dart

Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM

@luckysmg
Copy link
Contributor Author

I merged main to this branch and test passed.
Shall we merge this? @jmagman @cyanglaz @LongCatIsLooong

@LongCatIsLooong LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 27, 2022
@auto-submit auto-submit bot merged commit 8a7a359 into flutter:main Jul 27, 2022
@luckysmg luckysmg deleted the ios_ocr branch July 27, 2022 07:48
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 27, 2022
betrevisan pushed a commit to betrevisan/engine that referenced this pull request Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-ios
Projects
None yet
4 participants