Skip to content

Conversation

@justinmc
Copy link
Contributor

@justinmc justinmc commented Apr 8, 2019

Closes #24705

There was a crash when putting a TextField in a row aligned by baseline. I cherry picked the fix for this part from @GaryQian's upcoming PR #30069.

Also, the height of the row was not being calculated correctly when aligned by baseline, but is fixed in this PR.

For example, the very tall counter on this text field overflows the red Row container. With this PR, it will fit perfectly inside.
Screen Shot 2019-04-08 at 10 58 49 AM

@justinmc justinmc changed the title WIP Baseline Aligned Row Baseline Aligned Row Apr 9, 2019
@goderbauer goderbauer added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Apr 9, 2019
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

This is a really nice fix, and long overdue! The analyzer wants some const expressions and I think the test could be tightened up just a little.


group('Row', () {
testWidgets('multiple baseline aligned children', (WidgetTester tester) async {
final GlobalKey key1 = GlobalKey();
Copy link
Contributor

Choose a reason for hiding this comment

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

GlobalKeys shouldn't be needed here, Key key1 = UniqueKey() should be enough.

// contain both of them.
expect(rowBox.size.height, greaterThan(textBox1.size.height));
expect(rowBox.size.height, greaterThan(textBox2.size.height));
expect(rowBox.size.height, lessThan(textBox1.size.height + textBox2.size.height));
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to predict the row's exact overall height and the top or bottom edge of the two text widgets.

https://web-platform-tests.org/writing-tests/ahem.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I was able to calculate the right height.

@justinmc
Copy link
Contributor Author

@HansMuller Ready for re-review.

Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

LGTM

@justinmc justinmc merged commit ecb468f into flutter:master Apr 11, 2019
@justinmc justinmc deleted the row-text-field branch April 11, 2019 16:32
jiisd added a commit to jiisd/flutter that referenced this pull request Apr 12, 2019
* master: (209 commits)
  Allow mouse hover to only respond to some mouse events but not all. (flutter#30886)
  Fix issue 23527: Exception: RenderViewport exceeded its maximum number of layout cycles (flutter#30809)
  Keep hover annotation layers in sync with the mouse detector. (flutter#30829)
  Use identical instead of '==' in a constant expression. (flutter#30921)
  Add toggle for debugProfileWidgetBuilds (flutter#30867)
  Revert "Manual engine roll with disabled service authentication codes (flutter#30919)" (flutter#30930)
  Manual engine roll with disabled service authentication codes (flutter#30919)
  Baseline Aligned Row (flutter#30746)
  [Material] Fix showDialog crasher caused by old contexts (flutter#30754)
  Let `sliver.dart` `_createErrorWidget` work with other Widgets (flutter#30880)
  Add more dialog doc cross-reference (flutter#30887)
  Allow downloading of desktop embedding artifacts (flutter#30648)
  CupertinoDatePicker initialDateTime accounts for minuteInterval  (flutter#30862)
  add golden tests for CupertinoDatePicker (flutter#30828)
  Simplify toImage future handling (flutter#30876)
  Fixed Table flex column layout error flutter#30437 (flutter#30470)
  Fix iTunes Transporter quirk (flutter#30883)
  Bump Android build tools to 28.0.3 in Dockerfile (flutter#30832)
  Update the upload key which seems to have trouble for some reason (flutter#30871)
  Check for invalid elevations (flutter#30215)
  ...
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash when aligning TextField by baseline

5 participants