[`RenderEditable`] report real height when `maxLines == 1`. #112029

Merged

auto-submit[bot] merged 17 commits intoflutter:masterfrom

LongCatIsLooong:render-editable-maxLine-1

Sep 28, 2022

Contributor

LongCatIsLooong commented Sep 21, 2022 •

edited

Loading

Another one extracted from #110966.

When maxLines == 1 RenderEditable can't be scrolled vertically to reveal clipped contents so it must report its real height (instead of the estimated height preferredLineHeight). I'll try to fix #112123 after this and removes the hack in a later pull request.

~~Running TGP.~~ TGP passed with no scuba changes.

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

LongCatIsLooong added 9 commits

September 5, 2022 22:48


          text field overflow warning

5ba85be


          InputDecorator scales down content padding

ec3585e


          Merge remote-tracking branch 'upstream/master' into render-editable-o…

f423619

…verflow


          input decorator

8bbbfaa


          comments

2457ede


          fix single-line text field height and input decorator height

d4cea71


          Merge remote-tracking branch 'upstream/master' into render-editable-o…

eea22e3

…verflow


          fix RenderEditable maxLine == 1

26b4135


          Merge remote-tracking branch 'upstream/master' into render-editable-m…

88bc495

…axLine-1

flutter-dashboard bot added a: text input f: material design framework labels

LongCatIsLooong force-pushed the render-editable-maxLine-1 branch from 11e22ae to 8725a6b Compare

September 21, 2022 02:01


          tests

cfab6de

LongCatIsLooong force-pushed the render-editable-maxLine-1 branch from 8725a6b to cfab6de Compare

September 21, 2022 03:40

LongCatIsLooong marked this pull request as ready for review

September 21, 2022 03:47

flutter-dashboard bot commented Sep 21, 2022

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #112029 at sha cfab6de

flutter-dashboard bot added the will affect goldens label

LongCatIsLooong marked this pull request as draft

September 21, 2022 06:20

flutter-dashboard bot commented Sep 21, 2022

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.


          apply hack

0d15d46

LongCatIsLooong force-pushed the render-editable-maxLine-1 branch from e54fad9 to 0d15d46 Compare

September 22, 2022 20:26


          Merge remote-tracking branch 'upstream/master' into render-editable-m…

57292e0

…axLine-1

LongCatIsLooong marked this pull request as ready for review

September 22, 2022 20:27

LongCatIsLooong requested review from goderbauer and justinmc

September 22, 2022 20:27

flutter-dashboard bot commented Sep 22, 2022

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #112029 at sha 57292e0


          fix golden

395aab1

flutter-dashboard bot commented Sep 23, 2022

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #112029 at sha 395aab1


          Merge remote-tracking branch 'upstream/master' into render-editable-m…

d35a366

…axLine-1

LongCatIsLooong force-pushed the render-editable-maxLine-1 branch from c33fd3b to a72723b Compare

September 26, 2022 19:54

flutter-dashboard bot commented Sep 26, 2022

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #112029 at sha a72723bfb607a2e9747327cf0a882515ce3a63d5


          golden

933b32c

LongCatIsLooong force-pushed the render-editable-maxLine-1 branch from a72723b to 933b32c Compare

September 26, 2022 20:40

flutter-dashboard bot commented Sep 26, 2022

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #112029 at sha 933b32c

justinmc approved these changes

View reviewed changes

Contributor

justinmc left a comment

LGTM 👍 but take a look at your TODO below that you might want to still handle in this PR.

Thanks for fixing this and for splitting it out of your other PR.

packages/flutter/lib/src/painting/text_painter.dart

                 List<ui.LineMetrics> computeLineMetrics() {
                   assert(_debugAssertTextLayoutIsValid);
-                  return  _lineMetricsCache ??= _paragraph!.computeLineMetrics();
+                  return _lineMetricsCache ??= _paragraph!.computeLineMetrics();

Contributor

justinmc Sep 28, 2022

👍

packages/flutter/lib/src/rendering/editable.dart Outdated

+                  int count = 0;
+                  for (int index = 0; index < text.length; index += 1) {
+                    // TODO(LongCatIsLooong): account for other line terminators, or get it
+                    // from ui.Paragraph.

Contributor

justinmc Sep 28, 2022

Did you mean to handle this in this PR?

Contributor Author

LongCatIsLooong Sep 28, 2022

Added other line breakers in. But still it would be better to let skparagraph handle that.

packages/flutter/lib/src/rendering/editable.dart Outdated

+                  for (int index = 0; index < text.length; index += 1) {
+                    // TODO(LongCatIsLooong): account for other line terminators, or get it
+                    // from ui.Paragraph.
+                    count += text.codeUnitAt(index) == 0x0A ? 1 : 0;

Contributor

justinmc Sep 28, 2022

Nit: Maybe make 0x0A a constant for clarity about what it is, depending on your TODO above.

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

-                    return child!.getMaxIntrinsicHeight(width);
-                  }
-                  return 0.0;
+                  return child?.getMaxIntrinsicHeight(width) ?? 0.0;

Contributor

justinmc Sep 28, 2022

👏

LongCatIsLooong force-pushed the render-editable-maxLine-1 branch from 0c2fae6 to 8faf248 Compare

September 28, 2022 20:36


          add other hard line breaks

b07ef1e

LongCatIsLooong force-pushed the render-editable-maxLine-1 branch from 8faf248 to b07ef1e Compare

September 28, 2022 20:43

LongCatIsLooong added the autosubmit label

auto-submit bot removed the autosubmit label

Contributor

auto-submit bot commented Sep 28, 2022

auto label is removed for flutter/flutter, pr: 112029, due to - The status or check suite ci.yaml validation has failed. Please fix the issues identified (or deflake) before re-applying this label.


          Merge remote-tracking branch 'upstream/master' into render-editable-m…

e73fcc5

…axLine-1

LongCatIsLooong added the autosubmit label

auto-submit bot merged commit 78e9415 into flutter:master

LongCatIsLooong deleted the render-editable-maxLine-1 branch

September 28, 2022 21:42

engine-flutter-autoroll mentioned this pull request

Roll Flutter from 9bf849184d6c to a11bef968893 (37 revisions) flutter/packages#2660

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request


          78e9415 [RenderEditable] report real height when maxLines == 1. (f…

e0c0cdc

…lutter/flutter#112029)

engine-flutter-autoroll mentioned this pull request

Roll Flutter from 9bf849184d6c to a11bef968893 (37 revisions) flutter/plugins#6515

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request


          78e9415 [RenderEditable] report real height when maxLines == 1. (f…

c4f2a08

…lutter/flutter#112029)

LongCatIsLooong mentioned this pull request

TextField maxLines change causes text containing newline chars overflow #88422

Closed

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

Labels

a: text input autosubmit f: material design framework will affect goldens