RenderParagraphs _SelectableFragment.boundingBoxes should consider max line height
#155892
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
framework
Closed
2 tasks
Renzo-Olivares
force-pushed
the
sa-bounding-box-fix
branch
from
a5fbb5b to
e2e7934
Compare
Renzo-Olivares
commented
Sep 30, 2024
| 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); |
Contributor
Author
There was a problem hiding this comment.
I borrowed this from
, I couldn't really track down why this change was needed but it originally came from #4397
Contributor
There was a problem hiding this comment.
Interesting, well I guess we should keep it around 🤷 .
added 6 commits
September 30, 2024 09:56
Renzo-Olivares
force-pushed
the
sa-bounding-box-fix
branch
from
32de229 to
05962ae
Compare
justinmc
approved these changes
Sep 30, 2024
| 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); |
Contributor
There was a problem hiding this comment.
Interesting, well I guess we should keep it around 🤷 .
Comment on lines
704
to
705
| /// An estimate of the height of a line in the text. See [TextPainter.preferredLineHeight]. | ||
| /// This does not require the layout to be updated. |
Contributor
There was a problem hiding this comment.
Nit: Empty line of /// between these two lines, and the first line should be only 1 sentence.
chunhtai
approved these changes
Sep 30, 2024
|
|
||
| /// 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; |
Contributor
Author
There was a problem hiding this comment.
RenderEditable has this API public, I don't have a strong preference either way, what do you think?
Contributor
There was a problem hiding this comment.
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
Contributor
Author
There was a problem hiding this comment.
Sounds good to me!
bleroux
approved these changes
Sep 30, 2024
Renzo-Olivares
added
autosubmit
thejitenpatel
pushed a commit
to thejitenpatel/flutter
that referenced
this pull request
Oct 1, 2024
…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.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 1, 2024
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… 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
… consider max line height (flutter/flutter#155892)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 7, 2024
… consider max line height (flutter/flutter#155892)
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Oct 8, 2024
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. ...
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Dec 11, 2024
… 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
… consider max line height (flutter/flutter#155892)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #133637
This change updates the
_SelectableFragment.boundingBoxeslogic 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 thanSelectableTextwhich 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:
textOffsetToPositionto considerpreferredLineHeight. In cases when the text wraps, it was sometimes inaccurate.Pre-launch Checklist
///).