ImageButton border (BorderWidth) overlaps the image instead of adding space - fix

[kubaflo]

Issues Fixed

Fixes https://github.com/dotnet/maui/issues/24856 Fixes https://github.com/dotnet/maui/issues/22520

Before	After

[jsuarezruiz]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 3 pipeline(s).

[jsuarezruiz]

/azp run MAUI-UITests-public

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[IrynaDoroshenkoDev]

Thank you Kuba! I used your fix in my handler and it works perfectly!

[jsuarezruiz]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 3 pipeline(s).

[jsuarezruiz]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 3 pipeline(s).

[jsuarezruiz]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 3 pipeline(s).

[mattleibow]

@kubaflo I like these tests but I had to re-work the whole background/border/padding parts of the ImageButton because we lost the ripple at some point: https://github.com/dotnet/maui/pull/22298

I think I fixed the issue and my tests seemed to pass. But, not sure if you rebase but keep the tests if you get the same result.

[kubaflo]

@kubaflo I like these tests but I had to re-work the whole background/border/padding parts of the ImageButton because we lost the ripple at some point: #22298

I think I fixed the issue and my tests seemed to pass. But, not sure if you rebase but keep the tests if you get the same result.

@mattleibow I've merged the main branch. I've also added one commit because when I tested with the main branch the Android didn't work IMO. Please have a look:

iOS	Android

After my commit this is how the Android looks like:

But it might not be a correct fix, so please have a look at it 😊

[mattleibow]

Hmmm, this looks like the #22520 issue so you fixed that :)

There is also #22521 which may also be related.

[mattleibow]

This is nice that it fixes Android, I think iOS also has the same issue. How easy is it to fix ios and then we merge? If things are complicated, as buttons on iOS are, then we can do a separate PR.

I know TJ has been working on normal buttons and that has been a pain - especially since we can't derive from UIButton.

But no need to delay this fix for Android just because ios is being difficult. But, if there is basic padding math then it may be worth just doing it all in one go.

[kubaflo]

This is nice that it fixes Android, I think iOS also has the same issue. How easy is it to fix ios and then we merge? If things are complicated, as buttons on iOS are, then we can do a separate PR.

I know TJ has been working on normal buttons and that has been a pain - especially since we can't derive from UIButton.

But no need to delay this fix for Android just because ios is being difficult. But, if there is basic padding math then it may be worth just doing it all in one go.

@mattleibow Yeah, I've added a commit that fixes iOS

[mattleibow]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 3 pipeline(s).

[mattleibow]

I am just looking at the results and the ones with small radii are looking nice, but the one with the big corners has a much smaller image. This is probably the more correct, but I am wondering if this is a breaking change or even what we want.

If I have say a 100x100 image, and a button with 0 border, 0 padding and 0 corner, then the button will be maybe 100x100 (ignoring any weird default paddings)

If I add a 10px border, then I would sort of be OK with the bounds increasing to 120x120. Similarly, if i add 10px padding, the 120x120 is expected.

But, if I add a massive corner, then all of a sudden my image shrinks.

0 pad, border, corner	25% padding	25% border	25% corner
			Current
			Maybe?

If we look at it with more corner:

Base	50% corner PR	50% corner Maybe?

That sort of looks bad because of the image, but if we use a round image:

Base	50% corner PR	50% corner Maybe?

[kubaflo]

Hmmm setting an additional padding based only on the border width seems to work on iOS int additionalPadding = (int)platformButton.Layer.BorderWidth; ,but when doing the same on Android, the images don't look good when the corner radius is big enough

iOS	Android

[kubaflo]

Hmmm setting an additional padding based only on the border width seems to work on iOS int additionalPadding = (int)platformButton.Layer.BorderWidth; ,but when doing the same on Android, the images don't look good when the corner radius is big enough

iOS Android

@mattleibow I think Android would work if the border could overlay the image

[mattleibow]

@kubaflo I finished my comment lol - had to save it so I wouldn't lose it.

But yeah, the image looks weird with the shite, but mostly the image looks OK if the background is transparent.

But, this is maybe not so much the issue of what it looks like, but rather the predictability and intuitiveness.

For example:

• How would you make the ball image in my previous comment where the ball image is the same size as the button?
• When you get a big corner, how can you know what the "added padding" is if you don't know the algorithm.

When the button looks wrong, you know to add padding to make the image smaller. But, if we add the padding, devs have no way to remove when they actually want to clip.

The dotnet_bog image is bad because it is a solid color image, but most images will be some form of transparency where the corners do not matter.

Windows maybe looks OK because the border clips the contents?

We could maybe have a look at wrapping the image in a clipping drawable. I think there is a MaskDrawable or something that we are using already to clip the ripple animation.

[mattleibow]

I was playing with this:

<?xml version="1.0" encoding="utf-8" ?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
             xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
             x:Class="Maui.Controls.Sample.Issues.Issue20801"
             Title="Issue20801">
        <VerticalStackLayout
            Spacing="25"
            Padding="25">

            <ImageButton
                Source="beach_ball.png"
                BorderColor="Blue"
                BackgroundColor="Silver"
                HorizontalOptions="Center"
                HeightRequest="200"
                WidthRequest="200"
                Aspect="Fill"
                Padding="{Binding Value, Source={Reference PaddingSlider}, Converter={StaticResource ThicknessConverter}}"
                BorderWidth="{Binding Value, Source={Reference BorderWidthSlider}}"
                CornerRadius="{Binding Value, Source={Reference CornerRadiusSlider}}"
                AutomationId="imageButton"/>

            <Label Text="Padding" />
            <Slider
                x:Name="PaddingSlider"
                Maximum="200"
                Minimum="0"
                Value="0"/>

            <Label Text="Border Width" />
            <Slider
                x:Name="BorderWidthSlider"
                Maximum="200"
                Minimum="0"
                Value="0"/>

            <Label Text="Corner Radius" />
            <Slider
                x:Name="CornerRadiusSlider"
                Maximum="200"
                Minimum="0"
                Value="0"/>

        </VerticalStackLayout>
</ContentPage>

[mattleibow]

I also notice that changing the padding updates the things correctly, but if I just change the width/corner then the padding calculations do not kick in.

When the border/corner changes, we may have to fire Handler.UpdateValue(nameof(Padding)) in the mapper.

[kubaflo]

@kubaflo I finished my comment lol - had to save it so I wouldn't lose it.

But yeah, the image looks weird with the shite, but mostly the image looks OK if the background is transparent.

But, this is maybe not so much the issue of what it looks like, but rather the predictability and intuitiveness.

For example:

• How would you make the ball image in my previous comment where the ball image is the same size as the button?
• When you get a big corner, how can you know what the "added padding" is if you don't know the algorithm.

When the button looks wrong, you know to add padding to make the image smaller. But, if we add the padding, devs have no way to remove when they actually want to clip.

The dotnet_bog image is bad because it is a solid color image, but most images will be some form of transparency where the corners do not matter.

Windows maybe looks OK because the border clips the contents?

We could maybe have a look at wrapping the image in a clipping drawable. I think there is a MaskDrawable or something that we are using already to clip the ripple animation.

You're right! When the image inside the imageButton fills the available space, then devs can easily adjust the padding property to their needs. I've pushed a commit. I also added a commit that should solve the padding update

[kubaflo]

@mattleibow so is there a need to look into it?

We could maybe have a look at wrapping the image in a clipping drawable. I think there is a MaskDrawable or something that we are using already to clip the ripple animation.

Because I think that the current solution works fine. Check this video out:

https://github.com/dotnet/maui/assets/42434498/bccb2ad1-cd41-4166-aeff-9ec7c5c0c58a

[jsuarezruiz]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 3 pipeline(s).