DatePicker header overflow fix#31327
DatePicker header overflow fix#31327filipwin wants to merge 1 commit intoflutter:masterfrom filipwin:29736-date-picker-header-overflow
Conversation
…ault content wrap height
goderbauer
added
f: material design
| } | ||
|
|
||
| const double _kDatePickerHeaderPortraitHeight = 100.0; | ||
| const double _kDatePickerHeaderLandscapeWidth = 168.0; |
There was a problem hiding this comment.
Why are we leaving this one?
There was a problem hiding this comment.
Neither in tickets nor in my tests I found any issues with displaying this picker in the landscape mode, so I didn't touch this value. However, the problem with overflowing won't appear in that case, as the header text is wrapped to second line if it doesn't have enough space. And that's why (I think so, but didn't check it yet) it will need concrete width to avoid stretching this header to fit whole text.
There was a problem hiding this comment.
I believe you'll see this problem if you set the text scale factor a bit higher, even in landscape.
There was a problem hiding this comment.
I'm ok with this PR if it really does fix things for portrait even if we leave landscape with a potential problem though. Perfect doesn't have to be the enemy of the good here :)
|
This needs tests. I'm also pretty surprised this didn't break any existing tests. /cc @darrenaustin who is working on layout stuff for Date Picker. /cc @willlarche who was getting us some guidance on this control as it's currently broken when text scale is off or when large fonts are used. |
|
Does this still look right with a smaller font (the case where it's not currently overflowing)? |
I checked it on a couple of devices with different ratios/resolutions/densities & languages and didn't encounter any noticeable difference. I guess it shouldn't break anything, as it will wrap around the actual content, so there might be a couple of pixels difference at most.
Well, I'm not very experienced with tests yet and I was convinced that changing widget height isn't something actually testable. But if it is, then I'll have a look at this. |
|
You should be able to run some tests that change the font scale and verify that it works. Something like this: https://github.com/flutter/flutter/pull/29734/files#diff-421a3407c743099e7c05ef094015f095 But try it with text scale factors larger than 1.5 as well. |
|
Thanks for your work on this. There were several other layout problems we needed to fix in addition to this one, so I put them all in a single PR #31514 which removes the hard coded constraints on the layouts. |
5 participants
Description
In this PR I removed constant, hardcoded DatePicker header height. Leaving Container with no specified height enables it to wrap its content and prevent clipping/overflowing.
How it looked:
How it looks now:
Related Issues
#29736
#28778
#29681