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.

Sign up for GitHub

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

Jump to bottom

[web] Paragraph.getBoxesForRange uses LineMetrics #16625

Merged
merged 3 commits into from
Feb 19, 2020
Merged

[web] Paragraph.getBoxesForRange uses LineMetrics #16625

merged 3 commits into from
Feb 19, 2020

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Feb 14, 2020

@mdebbar mdebbar added affects: text input platform-web Code specifically for the web engine labels Feb 14, 2020
@mdebbar mdebbar requested review from yjbanov and ferhatb February 14, 2020 22:33
@mdebbar mdebbar self-assigned this Feb 14, 2020
ferhatb
ferhatb approved these changes Feb 14, 2020
View reviewed changes
Copy link
Contributor

@ferhatb ferhatb left a comment

Choose a reason for hiding this comment

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

small nit, otherwise LGTM

lib/web_ui/lib/src/engine/text/paragraph.dart Outdated
return ui.TextBox.fromLTRBD(
left + line.left,
top,
line.width + line.left - right,
Copy link
Contributor

Choose a reason for hiding this comment

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

please add comment above explaining why -right

yjbanov
yjbanov approved these changes Feb 18, 2020
View reviewed changes
lib/web_ui/lib/src/engine/text/paragraph.dart
// |-------------|------------------|-------------|-----------------|
//
// ^^^^^^^^^^^^^
// This is the box we want to return.
Copy link
Contributor

Choose a reason for hiding this comment

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

Love the ASCII art docs 👍

lib/web_ui/test/paragraph_test.dart
@@ -426,6 +426,209 @@ void main() async {
expect(paragraph.getBoxesForRange(0, 0), isEmpty);
});

testEachMeasurement('getBoxesForRange multi-line', () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth adding a test for a ParagraphStyle with maxLines such that the start/end range fall outside the max line bounds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good corner case to test. I'd like to merge this PR while the tree is green though. So I'll add the maxLines test in a future PR.

@mdebbar mdebbar merged commit 8dba815 into flutter:master Feb 19, 2020
@dnfield dnfield mentioned this pull request Feb 19, 2020
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 20, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
5c543a7 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 20, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
abe49c3 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 21, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
55724e9 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 21, 2020
Closed
jason-simmons added a commit to jason-simmons/flutter that referenced this pull request Feb 21, 2020
@jason-simmons
Manual engine roll: 84dc383..5cef6d0
b7d3b1d 
2020-02-20 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from YPr0t... to CZTpy... (flutter/engine#16708)
2020-02-20 skia-flutter-autoroll@skia.org Roll src/third_party/skia 2c2db2762809..9d4e31d6cda5 (1 commits) (flutter/engine#16707)
2020-02-20 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from WZgbp... to 78ZcV... (flutter/engine#16706)
2020-02-20 skia-flutter-autoroll@skia.org Roll src/third_party/dart e187e42593e8..81d4cc6bc99a (3 commits) (flutter/engine#16705)
2020-02-20 jmccandless@google.com Fix the newline on some keyboards (flutter/engine#16560)
2020-02-20 kustermann@google.com Enable lazy-async-stacks by-default in all modes (flutter/engine#16556)
2020-02-20 skia-flutter-autoroll@skia.org Roll src/third_party/skia a5097354217b..2c2db2762809 (1 commits) (flutter/engine#16704)
2020-02-20 skia-flutter-autoroll@skia.org Roll src/third_party/skia ed1ff23c2768..a5097354217b (1 commits) (flutter/engine#16703)
2020-02-20 skia-flutter-autoroll@skia.org Roll src/third_party/dart 7469b87b042a..e187e42593e8 (11 commits) (flutter/engine#16702)
2020-02-20 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from VHyDa... to YPr0t... (flutter/engine#16701)
2020-02-20 skia-flutter-autoroll@skia.org Roll src/third_party/skia 9baef3593c3c..ed1ff23c2768 (5 commits) (flutter/engine#16699)
2020-02-20 skia-flutter-autoroll@skia.org Roll src/third_party/skia 7dfb46e7f397..9baef3593c3c (3 commits) (flutter/engine#16696)
2020-02-19 xster@google.com fix param (flutter/engine#16694)
2020-02-19 skia-flutter-autoroll@skia.org Roll src/third_party/dart 0f141be8bd52..7469b87b042a (9 commits) (flutter/engine#16693)
2020-02-19 skia-flutter-autoroll@skia.org Roll src/third_party/skia c5ff41f2976e..7dfb46e7f397 (20 commits) (flutter/engine#16691)
2020-02-19 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from _jvYk... to WZgbp... (flutter/engine#16692)
2020-02-19 dnfield@google.com Revert "Remove usage of Dart_AllocateWithNativeFields from tonic (flutter#16588)" (flutter/engine#16684)
2020-02-19 mouad.debbar@gmail.com [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/engine#16625)
2020-02-19 skia-flutter-autoroll@skia.org Roll src/third_party/dart 707ecda05e14..0f141be8bd52 (4 commits) (flutter/engine#16683)
@jason-simmons jason-simmons mentioned this pull request Feb 21, 2020
Closed
13 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
d6301fe 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 21, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
ebee667 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 21, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
ea38f45 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 21, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
a8f2e2d 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 21, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
94e9369 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 21, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
fefebd6 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 21, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
358773f 
…gine#16625)
This was referenced Feb 21, 2020
Closed
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
c750908 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 21, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
05c6eac 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 21, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
1e0e049 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 21, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
dc5bdc4 
…gine#16625)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020
@engine-flutter-autoroll
8dba815 [web] Paragraph.getBoxesForRange uses LineMetrics (flutter/en…
b45e891 
…gine#16625)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 21, 2020
Closed
NoamDev pushed a commit to NoamDev/engine that referenced this pull request Feb 27, 2020
@mdebbar @NoamDev
[web] Paragraph.getBoxesForRange uses LineMetrics (flutter#16625)
70767fb
NoamDev added a commit to NoamDev/engine that referenced this pull request Feb 27, 2020
@NoamDev
Revert "[web] Paragraph.getBoxesForRange uses LineMetrics (flutter#16625
e7216b8 
)"

This reverts commit 70767fb.
@mdebbar mdebbar deleted the boxes_align branch April 15, 2021 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ferhatb ferhatb ferhatb approved these changes

@yjbanov yjbanov yjbanov approved these changes

Assignees

@mdebbar mdebbar

Labels
affects: text input cla: yes platform-web Code specifically for the web engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Flutter web] Cursor text field don't move correctly
4 participants
@mdebbar @yjbanov @ferhatb @googlebot