`RenderParagraph`s `_SelectableFragment.boundingBoxes` should consider max line height by Renzo-Olivares · Pull Request #155892 · flutter/flutter

Renzo-Olivares · 2024-09-29T00:45:42Z

This change updates the _SelectableFragment.boundingBoxes logic to consider the max line height. Previously we were using boxes that were tightly bound around each glyph, so you would have to click within the bounds of the glyph for double tap to select word to work. This is different than SelectableText which considers the max line height, as well as the native web behavior that also considers the max line height.

Web

Screen.Recording.2024-09-28.at.6.05.47.PM.mov

Flutter SelectableText

Screen.Recording.2024-09-30.at.10.05.11.AM.mov

Flutter Text widget under SelectionArea

Screen.Recording.2024-09-30.at.10.07.32.AM.mov

After this change, Flutter's Text widget under a SelectionArea now matches the SelectableText and native web behavior.

This change also:

Invalidates the cached bounding boxes when the paragraph layout changes.
Updates textOffsetToPosition to consider preferredLineHeight. In cases when the text wraps, it was sometimes inaccurate.

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].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

Renzo-Olivares · 2024-09-30T16:26:27Z

packages/flutter/test/widgets/selectable_region_test.dart

-  final Offset localOffset = paragraph.getOffsetForCaret(TextPosition(offset: offset), caret);
-  return paragraph.localToGlobal(localOffset);
+  final Offset localOffset = paragraph.getOffsetForCaret(TextPosition(offset: offset), caret) + Offset(0.0, paragraph.preferredLineHeight);
+  return paragraph.localToGlobal(localOffset) + const Offset(kIsWeb ? 1.0 : 0.0, -2.0);


I borrowed this from

flutter/packages/flutter/test/widgets/editable_text_utils.dart

Line 50 in 84f6568

return endpoints[0].point + const Offset(kIsWeb? 1.0 : 0.0, -2.0);

, I couldn't really track down why this change was needed but it originally came from #4397

Interesting, well I guess we should keep it around 🤷 .

…ayout change

This reverts commit 810fd9b.

justinmc

LGTM 👍

justinmc · 2024-09-30T17:50:52Z

packages/flutter/test/widgets/selectable_region_test.dart

-  final Offset localOffset = paragraph.getOffsetForCaret(TextPosition(offset: offset), caret);
-  return paragraph.localToGlobal(localOffset);
+  final Offset localOffset = paragraph.getOffsetForCaret(TextPosition(offset: offset), caret) + Offset(0.0, paragraph.preferredLineHeight);
+  return paragraph.localToGlobal(localOffset) + const Offset(kIsWeb ? 1.0 : 0.0, -2.0);


Interesting, well I guess we should keep it around 🤷 .

justinmc · 2024-09-30T17:51:38Z

packages/flutter/lib/src/rendering/paragraph.dart

+  /// An estimate of the height of a line in the text. See [TextPainter.preferredLineHeight].
+  /// This does not require the layout to be updated.


Nit: Empty line of /// between these two lines, and the first line should be only 1 sentence.

chunhtai

LGTM

chunhtai · 2024-09-30T17:59:19Z

packages/flutter/lib/src/rendering/paragraph.dart


+  /// An estimate of the height of a line in the text. See [TextPainter.preferredLineHeight].
+  /// This does not require the layout to be updated.
+  double get preferredLineHeight => _textPainter.preferredLineHeight;


add @visibleForTest?

RenderEditable has this API public, I don't have a strong preference either way, what do you think?

I will add it just to be conservative, we can always move to public in the future, but not backward.

and if someone does need it, we can then ask for use case and figure out whether this is really appropriate

Sounds good to me!

bleroux

So happy to see this fixed 🎉 , thanks!

…r max line height (flutter#155892) Fixes flutter#133637 This change updates the `_SelectableFragment.boundingBoxes` logic to consider the max line height. Previously we were using boxes that were tightly bound around each glyph, so you would have to click within the bounds of the glyph for double tap to select word to work. This is different than `SelectableText` which considers the max line height, as well as the native web behavior that also considers the max line height. ## Web https://github.com/user-attachments/assets/4ce8c0ca-ec6f-4969-88b1-baa356be8278 ## Flutter SelectableText https://github.com/user-attachments/assets/54c22ad3-75d7-475b-856b-e9b2dbe09d54 ## Flutter Text widget under SelectionArea https://github.com/user-attachments/assets/27db0e2b-1d19-43cc-8ab3-16050e3a5bc7 After this change, Flutter's Text widget under a SelectionArea now matches the SelectableText and native web behavior. This change also: * Invalidates the cached bounding boxes when the paragraph layout changes. * Updates `textOffsetToPosition` to consider `preferredLineHeight`. In cases when the text wraps, it was sometimes inaccurate.

… consider max line height (flutter/flutter#155892)

Manual roll Flutter from ead6b0d to 6bba08c (37 revisions) Manual roll requested by [email protected] flutter/flutter@ead6b0d...6bba08c 2024-10-01 [email protected] Feat: Add opportunity to change CupertinoTextField suffix alignment (flutter/flutter#154601) 2024-10-01 [email protected] Roll Flutter Engine from da28db8ff41d to 3fdb546bf595 (2 revisions) (flutter/flutter#155993) 2024-10-01 [email protected] Roll Flutter Engine from 6b21b796cc94 to da28db8ff41d (1 revision) (flutter/flutter#155985) 2024-10-01 [email protected] Roll Flutter Engine from 259f56c6e91b to 6b21b796cc94 (1 revision) (flutter/flutter#155983) 2024-10-01 [email protected] Roll Flutter Engine from 6ee04ed763d9 to 259f56c6e91b (1 revision) (flutter/flutter#155981) 2024-10-01 [email protected] Roll Flutter Engine from bfb6dddb2b30 to 6ee04ed763d9 (3 revisions) (flutter/flutter#155978) 2024-10-01 [email protected] Roll Flutter Engine from e61bc853acb2 to bfb6dddb2b30 (8 revisions) (flutter/flutter#155967) 2024-10-01 [email protected] Disable flaky menu test (flutter/flutter#155968) 2024-10-01 [email protected] Fix crash in Linux platform channel example. (flutter/flutter#155735) 2024-09-30 [email protected] Fix leak in input_decorator [prod-leak-fix] (flutter/flutter#155885) 2024-09-30 [email protected] Move platform specific text selection behavior out of styled TextField classes (flutter/flutter#155774) 2024-09-30 [email protected] Roll Flutter Engine from b466a0dd7834 to e61bc853acb2 (5 revisions) (flutter/flutter#155952) 2024-09-30 [email protected] `RenderParagraph`s `_SelectableFragment.boundingBoxes` should consider max line height (flutter/flutter#155892) 2024-09-30 [email protected] Roll Packages from 0321757 to 27c9853 (3 revisions) (flutter/flutter#155945) 2024-09-30 [email protected] Roll Flutter Engine from f4507e7a4beb to b466a0dd7834 (1 revision) (flutter/flutter#155944) 2024-09-30 [email protected] when `ResidentRunner.tryInitLogReader` fails, only log warning on Android (flutter/flutter#155800) 2024-09-30 [email protected] Move FlutterLogo from material to widget (flutter/flutter#155864) 2024-09-30 [email protected] fix: support android 15 16k page size for template plugin_ffi (flutter/flutter#155508) 2024-09-30 [email protected] Roll Flutter Engine from daf126b38b8f to f4507e7a4beb (1 revision) (flutter/flutter#155932) 2024-09-30 [email protected] Roll Flutter Engine from 734205fbcd62 to daf126b38b8f (1 revision) (flutter/flutter#155929) 2024-09-30 [email protected] Roll Flutter Engine from 338f09c4ea72 to 734205fbcd62 (1 revision) (flutter/flutter#155923) 2024-09-30 [email protected] Roll Flutter Engine from 897f5caffe2d to 338f09c4ea72 (2 revisions) (flutter/flutter#155917) 2024-09-30 [email protected] Roll Flutter Engine from 569abc4044b8 to 897f5caffe2d (1 revision) (flutter/flutter#155912) 2024-09-29 [email protected] Roll Flutter Engine from c4784aa7eade to 569abc4044b8 (2 revisions) (flutter/flutter#155894) 2024-09-28 [email protected] Roll Flutter Engine from ff4541712df4 to c4784aa7eade (2 revisions) (flutter/flutter#155889) 2024-09-28 [email protected] Fixes column text width calculation in CupertinoDatePicker (flutter/flutter#151128) 2024-09-28 [email protected] Roll Flutter Engine from 380fd814448c to ff4541712df4 (1 revision) (flutter/flutter#155886) 2024-09-28 [email protected] Optimize `Overlay` sample to avoid overflow (flutter/flutter#155861) 2024-09-28 [email protected] Roll Flutter Engine from f3b11bcd9c37 to 380fd814448c (1 revision) (flutter/flutter#155876) 2024-09-28 [email protected] Roll Flutter Engine from 9c8e5cb226e4 to f3b11bcd9c37 (3 revisions) (flutter/flutter#155865) 2024-09-28 [email protected] Roll Flutter Engine from f9e4ed28f103 to 9c8e5cb226e4 (1 revision) (flutter/flutter#155857) 2024-09-27 [email protected] Roll Flutter Engine from f21f2b232b8a to f9e4ed28f103 (2 revisions) (flutter/flutter#155855) 2024-09-27 [email protected] Add magnificationScale to CupertinoMagnifier for Zoom Effect (flutter/flutter#155276) 2024-09-27 [email protected] Fix typo on theme_data (flutter/flutter#155644) 2024-09-27 [email protected] Roll Flutter Engine from 7c603de2dca7 to f21f2b232b8a (6 revisions) (flutter/flutter#155843) 2024-09-27 [email protected] Turn the packages roller bot back on (flutter/flutter#155842) 2024-09-27 [email protected] Roll Packages from f38b780 to 0321757 (4 revisions) (flutter/flutter#155832) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. ...

… consider max line height (flutter/flutter#155892)

github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Sep 29, 2024

Renzo-Olivares mentioned this pull request Sep 29, 2024

Text is not selected when selection starts within line height but above or below the text #133637

Closed

2 tasks

Renzo-Olivares marked this pull request as ready for review September 29, 2024 01:31

Renzo-Olivares force-pushed the sa-bounding-box-fix branch from a5fbb5b to e2e7934 Compare September 30, 2024 00:08

Renzo-Olivares commented Sep 30, 2024

View reviewed changes

Renzo Olivares added 6 commits September 30, 2024 09:56

boundingBoxes should take up maxHeight and should be invalidated on l…

6d1e815

…ayout change

Debug

a9a6634

Revert "Debug"

685798a

This reverts commit 810fd9b.

textOffsetToPosition should consider preferredLineHeight

86706a4

update

41606b9

add test for cached bounding boxes

05962ae

Renzo-Olivares force-pushed the sa-bounding-box-fix branch from 32de229 to 05962ae Compare September 30, 2024 16:56

Renzo-Olivares requested review from chunhtai and justinmc September 30, 2024 17:14

justinmc approved these changes Sep 30, 2024

View reviewed changes

address reviewer comments

4fae75b

chunhtai approved these changes Sep 30, 2024

View reviewed changes

bleroux approved these changes Sep 30, 2024

View reviewed changes

make preferredLineHeight visibleForTesting only

a89e9f2

Renzo-Olivares added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Sep 30, 2024

auto-submit bot merged commit 350b475 into flutter:master Sep 30, 2024

engine-flutter-autoroll mentioned this pull request Oct 1, 2024

Roll Flutter from fa402c8057a1 to b05246d305ac (50 revisions) flutter/packages#7746

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 1, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

901b9aa

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll mentioned this pull request Oct 1, 2024

Roll Flutter from fa402c8057a1 to 6bba08cbcc51 (51 revisions) flutter/packages#7748

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 1, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

d3cbde2

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll mentioned this pull request Oct 1, 2024

Roll Flutter from fa402c8057a1 to 0975e612c04a (52 revisions) flutter/packages#7749

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 1, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

a8c8566

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 1, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

4a7d19d

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll mentioned this pull request Oct 1, 2024

Roll Flutter from fa402c8057a1 to f7fb0f1cd4f3 (58 revisions) flutter/packages#7757

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 1, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

d555cf8

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll mentioned this pull request Oct 2, 2024

Roll Flutter from fa402c8057a1 to 4342887826c2 (82 revisions) flutter/packages#7763

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 2, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

d9dc761

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll mentioned this pull request Oct 2, 2024

Roll Flutter from fa402c8057a1 to e819a3829240 (83 revisions) flutter/packages#7764

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 2, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

c9d2483

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 2, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

84f807a

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 2, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

4077f2c

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 2, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

ea1e1c3

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 2, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

16a1b79

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 3, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

d569d41

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 3, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

c347bf8

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 3, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

17a7353

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 3, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

403b36d

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 3, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

9e220c3

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 3, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

a479d75

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 4, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

61a7c11

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 4, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

07bba94

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 4, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

5572d71

… consider max line height (flutter/flutter#155892)

Piinks mentioned this pull request Oct 4, 2024

Flutter Framework Team: Notable Commits #121415

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 5, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

31517a9

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 5, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

9f464e0

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 6, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

6d07902

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 6, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

b5ab520

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll mentioned this pull request Oct 7, 2024

Manual roll Flutter from ead6b0d17c89 to 6bba08cbcc51 (37 revisions) flutter/packages#7809

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 7, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

fae6e92

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

6f6ecc1

… consider max line height (flutter/flutter#155892)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024

350b475 RenderParagraphs _SelectableFragment.boundingBoxes should…

2839fa9

… consider max line height (flutter/flutter#155892)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`RenderParagraph`s `_SelectableFragment.boundingBoxes` should consider max line height#155892

`RenderParagraph`s `_SelectableFragment.boundingBoxes` should consider max line height#155892
auto-submit[bot] merged 8 commits intoflutter:masterfrom
Renzo-Olivares:sa-bounding-box-fix

Renzo-Olivares commented Sep 29, 2024 •

edited

Loading

Uh oh!

Renzo-Olivares Sep 30, 2024

Uh oh!

justinmc Sep 30, 2024

Uh oh!

justinmc left a comment

Uh oh!

justinmc Sep 30, 2024

Uh oh!

justinmc Sep 30, 2024

Uh oh!

chunhtai left a comment

Uh oh!

chunhtai Sep 30, 2024

Uh oh!

Renzo-Olivares Sep 30, 2024

Uh oh!

chunhtai Sep 30, 2024 •

edited

Loading

Uh oh!

Renzo-Olivares Sep 30, 2024

Uh oh!

bleroux left a comment •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		/// An estimate of the height of a line in the text. See [TextPainter.preferredLineHeight].
		/// This does not require the layout to be updated.

Conversation

Renzo-Olivares commented Sep 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Web

Flutter SelectableText

Flutter Text widget under SelectionArea

Pre-launch Checklist

Uh oh!

Renzo-Olivares Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

justinmc Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

justinmc Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

chunhtai Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

chunhtai Sep 30, 2024 • 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!

Renzo-Olivares Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

bleroux left a comment • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Renzo-Olivares commented Sep 29, 2024 •

edited

Loading

chunhtai Sep 30, 2024 •

edited

Loading

bleroux left a comment •

edited

Loading