Conversation
src/Controls/samples/Controls.Sample.UITests/Issues/Issue18242.xaml
Outdated
Show resolved
Hide resolved
|
It's looking pretty good to me on iOS and android! |
|
/rebase |
53130de to
a33e802
Compare
|
/rebase |
a33e802 to
774af30
Compare
|
It appears that there is an issue on iOS with an ImageButton inside a scrollview. The image does not resize with the width request as android and Windows does. Check out the bottom imagebutton in each of the screenshots: iOS: Android: Windows: This is the relevant xaml: <ScrollView HeightRequest="100" WidthRequest="50">
<ImageButton Source="dotnet_bot.png" />
</ScrollView> |
|
We already have the changes to fix Windows merged on main branch and the iOS fix will be #20953 |
tj-devel709
left a comment
There was a problem hiding this comment.
Not sure why the windows changes still appear in this PR as main was merged into here and those changes are present in https://github.com/dotnet/maui/pull/20949/files. But otherwise, LGTM
| // For IconGravityTextEnd and IconGravityTextStart, setting the Icon twice | ||
| // is needed to work around the Android behavior that caused | ||
| // https://github.com/dotnet/maui/issues/11755 | ||
| switch (contentLayout.Position) |
There was a problem hiding this comment.
Instead of toggling the icon here, we are doing that as part of the measure. This makes the updates happen all together instead of here, and then it is better for maintenance.
| // The TextView might need an additional measurement pass at the final size | ||
| this.PrepareForTextViewArrange(frame); |
There was a problem hiding this comment.
This is needed because TextView controls may be measured with an "AtMost" which means the measure is more "give me the smallest size". Because the text in a TextView is positioned using an absolute coordinate, it needs to be re-measured with the final sizes to get the new coordinates.
| button.Icon = platformImage is null | ||
| ? null | ||
| : new MauiMaterialButton.MauiResizableDrawable(platformImage); |
There was a problem hiding this comment.
When we set the icon, we wrap it in a special "resizable drawable" that allows us to give the icon a specific size that is used by the measure and layout passes.
| } | ||
|
|
||
| protected override void OnLayout(bool changed, int left, int top, int right, int bottom) | ||
| public override int IconGravity |
There was a problem hiding this comment.
If we set an invalid/new value here, then the measure and layout passes will not work properly. Luckily, we can pretend that bottom is top, and then swap to the bottom at the last minute after the layout has completed.
| // if there is BOTH an icon AND text, but the text layout has NOT been measured yet, | ||
| // we need to measure JUST the text first to get the remaining space for the icon | ||
| if (Layout is null && TextFormatted?.Length() > 0) | ||
| { | ||
| // remove the icon and measure JUST the text | ||
| Icon = null; | ||
| base.OnMeasure(widthMeasureSpec, heightMeasureSpec); | ||
|
|
||
| // restore the icon | ||
| Icon = currentIcon; | ||
| } |
There was a problem hiding this comment.
Due to the way TextView measures, it appears to prioritize the icon and will clip/squash the text. However, we want the text to be the priority. In order for this to happen, we have to measure the initial pass without an icon and this sets us up to re-measure with a smaller/fitting icon.
| if (ForceBottomIconGravity) | ||
| { | ||
| var icons = TextViewCompat.GetCompoundDrawablesRelative(this); | ||
| if (icons[1] is { } icon) | ||
| { | ||
| TextViewCompat.SetCompoundDrawablesRelative(this, null, null, null, icon); | ||
| icon.SetBounds(0, 0, icon.IntrinsicWidth, icon.IntrinsicHeight); | ||
| } | ||
| } |
Matt took over as the owner of this PR
|
@PureWeen is there a way to know on which version this fix is gonna be released? |
Description of Change
Correctly scale the Button image on Android and Windows.
NOTE: We require the server's snapshots for the golden test. The PR is a Draft until the snapshots are generated.
Issues Fixed
Fixes #9734
Fixes #18242