Skip to content

Send text direction in selection rects - #117436

Merged
auto-submit[bot] merged 1 commit into
flutter:masterfrom
moffatman:selection_rects
Jan 4, 2023
Merged

Send text direction in selection rects#117436
auto-submit[bot] merged 1 commit into
flutter:masterfrom
moffatman:selection_rects

Conversation

@moffatman

@moffatmanmoffatman commented Dec 21, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

In order to properly calculate the closestPositionToPoint in FlutterTextInputPlugin.mm in the engine, we need to know the text direction in each selectionrect.

Needed for #30476

Needed for flutter-team-archive/engine#36643

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], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboardflutter-dashboardBot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. labels Dec 21, 2022
LongCatIsLooong
LongCatIsLooong reviewed Dec 21, 2022
View reviewed changes

@LongCatIsLooongLongCatIsLooong left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The change LGTM but you might have missed some getBoxesForSelection call sites now that the return type is different (there're failing tests). Let's see if this is a breaking change.

Comment threadpackages/flutter/lib/src/services/text_input.dart Outdated
@hellohuanlinhellohuanlin mentioned this pull request Dec 22, 2022
Merged
8 tasks
@goderbauer

goderbauer commented Jan 3, 2023

Copy link
Copy Markdown
Member

(Triage) @moffatman Looks like there are some conflicts. Could you resolve those? Is this ready to land then?

@moffatmanmoffatman added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 4, 2023
@auto-submit
auto-submitBot merged commit c53501d into flutter:masterJan 4, 2023
This was referenced Jan 4, 2023
Closed
Merged
Merged
hellohuanlin
hellohuanlin reviewed May 17, 2023
View reviewed changes
Comment threadpackages/flutter/lib/src/services/text_input.dart
const SelectionRect({
required this.position,
required this.bounds,
this.direction = TextDirection.ltr,

@hellohuanlinhellohuanlinMay 17, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe better to remove the default value here in case we incorrectly assume the writing direction?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hellohuanlinhellohuanlinhellohuanlin left review comments
@LongCatIsLooongLongCatIsLooongLongCatIsLooong approved these changes

Assignees

No one assigned

Labels

a: text inputEntering text in a text field or keyboard related problemsautosubmitMerge PR when tree becomes green via auto submit Appframeworkflutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@moffatman@goderbauer@LongCatIsLooong@hellohuanlin