[iOS] Fixed ImageButton not honoring Padding value. by NirmalKumarYuvaraj · Pull Request #26785 · dotnet/maui

NirmalKumarYuvaraj · 2024-12-23T14:55:39Z

Issue Details

The ImageButton size calculation is incorrect. When an ImageButton has both padding and an image, its measured size does not adapt to include the padding values, causing the image to render smaller than expected.

Root Cause

The ImageButton DesiredSize is determined solely based on the ImageSize during the SizeThatFitsImage calculation. As a result, the ImageButton renders according to the image size alone, without accounting for the padding values in its measurement.

Description of Change

Updated the ImageButton to implement ICrossPlatformLayout for handling measurements, ensuring that content measurement accounts for padding values as expected, thereby improving the layout behavior.

Tested the behaviour in the following platforms

Android
Windows
iOS
Mac

Issue Fixed

Fixes #22521

Output

Before	After

AmrAlSayed0 · 2025-01-06T08:19:22Z

src/Controls/src/Core/ImageButton/ImageButton.iOS.cs

+			CGSize imageSize = platformButton.CurrentImage?.Size ?? CGSize.Empty;
+
+			double contentWidth = imageSize.Width + contentEdgeInsets.Left + contentEdgeInsets.Right;
+			double contentHeight = imageSize.Height + contentEdgeInsets.Top + contentEdgeInsets.Bottom;


Thickness has a HorizontalThickness and VerticalThickness properties you could use.

jfversluis · 2025-01-08T13:12:21Z

/azp run

azure-pipelines · 2025-01-08T13:12:43Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen

Padding_Add failing

prakashKannanSf3972 · 2025-01-09T08:20:56Z

Padding_Add failing

@PureWeen , The failure was due to the macOS snapshot for Padding_Add not being updated earlier. The Snapshot has been updated, and the test is expected to pass in the next CI run.

PureWeen

Fix conflicts

prakashKannanSf3972 · 2025-01-10T09:15:15Z

Fix conflicts

@PureWeen, Conflicts have been resolved.

jsuarezruiz · 2025-01-14T11:14:20Z

/azp run

azure-pipelines · 2025-01-14T11:14:41Z

Azure Pipelines successfully started running 3 pipeline(s).

src/Controls/src/Core/ImageButton/ImageButton.iOS.cs

prakashKannanSf3972 · 2025-01-15T09:19:23Z

@MartyIX, @PureWeen,

Thank you for the review! The nullable directive code has been removed as suggested.

PureWeen · 2025-01-16T19:54:00Z

/azp run

PureWeen · 2025-02-10T22:49:38Z

/azp run

azure-pipelines · 2025-02-10T22:49:58Z

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz · 2025-02-12T11:01:59Z

src/Controls/tests/TestCases.Mac.Tests/snapshots/mac/ImageButtonUITests_Padding_Add_Added.png

Must update the macOS snapshots after: #27531

@jsuarezruiz,

Updated the macOS snapshots as per your suggestion in the latest commit. Let me know if you have any further concerns. Thanks!

prakashKannanSf3972 · 2025-02-13T09:37:16Z

The test Issue18242Test is failing on iOS. Small differences in the image related with the Padding: Could you review it?

The failing test case has been verified and resolved. It has been successfully validated locally, and expect it will pass in the next CI run.

PureWeen · 2025-02-13T13:00:28Z

/azp run

azure-pipelines · 2025-02-13T13:00:58Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen

is there a way we can encapsulate this a bit into this method?

maui/src/Core/src/Platform/iOS/ImageViewExtensions.cs

Line 71 in dc4af6c

    
           internal static CGSize SizeThatFitsImage(this UIImageView imageView, CGSize constraints)

It looks like they are basically the same except for the padding part?

Maybe we can create an internal version of this for now that takes padding?

Addressed

prakashKannanSf3972 · 2025-02-17T14:19:41Z

is there a way we can encapsulate this a bit into this method?

maui/src/Core/src/Platform/iOS/ImageViewExtensions.cs

Line 71 in dc4af6c

internal static CGSize SizeThatFitsImage(this UIImageView imageView, CGSize constraints)

It looks like they are basically the same except for the padding part?

Maybe we can create an internal version of this for now that takes padding?

@PureWeen,

Thanks for the suggestion! I have refactored the code to encapsulate the logic as recommended. The updated method now includes padding values for improved reusability while preserving the existing constraints. Please review and let me know if you have any further feedback!

PureWeen · 2025-02-18T16:45:57Z

/azp run

azure-pipelines · 2025-02-18T16:46:19Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen

Looks like some new images will need to be added to the test

jsuarezruiz · 2025-02-21T12:08:24Z

/azp run

azure-pipelines · 2025-02-21T12:08:45Z

Azure Pipelines successfully started running 3 pipeline(s).

prakashKannanSf3972 · 2025-02-25T07:32:38Z

Looks like some new images will need to be added to the test

New images have been added.

* Fixed-ImageButton-Padding-Issue-iOS * Modified-code * Optimized-code * Added-Mac-Snapshot * fixed-conflicts * Removed-nullable-directive * Resolved-Conflicts * Modified-Code * Updated-Snapshots * Modified-Code * Modified-Snapshot --------- Co-authored-by: prakashKannanSf3972 <127308739+prakashKannanSf3972@users.noreply.github.com> Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>

Fixed-ImageButton-Padding-Issue-iOS

1b68b7d

dotnet-policy-service bot added the community ✨ Community Contribution label Dec 23, 2024

sheiksyedm added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Dec 23, 2024

PureWeen added the area-controls-button Button, ImageButton label Dec 26, 2024

prakashKannanSf3972 added 2 commits January 2, 2025 10:48

Merge branch 'main' into fix-22521

cef70b2

Modified-code

0721d22

AmrAlSayed0 reviewed Jan 6, 2025

View reviewed changes

Optimized-code

0dce52d

NirmalKumarYuvaraj marked this pull request as ready for review January 8, 2025 05:05

NirmalKumarYuvaraj requested a review from a team as a code owner January 8, 2025 05:05

NirmalKumarYuvaraj requested review from PureWeen and jsuarezruiz January 8, 2025 05:05

PureWeen requested changes Jan 8, 2025

View reviewed changes

Added-Mac-Snapshot

2c2c042

PureWeen requested changes Jan 9, 2025

View reviewed changes

prakashKannanSf3972 added 3 commits January 10, 2025 10:36

Merge branch 'main' into fix-22521

1ca72b6

Merge remote-tracking branch 'upstream/main' into fix-22521

63eb652

fixed-conflicts

d1ea0f9

MartyIX reviewed Jan 14, 2025

View reviewed changes

src/Controls/src/Core/ImageButton/ImageButton.iOS.cs Outdated Show resolved Hide resolved

PureWeen added this to the .NET 9 SR4 milestone Jan 14, 2025

PureWeen requested changes Jan 14, 2025

View reviewed changes

src/Controls/src/Core/ImageButton/ImageButton.iOS.cs Outdated Show resolved Hide resolved

Removed-nullable-directive

586ca3e

prakashKannanSf3972 added 2 commits February 10, 2025 16:39

Resolved-Conflicts

fef2136

Modified-Code

1fd2219

jsuarezruiz previously requested changes Feb 12, 2025

View reviewed changes

prakashKannanSf3972 added 2 commits February 13, 2025 11:21

Merge branch 'dotnet:main' into fix-22521

2463386

Updated-Snapshots

ff6d520

PureWeen requested changes Feb 13, 2025

View reviewed changes

Modified-Code

1f7942c

PureWeen approved these changes Feb 18, 2025

View reviewed changes

PureWeen requested changes Feb 19, 2025

View reviewed changes

Modified-Snapshot

49fec87

PureWeen approved these changes Mar 2, 2025

View reviewed changes

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

PureWeen merged commit 24db165 into dotnet:main Mar 3, 2025
123 checks passed

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

karthikraja-arumugam mentioned this pull request Mar 5, 2025

[regression/8.0.3] [iOS] Image layout issue in MAUI 8 (regression, worked correct in MAUI 7) #18983

Closed

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

Conversation

NirmalKumarYuvaraj commented Dec 23, 2024

Issue Details

Root Cause

Description of Change

Issue Fixed

Output

Uh oh!

AmrAlSayed0 Jan 6, 2025

Choose a reason for hiding this comment

Uh oh!

jfversluis commented Jan 8, 2025

Uh oh!

azure-pipelines bot commented Jan 8, 2025

Uh oh!

PureWeen left a comment

Choose a reason for hiding this comment

Uh oh!

prakashKannanSf3972 commented Jan 9, 2025

Uh oh!

PureWeen left a comment

Choose a reason for hiding this comment

Uh oh!

prakashKannanSf3972 commented Jan 10, 2025

Uh oh!

jsuarezruiz commented Jan 14, 2025

Uh oh!

azure-pipelines bot commented Jan 14, 2025

Uh oh!

Uh oh!

Uh oh!

prakashKannanSf3972 commented Jan 15, 2025

Uh oh!

PureWeen commented Jan 16, 2025 • edited by jfversluis Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PureWeen commented Feb 10, 2025

Uh oh!

azure-pipelines bot commented Feb 10, 2025

Uh oh!

jsuarezruiz Feb 12, 2025

Choose a reason for hiding this comment

Uh oh!

prakashKannanSf3972 Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

prakashKannanSf3972 commented Feb 13, 2025

Uh oh!

PureWeen commented Feb 13, 2025

Uh oh!

azure-pipelines bot commented Feb 13, 2025

Uh oh!

PureWeen left a comment

Choose a reason for hiding this comment

Uh oh!

prakashKannanSf3972 commented Feb 17, 2025

Uh oh!

PureWeen commented Feb 18, 2025

Uh oh!

azure-pipelines bot commented Feb 18, 2025

Uh oh!

PureWeen left a comment

Choose a reason for hiding this comment

Uh oh!

jsuarezruiz commented Feb 21, 2025

Uh oh!

azure-pipelines bot commented Feb 21, 2025

Uh oh!

prakashKannanSf3972 commented Feb 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

PureWeen commented Jan 16, 2025 •

edited by jfversluis

Loading