[Android] Fix ImageButton Padding the third by aheubusch · Pull Request #25223 · dotnet/maui

aheubusch · 2024-10-11T22:46:30Z

Description of Change

Improve the workaround (#22298) for the iffy Android ShapeImageView contentPadding/padding handling.
The underlying problem is that ShapeImageView updates Padding in onMeasure leading to a double padding. The previous workaround reset the Padding after one eventloop hoping that onMeasure was called in the meantime. This worked for most cases but not all. The "improved" workaround overrides onMeasure and resets the Padding as soon as it was set.

I ran all ImageButton tests I could find and they are all green. So thats a good sign.

Issues Fixed

Fixes #25201
Fixes #16713
Fixes #18001
Fixes #13101

aheubusch · 2024-10-11T22:52:09Z

src/Core/src/Platform/Android/ImageButtonExtensions.cs

-			platformButton.SetContentPadding((int)padding.Left, (int)padding.Top, (int)padding.Right, (int)padding.Bottom);
-			platformButton.SetPadding(0, 0, 0, 0);
-
-			// Just like before, the bugs are not done. This needs to trigger a re-calculation of


@mattleibow You have added this part (ShapeAppearanceModel) in #22298. I am not sure if I can just remove it. None of the tests that looked relevant failed on my machine.

I am scared tbh. I forget the corner cases where this happens - and they yield is especially scary. I think I was going off the fixes the native side was doing.

The current test set may not be enough and we will have to go remove this and see what breaks manually, and then make your change to see if it fixes it.

jsuarezruiz · 2024-10-14T06:25:04Z

/azp run

azure-pipelines · 2024-10-14T06:25:24Z

Azure Pipelines successfully started running 3 pipeline(s).

aheubusch · 2024-10-14T09:50:57Z

Not sure if the test failure are because of this PR. They all fail with a System.TimeoutException.

jfversluis · 2024-12-06T15:01:35Z

/rebase

Remove ShapeableImageView Padding immediately after it is set in its onMeasure method to avoid double padding (ContentPadding + Padding).

azure-pipelines · 2024-12-06T15:04:49Z

Azure Pipelines successfully started running 3 pipeline(s).

mattleibow

I am super scared with this as the ImageButton was super fragile and I think I have seen padding issues but was super hard to repro in a test or I was not able to reliably repro.

I added tests as I did things, and TJ probably added more. But, I am not sure there are enough.

This comment does not help move the PR along, but I need to play a bit more and try and repro the issues that required the terrible hacks.

mattleibow · 2024-12-13T14:26:17Z

src/Core/src/Platform/Android/ImageButtonExtensions.cs

-			platformButton.SetContentPadding((int)padding.Left, (int)padding.Top, (int)padding.Right, (int)padding.Bottom);
-			platformButton.SetPadding(0, 0, 0, 0);
-
-			// Just like before, the bugs are not done. This needs to trigger a re-calculation of


I am scared tbh. I forget the corner cases where this happens - and they yield is especially scary. I think I was going off the fixes the native side was doing.

The current test set may not be enough and we will have to go remove this and see what breaks manually, and then make your change to see if it fixes it.

BlueRaja · 2025-02-19T01:49:44Z

Any chance to get this in for the next release? Both Border and Padding on ImageButton are still broken, with no known workaround. This seems like pretty basic functionality to just straight up not work at all.

jsuarezruiz · 2025-03-10T14:45:39Z

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue25201.cs

@@ -0,0 +1,35 @@
+#if ANDROID


Could we run the test in all the platforms? In that way, we can just verify that the Padding property behavior is aligned.

karthikraja-arumugam · 2025-03-14T05:38:49Z

This PR also resolves the issue #18875

mattleibow

Now that I have had a nap, I don't think we should be writing C# anymore ;) OK, maybe we just need to put that new maui shape view into Java and use it there. The hopping back and forth will be a perf hit.

mattleibow · 2025-03-20T11:16:22Z

src/Core/src/Platform/Android/MauiShapeableImageView.cs

+
+namespace Microsoft.Maui.Platform
+{
+	internal class MauiShapeableImageView : ShapeableImageView


Just looking at this and I think that this is all Java code so we probably should just do it in Java and let the bonding do the magic.

Doing it in C# is probably going have a big perf impact.

New issue for this Java work: #28532

mattleibow · 2025-03-20T11:30:48Z

We can do Java in a new PR: #28532

mattleibow · 2025-03-20T11:31:38Z

src/Core/src/Platform/Android/MauiShapeableImageView.cs

+
+namespace Microsoft.Maui.Platform
+{
+	internal class MauiShapeableImageView : ShapeableImageView


New issue for this Java work: #28532

Remove ShapeableImageView Padding immediately after it is set in its onMeasure method to avoid double padding (ContentPadding + Padding).

aheubusch requested a review from a team as a code owner October 11, 2024 22:46

aheubusch requested review from jfversluis and jsuarezruiz October 11, 2024 22:46

dotnet-policy-service bot added the community ✨ Community Contribution label Oct 11, 2024

aheubusch commented Oct 11, 2024

View reviewed changes

jsuarezruiz added platform/android area-controls-button Button, ImageButton labels Oct 17, 2024

aheubusch closed this Oct 23, 2024

aheubusch deleted the android_imagebutton_padding branch October 23, 2024 07:48

aheubusch restored the android_imagebutton_padding branch October 23, 2024 08:34

aheubusch reopened this Oct 23, 2024

EmilienDup mentioned this pull request Dec 6, 2024

[Android] ImageButton configured with a FontImageSource do not respect size and padding #26431

Closed

[Android] Fix ImageButton Padding the third

8a3a074

Remove ShapeableImageView Padding immediately after it is set in its onMeasure method to avoid double padding (ContentPadding + Padding).

github-actions bot force-pushed the android_imagebutton_padding branch from 0a0de1e to 8a3a074 Compare December 6, 2024 15:02

mattleibow reviewed Dec 13, 2024

View reviewed changes

PureWeen added the partner/syncfusion/review label Feb 10, 2025

PureWeen added this to the .NET 9 SR6 milestone Mar 5, 2025

PureWeen added this to MAUI SDK Ongoing Mar 5, 2025

github-project-automation bot moved this to Todo in MAUI SDK Ongoing Mar 5, 2025

PureWeen assigned mattleibow Mar 5, 2025

jsuarezruiz reviewed Mar 10, 2025

View reviewed changes

rmarinho moved this from Todo to Ready To Review in MAUI SDK Ongoing Mar 20, 2025

rmarinho approved these changes Mar 20, 2025

View reviewed changes

github-project-automation bot moved this from Ready To Review to Approved in MAUI SDK Ongoing Mar 20, 2025

rmarinho moved this from Approved to Ready To Review in MAUI SDK Ongoing Mar 20, 2025

mattleibow requested changes Mar 20, 2025

View reviewed changes

github-project-automation bot moved this from Ready To Review to Changes Requested in MAUI SDK Ongoing Mar 20, 2025

mattleibow mentioned this pull request Mar 20, 2025

Just looking at this and I think that this is all Java code so we probably should just do it in Java and let the bonding do the magic. #28532

Open

mattleibow changed the base branch from main to inflight/current March 20, 2025 11:30

mattleibow approved these changes Mar 20, 2025

View reviewed changes

github-project-automation bot moved this from Changes Requested to Approved in MAUI SDK Ongoing Mar 20, 2025

mattleibow merged commit 320b488 into dotnet:inflight/current Mar 20, 2025
3 checks passed

github-project-automation bot moved this from Approved to Done in MAUI SDK Ongoing Mar 20, 2025

PureWeen pushed a commit that referenced this pull request Mar 26, 2025

[Android] Fix ImageButton Padding the third (#25223)

5c54f04

Remove ShapeableImageView Padding immediately after it is set in its onMeasure method to avoid double padding (ContentPadding + Padding).

PureWeen pushed a commit that referenced this pull request Mar 26, 2025

[Android] Fix ImageButton Padding the third (#25223)

b2570d8

Remove ShapeableImageView Padding immediately after it is set in its onMeasure method to avoid double padding (ContentPadding + Padding).

github-actions bot pushed a commit that referenced this pull request Mar 27, 2025

[Android] Fix ImageButton Padding the third (#25223)

7232f55

Remove ShapeableImageView Padding immediately after it is set in its onMeasure method to avoid double padding (ContentPadding + Padding).

github-actions bot locked and limited conversation to collaborators Apr 20, 2025

Conversation

aheubusch commented Oct 11, 2024

Description of Change

Issues Fixed

Uh oh!

aheubusch Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

mattleibow Dec 13, 2024

Choose a reason for hiding this comment

Uh oh!

jsuarezruiz commented Oct 14, 2024 • edited by jfversluis Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-pipelines bot commented Oct 14, 2024

Uh oh!

aheubusch commented Oct 14, 2024

Uh oh!

jfversluis commented Dec 6, 2024

Uh oh!

azure-pipelines bot commented Dec 6, 2024

Uh oh!

mattleibow left a comment

Choose a reason for hiding this comment

Uh oh!

mattleibow Dec 13, 2024

Choose a reason for hiding this comment

Uh oh!

BlueRaja commented Feb 19, 2025

Uh oh!

jsuarezruiz Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

karthikraja-arumugam commented Mar 14, 2025

Uh oh!

mattleibow left a comment

Choose a reason for hiding this comment

Uh oh!

mattleibow Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

mattleibow Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

mattleibow commented Mar 20, 2025

Uh oh!

mattleibow Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jsuarezruiz commented Oct 14, 2024 •

edited by jfversluis

Loading