Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iOS] Fix Button image sizing and layout #22476

Merged
merged 73 commits into from
Aug 27, 2024

Conversation

tj-devel709
Copy link
Contributor

@tj-devel709 tj-devel709 commented May 17, 2024

Description of Change

This PR is a continuation of this previous PR: #21759.
This PR will now do some of the heavy work to try to resize the image, improve centering especially in situations where the image is above or below the title, respect the padding, spacing, and borderwidth to give more control to the developer, match Windows and Android where it can, and overall give a better button experience on iOS.

Note: With the introduction of the UIButton.Configuration API in the future, button layout will be a little simpler.

Notable Changes in iOS Button

Spacing

iOS will now respect the spacing, padding, borderwidth, and margins more accurately than before. As a result, the default spacing between the image and title will actually show up as 10 units. Previously, the button was trying to set 10 units but actually rendered around 6. The image below shows the old behavior at the top and the new behavior on the bottom. If you want the old behavior, you can manually add ContentLayout="Left, 6"or whatever width you'd like.
image

Image Resizing

In the old implementation, images would not be resized if they were too large and the padding, borderwidth, spacing, and margins would changes internally to try to fit the button contents if it could. Now the image will be resized to the max size given the padding, borderwidth, spacing, and margins. If there is also a title, the button content may not all fit inside the button and we would recommend that you size your image manually to achieve the layout you desire.

Example 1

        <VerticalStackLayout Background="LightBlue">
                <Button Background="LightGray" HeightRequest="100" WidthRequest="100" ImageSource="dotnet_bot.png" Text="Button 1" />
        </VerticalStackLayout>
image

Example 2

        <VerticalStackLayout Background="LightBlue">
                <Button Background="LightGray" ContentLayout="Top, 5" HeightRequest="100" WidthRequest="100" ImageSource="dotnet_bot.png" Text="Button 1" />
        </VerticalStackLayout>
image

Example 3

        <VerticalStackLayout Background="LightBlue">
                <Button Background="LightGray" ContentLayout="Top, 5" HeightRequest="100" WidthRequest="100" ImageSource="dotnet_bot.png" Text="Button 1" />
        </VerticalStackLayout>
    <MauiImage Update="Resources\Images\dotnet_bot.svg" Color="#FFFFFF" BaseSize="50,50" />
image

BorderWidth

BorderWidth now matches Windows and should be similar to Android in the future. Before, the button content would be inset by the BorderWidth every time. Now the BorderWidth will try to use the space outside the button if it is available and if it is not, it will use the space inside the button.

In this example below, the top button does not have space outside the button so the BorderWidth will inset the space and the second button has more space available so it will outset the button.

image image

If you'd like to test some cases, here are some code samples:

Issues Fixed

Fixes #22306
Fixes #21285
Fixes #23635

@tj-devel709 tj-devel709 added platform/iOS 🍎 partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with area-controls-button Button, ImageButton labels May 17, 2024
@tj-devel709 tj-devel709 requested a review from a team as a code owner May 17, 2024 04:11
@tj-devel709 tj-devel709 marked this pull request as draft May 17, 2024 16:46
@tj-devel709
Copy link
Contributor Author

I think this needs to rethink how the Measure for the button is taken in the case of moving all the layout logic into the ArrangeOverride

@tj-devel709 tj-devel709 force-pushed the dev/TJ/iOS18242-Compat3-FixRotation branch from fb92003 to a6d0772 Compare May 22, 2024 14:54
@tj-devel709 tj-devel709 marked this pull request as ready for review May 22, 2024 18:17
@tj-devel709 tj-devel709 force-pushed the dev/TJ/iOS18242-Compat3-FixRotation branch from 44f5aa8 to 3147b11 Compare May 23, 2024 14:44
@tj-devel709 tj-devel709 marked this pull request as draft May 28, 2024 16:17
Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some UI Tests related with image sizing failing on iOS.
image

@tj-devel709 tj-devel709 force-pushed the dev/TJ/iOS18242-Compat3-FixRotation branch from b4a6699 to 512c841 Compare June 20, 2024 21:43
@tj-devel709
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@tj-devel709 tj-devel709 force-pushed the dev/TJ/iOS18242-Compat3-FixRotation branch from 696765a to 99ed008 Compare June 28, 2024 13:13
@tj-devel709
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@tj-devel709 tj-devel709 force-pushed the dev/TJ/iOS18242-Compat3-FixRotation branch from be6e2a1 to bbb8045 Compare July 2, 2024 17:00
@tj-devel709 tj-devel709 marked this pull request as ready for review July 2, 2024 21:24
@PureWeen
Copy link
Member

PureWeen commented Jul 3, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigate not setting ContentEdgeInsets for the padding to fix issues with clipping

@tj-devel709 tj-devel709 force-pushed the dev/TJ/iOS18242-Compat3-FixRotation branch from 1b0e5af to 61886b8 Compare August 26, 2024 19:56
@tj-devel709 tj-devel709 merged commit 588eece into main Aug 27, 2024
97 checks passed
@tj-devel709 tj-devel709 deleted the dev/TJ/iOS18242-Compat3-FixRotation branch August 27, 2024 19:54
@samhouts samhouts added fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-button Button, ImageButton fixed-in-net8.0-nightly This may be available in a nightly release! fixed-in-net9.0-nightly This may be available in a nightly release! partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with platform/iOS 🍎
Projects
None yet
4 participants