[Android] Correctly scale Button image #19834

jsuarezruiz · 2024-01-11T14:20:09Z

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

src/Core/src/Platform/Windows/ButtonExtensions.cs

src/Core/src/Handlers/Button/ButtonHandler.Android.cs

src/Core/src/Handlers/Button/ButtonHandler.iOS.cs

src/Core/src/Handlers/Button/ButtonHandler.Android.cs

src/Controls/tests/UITests/Tests/Issues/Issue18242.cs

src/Controls/samples/Controls.Sample.UITests/Issues/Issue18242.xaml

src/Core/src/Handlers/Button/ButtonHandler.iOS.cs

tj-devel709 · 2024-02-15T16:52:06Z

It's looking pretty good to me on iOS and android!

mattleibow · 2024-02-16T14:59:45Z

/rebase

mattleibow · 2024-02-16T15:15:58Z

/rebase

src/Core/src/Platform/Windows/ButtonExtensions.cs

src/Core/src/Handlers/Button/ButtonHandler.iOS.cs

tj-devel709 · 2024-03-01T20:33:55Z

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>

jsuarezruiz · 2024-03-04T12:55:36Z

We already have the changes to fix Windows merged on main branch and the iOS fix will be #20953

tj-devel709

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

mattleibow · 2024-03-14T20:19:02Z

src/Controls/src/Core/Platform/Android/Extensions/ButtonExtensions.cs

 				// Which is why we don't have to worry about calling setCompoundDrawablePadding
 				// ourselves for our custom implemented IconGravityBottom
 				materialButton.IconPadding = (int)context.ToPixels(contentLayout.Spacing);

-				// 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)


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.

mattleibow · 2024-03-14T20:23:38Z

src/Core/src/Handlers/Button/ButtonHandler.Android.cs

+			// The TextView might need an additional measurement pass at the final size
 			this.PrepareForTextViewArrange(frame);


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.

mattleibow · 2024-03-14T20:29:13Z

src/Core/src/Handlers/Button/ButtonHandler.Android.cs

+				button.Icon = platformImage is null
+					? null
+					: new MauiMaterialButton.MauiResizableDrawable(platformImage);


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.

mattleibow · 2024-03-14T20:31:03Z

src/Core/src/Platform/Android/MauiMaterialButton.cs

 		{
 		}

-		protected override void OnLayout(bool changed, int left, int top, int right, int bottom)
+		public override int IconGravity


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.

mattleibow · 2024-03-14T20:32:52Z

src/Core/src/Platform/Android/MauiMaterialButton.cs

+				// 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;
+				}


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.

mattleibow · 2024-03-14T20:33:19Z

src/Core/src/Platform/Android/MauiMaterialButton.cs

+			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);
+				}
+			}


Here we flip at the end.

Matt took over as the owner of this PR

romerotg · 2024-03-18T15:13:26Z

@PureWeen is there a way to know on which version this fix is gonna be released?

jsuarezruiz added platform/windows 🪟 platform/android 🤖 legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor area-controls-button Button, ImageButton labels Jan 11, 2024

jsuarezruiz commented Jan 11, 2024

View reviewed changes

src/Core/src/Platform/Windows/ButtonExtensions.cs Outdated Show resolved Hide resolved

jsuarezruiz commented Jan 11, 2024

View reviewed changes

src/Core/src/Handlers/Button/ButtonHandler.Android.cs Outdated Show resolved Hide resolved

jsuarezruiz commented Jan 12, 2024

View reviewed changes

src/Core/src/Handlers/Button/ButtonHandler.iOS.cs Outdated Show resolved Hide resolved

jsuarezruiz changed the title ~~[Android, Windows] Correctly scale Button image~~ [Android, iOS, Windows] Correctly scale Button image Jan 16, 2024

jsuarezruiz marked this pull request as ready for review January 18, 2024 07:32

jsuarezruiz requested a review from a team as a code owner January 18, 2024 07:32

jsuarezruiz requested review from mattleibow, tj-devel709 and jfversluis and removed request for jfversluis January 18, 2024 07:32

mattleibow reviewed Jan 19, 2024

View reviewed changes

github-actions bot force-pushed the fix-18242 branch from 53130de to a33e802 Compare February 16, 2024 15:00

github-actions bot force-pushed the fix-18242 branch from a33e802 to 774af30 Compare February 16, 2024 15:16

mattleibow requested changes Feb 16, 2024

View reviewed changes

src/Core/src/Platform/Windows/ButtonExtensions.cs Outdated Show resolved Hide resolved

mattleibow reviewed Feb 21, 2024

View reviewed changes

src/Core/src/Handlers/Button/ButtonHandler.iOS.cs Outdated Show resolved Hide resolved

PureWeen requested a review from mattleibow February 29, 2024 23:06

Foda mentioned this pull request Mar 1, 2024

[Windows] Fix image button scaling #20949

Merged

tj-devel709 mentioned this pull request Mar 1, 2024

[iOS] Use updated APIs for the button with images #20953

Closed

jsuarezruiz changed the title ~~[Android, iOS, Windows] Correctly scale Button image~~ [Android] Correctly scale Button image Mar 4, 2024

tj-devel709 previously approved these changes Mar 4, 2024

View reviewed changes

PureWeen dismissed tj-devel709’s stale review via c1e38fe March 4, 2024 16:07

jsuarezruiz and others added 11 commits March 14, 2024 16:48

Added iOS snapshot

2629234

Changed logic to resize on iOS (now rescale)

cd695f7

Updated snapshots

497d887

Moved changes in Button iOS from Core to Controls

efafcc6

Revert Windows related changes

a07258c

Revert iOS snapshot

dc308ec

Revert more changes

64bf6a6

Resize the image to fit the button

c00258b

Use the correct measure spec mode

632235d

nice things

a7834a2

this

24d1697

mattleibow force-pushed the fix-18242 branch from da835c4 to 24d1697 Compare March 14, 2024 14:49

mattleibow changed the base branch from release/8.0.1xx-sr3 to main March 14, 2024 14:50

mattleibow added 3 commits March 14, 2024 21:50

More things

c7ace2e

add a comment

52b7ece

rename

269f0f5

mattleibow reviewed Mar 14, 2024

View reviewed changes

Update Issue18242.cs

1a679ec

mattleibow self-assigned this Mar 15, 2024

Add files via upload

f87b33e

PureWeen approved these changes Mar 15, 2024

View reviewed changes

PureWeen merged commit 1b423ab into main Mar 16, 2024
47 checks passed

PureWeen deleted the fix-18242 branch March 16, 2024 22:50

PureWeen mentioned this pull request Mar 18, 2024

[Android] Fix Button IconSize #16164

Closed

github-actions bot locked and limited conversation to collaborators Apr 18, 2024

Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024

samhouts added fixed-in-8.0.20 fixed-in-9.0.0-preview.3.10457 labels Aug 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Android] Correctly scale Button image #19834

[Android] Correctly scale Button image #19834

jsuarezruiz commented Jan 11, 2024 •

edited

Loading

tj-devel709 commented Feb 15, 2024

mattleibow commented Feb 16, 2024

mattleibow commented Feb 16, 2024

tj-devel709 commented Mar 1, 2024

jsuarezruiz commented Mar 4, 2024

tj-devel709 left a comment

mattleibow Mar 14, 2024

mattleibow Mar 14, 2024

mattleibow Mar 14, 2024

mattleibow Mar 14, 2024

mattleibow Mar 14, 2024

mattleibow Mar 14, 2024

romerotg commented Mar 18, 2024

		// The TextView might need an additional measurement pass at the final size
		this.PrepareForTextViewArrange(frame);

[Android] Correctly scale Button image #19834

[Android] Correctly scale Button image #19834

Conversation

jsuarezruiz commented Jan 11, 2024 • edited Loading

Description of Change

Issues Fixed

tj-devel709 commented Feb 15, 2024

mattleibow commented Feb 16, 2024

mattleibow commented Feb 16, 2024

tj-devel709 commented Mar 1, 2024

jsuarezruiz commented Mar 4, 2024

tj-devel709 left a comment

Choose a reason for hiding this comment

mattleibow Mar 14, 2024

Choose a reason for hiding this comment

mattleibow Mar 14, 2024

Choose a reason for hiding this comment

mattleibow Mar 14, 2024

Choose a reason for hiding this comment

mattleibow Mar 14, 2024

Choose a reason for hiding this comment

mattleibow Mar 14, 2024

Choose a reason for hiding this comment

mattleibow Mar 14, 2024

Choose a reason for hiding this comment

romerotg commented Mar 18, 2024

jsuarezruiz commented Jan 11, 2024 •

edited

Loading