-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Baseline Aligned Row #30746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Baseline Aligned Row #30746
Conversation
HansMuller
left a comment
There was a problem hiding this 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(); |
There was a problem hiding this comment.
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)); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
@HansMuller Ready for re-review. |
HansMuller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* 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) ...
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

counteron this text field overflows the red Row container. With this PR, it will fit perfectly inside.