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

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

Merged
auto-submit merged 6 commits into from
Jul 27, 2022
Merged

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

auto-submit merged 6 commits into from
Jul 27, 2022

Conversation

@luckysmg
Copy link
Contributor

@luckysmg luckysmg commented Jul 19, 2022 •
edited
Loading

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
This was referenced Jul 19, 2022
Closed
Merged
@luckysmg luckysmg changed the title [iOS] Add iOS live text input support [iOS] Add iOS live text input engine side support Jul 20, 2022
jmagman
jmagman reviewed Jul 20, 2022
View reviewed changes
@luckysmg luckysmg requested a review from jmagman July 25, 2022 11:47
jmagman
jmagman approved these changes Jul 25, 2022
View reviewed changes
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

@jmagman jmagman requested a review from LongCatIsLooong July 25, 2022 22:02
cyanglaz
cyanglaz suggested changes Jul 25, 2022
View reviewed changes
@luckysmg luckysmg requested a review from cyanglaz July 25, 2022 22:25
LongCatIsLooong
LongCatIsLooong approved these changes Jul 25, 2022
View reviewed changes
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 •
edited
Loading

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

cyanglaz
cyanglaz approved these changes Jul 26, 2022
View reviewed changes
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

luckysmg commented Jul 27, 2022

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 engine-flutter-autoroll mentioned this pull request Jul 27, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 27, 2022
@engine-flutter-autoroll
8a7a359 [iOS] Add iOS live text input engine side support (flutter/en…
7f1efb4 
…gine#34751)
betrevisan pushed a commit to betrevisan/engine that referenced this pull request Jul 29, 2022
@luckysmg @betrevisan
[iOS] Add iOS live text input engine side support (flutter#34751)
a1ceb9c
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@jmagman jmagman jmagman approved these changes

@cyanglaz cyanglaz cyanglaz approved these changes

@LongCatIsLooong LongCatIsLooong LongCatIsLooong approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-ios

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@luckysmg @jmagman @cyanglaz @LongCatIsLooong