Skip to content

Baseline Aligned Row - #30746

Merged
justinmc merged 9 commits into
flutter:masterfrom
justinmc:row-text-field
Apr 11, 2019
Merged

justinmc merged 9 commits into
flutter:masterfrom
justinmc:row-text-field

Conversation

@justinmc

@justinmc justinmc commented Apr 8, 2019 •

Copy link
Copy Markdown
Contributor

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 p: material_ui material_ui package in flutter/packages framework flutter/packages/flutter repository. See also f: labels. labels Apr 9, 2019

@HansMuller HansMuller left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

@HansMuller Ready for re-review.

@HansMuller HansMuller left a comment

Copy link
Copy Markdown
Contributor

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

framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash when aligning TextField by baseline

5 participants