Positioning IME bars on iOS by LongCatIsLooong · Pull Request #61981 · flutter/flutter

LongCatIsLooong · 2020-07-21T21:36:22Z

Description

Engine: flutter-team-archive/engine#19929

Keep sending marked text rect / caret rect information for the iOS text input plugin to consume.
Turning this into a real PR when #61940 is merged .

Related Issues

Depends on #61940
Fixes #9344

Tests

I added the following tests:

the "setMarkedTextRect" group

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.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: https://flutter.dev/go/breaking-changes-template Replace this with a link to a pull request that adds the migration guide to https://flutter.dev/docs/release/breaking-changes

justinmc

LGTM, super excited for this.

justinmc · 2020-07-29T16:29:29Z

Why is a 1 frame delay needed here?

The information is sent to the platform every frame if needed.

justinmc · 2020-07-29T16:29:57Z

Suggested change

// No composing text so the width should not be too wide becomes

// No composing text so the width should not be too wide because

GaryQian · 2020-07-30T21:57:15Z

Can this lag cause jitter when the true information is received? 1 frame swapping between an estimate and true info may be a source of jank.

The "true" information does not trigger a UI update of any sort when it's sent to the platform. It will be used as an estimate the next time iOS requests this information.

GaryQian · 2020-07-30T22:02:37Z

For multiline text, first and last may not get you the bounding boxes you expect for the left and right positions

We should add a test for this case as well.

Could you elaborate a bit on this? I would expect the marked text to be continuous and has a consistent writing direction, because it's produced by the same IME. Is it because the text within the given TextRange could be using different fonts so they might be grouped in different TextBoxes even if they are on the same line?

I guess the line can have more than 1 run.

LongCatIsLooong · 2020-08-14T21:18:19Z

@GaryQian @justinmc updated. Should be ready for review now.

justinmc

LGTM 👍

justinmc · 2020-08-14T23:12:48Z

Suggested change

// invalid range.

// Invalid range.

justinmc · 2020-08-14T23:12:55Z

Suggested change

// collapsed range.

// Collapsed range.

GaryQian

LGTM, thanks!

LongCatIsLooong · 2020-09-09T21:53:32Z

Waiting for the release to be cut.

skia-gold · 2020-09-17T02:06:43Z

Gold has detected about 3 untriaged digest(s) on patchset 16.
View them at https://flutter-gold.skia.org/cl/github/61981

justinmc

New changes LGTM 👍

justinmc · 2020-09-18T21:47:37Z

  /// This property is typically used to notify the renderer of input gestures
  /// when [RenderEditable.ignorePointer] is true.
-  RenderEditable? get renderEditable => _editableKey.currentContext!.findRenderObject() as RenderEditable?;
+  RenderEditable get renderEditable => _editableKey.currentContext!.findRenderObject()! as RenderEditable;


Nice way of getting rid of all of the exclamation marks elsewhere in the file. I'm still figuring out all of the NNBD stuff but it's looking pretty slick.

fluttergithubbot · 2020-09-22T17:02:06Z

This pull request is not suitable for automatic merging in its current state.

The status or check suite Mac framework_tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

fluttergithubbot · 2020-09-22T18:32:05Z

This pull request is not suitable for automatic merging in its current state.

The status or check suite Linux web_tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

fluttergithubbot added framework flutter/packages/flutter repository. See also f: labels. work in progress; do not review labels Jul 21, 2020

googlebot added the cla: yes label Jul 21, 2020

LongCatIsLooong mentioned this pull request Jul 22, 2020

Implement iOS [UITextInput firstRectForRange:] with markedText flutter-team-archive/engine#19929

Merged

7 of 12 tasks

LongCatIsLooong force-pushed the ime-tooltip branch from f3c1ac4 to 4ba45f7 Compare July 22, 2020 17:38

LongCatIsLooong requested a review from justinmc July 28, 2020 23:03

LongCatIsLooong marked this pull request as ready for review July 28, 2020 23:04

LongCatIsLooong requested a review from GaryQian July 28, 2020 23:04

justinmc approved these changes Jul 29, 2020

View reviewed changes

GaryQian reviewed Jul 30, 2020

View reviewed changes

LongCatIsLooong removed the work in progress; do not review label Jul 31, 2020

goderbauer requested a review from GaryQian August 5, 2020 21:44

justinmc approved these changes Aug 14, 2020

View reviewed changes

GaryQian approved these changes Sep 1, 2020

View reviewed changes

Piinks added a: fidelity Matching the OEM platforms better a: internationalization Supporting other languages or locales. (aka i18n) a: text input Entering text in a text field or keyboard related problems labels Sep 2, 2020

reapply diff

57e005f

LongCatIsLooong force-pushed the ime-tooltip branch from 40d0013 to 57e005f Compare September 17, 2020 02:26

justinmc mentioned this pull request Sep 17, 2020

[ReorderableListView] remove extra margin added after picking up the item #65080

Merged

9 tasks done

Merge remote-tracking branch 'upstream/master' into ime-tooltip

d58bc31

LongCatIsLooong force-pushed the ime-tooltip branch 3 times, most recently from 38e3b15 to d9e9759 Compare September 18, 2020 05:15

fix web tests

96ad2f1

LongCatIsLooong force-pushed the ime-tooltip branch from d9e9759 to 96ad2f1 Compare September 18, 2020 06:56

Merge remote-tracking branch 'upstream/master' into ime-tooltip

a5495eb

justinmc approved these changes Sep 18, 2020

View reviewed changes

LongCatIsLooong added the waiting for tree to go green label Sep 22, 2020

fluttergithubbot removed the waiting for tree to go green label Sep 22, 2020

Merge remote-tracking branch 'upstream/master' into ime-tooltip

0891285

LongCatIsLooong added the waiting for tree to go green label Sep 22, 2020

fluttergithubbot removed the waiting for tree to go green label Sep 22, 2020

Merge remote-tracking branch 'upstream/master' into ime-tooltip

ccd1dc1

LongCatIsLooong added the waiting for tree to go green label Sep 23, 2020

fluttergithubbot merged commit 525e76b into flutter:master Sep 23, 2020

LongCatIsLooong deleted the ime-tooltip branch September 23, 2020 01:35

	// No composing text so the width should not be too wide becomes
	// No composing text so the width should not be too wide because

Conversation

LongCatIsLooong commented Jul 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Aug 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong commented Aug 14, 2020

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GaryQian left a comment

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong commented Sep 9, 2020

Uh oh!

skia-gold commented Sep 17, 2020

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fluttergithubbot commented Sep 22, 2020

Uh oh!

fluttergithubbot commented Sep 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

LongCatIsLooong commented Jul 21, 2020 •

edited

Loading

LongCatIsLooong Aug 13, 2020 •

edited

Loading