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

[9.0, GA, Android] An Android Button Minimum Size is Enforced #25903

Closed
david-maw opened this issue Nov 16, 2024 · 16 comments
Closed

[9.0, GA, Android] An Android Button Minimum Size is Enforced #25903

david-maw opened this issue Nov 16, 2024 · 16 comments
Labels
area-controls-button Button, ImageButton i/regression This issue described a confirmed regression on a currently supported version platform/android 🤖 s/needs-attention Issue has more information and needs another look s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working

Comments

@david-maw
Copy link

david-maw commented Nov 16, 2024

Description

In RC2 you could set an Android Button height or width to numbers less than 44, now the limit is 44.

Steps to Reproduce

This XAML snippet shows it

        <HorizontalStackLayout BackgroundColor="Yellow">
            <Button BackgroundColor="Green" HeightRequest="30"/>
            <Rectangle Background="Red" HeightRequest="30" WidthRequest="30"/>
        </HorizontalStackLayout>

On RC2 Android the button and rectangle will be the same height, on GA Android (or on GA or RC2 Windows) the button will be height 44, visibly larger than the rectangle.

It looks like this was a result of #25163 which, on the face of it, is a reasonable observation, but fixing it creates surprises for anyone who (like me) was inadvertently relying on the fact that on Android the default minimum height was ignored. I'd naively assumed it was something very small without checking. At the very least it's worth documenting as a breaking change because it has worked like this for a long time.

Link to public reproduction project repository

No response

Version with bug

9.0.0 GA

Is this a regression from previous behavior?

Yes, this used to work in .NET MAUI

Last version that worked well

9.0.0-rc.2.24503.2

Affected platforms

Android, I was not able test on other platforms

Affected platform versions

No response

Did you find any workaround?

No response

Relevant log output

@david-maw david-maw added the t/bug Something isn't working label Nov 16, 2024
Copy link

We've found some similar issues:

If any of the above are duplicates, please consider closing this issue out and adding additional context in the original issue.

Note: You can give me feedback by 👍 or 👎 this comment.

@jfversluis jfversluis added platform/android 🤖 area-controls-button Button, ImageButton potential-regression This issue described a possible regression on a currently supported version., verification pending labels Nov 17, 2024
@jfversluis
Copy link
Member

@spadapet does that right a bell?

@spadapet
Copy link
Contributor

@jfversluis that does ring a bell. Although the fix for the following bug was only supposed to have an effect if Minimum/MaximumHeightRequest were set along with HeightRequest:

Neither of those are set in the example, but maybe they are coming from a Style?

@mattleibow
Copy link
Member

I think the min widths and heights are set in the styles.xaml? Maybe remove it there?

@mattleibow mattleibow added the s/needs-info Issue needs more info from the author label Nov 18, 2024
@spadapet
Copy link
Contributor

I'm using .NET 9:

Image

And I couldn't reproduce the issue with a new MAUI app from template. Here's what I tried (along with adding a Button of height 44, with the rendered height set as the text in the Buttons):

Image

So I'm probably missing something from what @david-maw had. Please provide a small sample app if I'm missing something.

@david-maw
Copy link
Author

Thanks @spadapet your speculation about styles was correct, my Styles.xaml was generated by VS and contains under the Button style:

        <Setter Property="MinimumHeightRequest" Value="44"/>
        <Setter Property="MinimumWidthRequest" Value="44"/>

I just generated a new .NET 9 MAUI project in VS and those lines are present there too. How did you generate your app?

@dotnet-policy-service dotnet-policy-service bot added s/needs-attention Issue has more information and needs another look and removed s/needs-info Issue needs more info from the author labels Nov 18, 2024
@david-maw
Copy link
Author

Oops, sorry @mattleibow I misattributed your styles speculation (which was correct).

@spadapet
Copy link
Contributor

spadapet commented Nov 18, 2024

Thanks @spadapet your speculation about styles was correct, my Styles.xaml was generated by VS and contains under the Button style:

    <Setter Property="MinimumHeightRequest" Value="44"/>
    <Setter Property="MinimumWidthRequest" Value="44"/>

I just generated a new .NET 9 MAUI project in VS and those lines are present there too. How did you generate your app?

@david-maw I'm using Visual Studio 2022 version 17.12. I simply created a new project from MAUI template:

Image

Then in MainPage.xaml I replaced the contents of the ContentPage with:

    <HorizontalStackLayout BackgroundColor="Yellow">
        <Button BackgroundColor="Green" HeightRequest="30" Text="{Binding Height, Source={x:RelativeSource Self}}" />
        <Rectangle Background="Red" HeightRequest="30" WidthRequest="30"/>
        <Button BackgroundColor="Green" HeightRequest="44" Text="{Binding Height, Source={x:RelativeSource Self}}" />
    </HorizontalStackLayout>

In Styles.xaml it does have a MinimumHeightRequest for Buttons:

Image

So this behavior I'm seeing is exactly the bug I made a fix for:

Now I see my PR for that bug was merged into .NET 9 SR1, so it's not part of the .NET 9 GA release. Maybe the SR1 release will fix this bug?

@david-maw
Copy link
Author

Weirder and weirder, you see the MinimumHeighRrequest ignored in .NET 9, I see it honored (which is the behavior change I was complaining about), The change I saw was between RC2 and GA, RC2 ignored the MinimumHeighRrequest just like earlier releases, but the GA release honored it as it always should have.

So something fixed the bug (and thus changed the behavior) in the GA release, but it did it for me and not for you. I repeated your test and it performed exactly as you described, then I repeated my original test which was to stick the XAML into an existing test app and the problem reappeared. I'll spend some time figuring out the difference and get back to you.

@kevinxufei kevinxufei added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed i/regression This issue described a confirmed regression on a currently supported version and removed potential-regression This issue described a possible regression on a currently supported version., verification pending labels Nov 19, 2024
@kevinxufei
Copy link

I can repro this issue at Android platform on the latest 17.13 P1(9.0.10).The results are shown in the figure.Image
However, the button and rectangle will be the same height on 9.0.0 and 9.0.0-rc.2.24503.2.
Image

@spadapet
Copy link
Contributor

I can repro this issue at Android platform on the latest 17.13 P1(9.0.10).The results are shown in the figure. However, the button and rectangle will be the same height on 9.0.0 and 9.0.0-rc.2.24503.2.

Thanks for testing @kevinxufei. It verifies that MAUI 9.0.10 does fix the issue where MinimumHeightRequest=44 from the Style was being ignored. It's definitely a change from RC and GA because now HeightRequest less than 44 won't work unless you change the default Style for Button.

@david-maw
Copy link
Author

Which is weird @spadapet, since your fix didn't go into the GA release and, as you noted (and I verified), it does not fail in the GA release with a new MAUI app from the template. The explanation for that seems to be that a new MAUI app from the template uses Microsoft.Maui.Controls 9.0.0. All you have to do is switch to 9.0.10 and the changed behavior appears.

I thought 9.0.10 was the GA release, but it seems to be the SR1 with your change in it?

I dont think it matters now we understand what's going on but I was able to reproduce the problem in an App migrated from .NET 8, because it uses Microsoft.Maui.Controls 9.0.10 we now know. That repo is https://github.com/david-maw/SimpleCollectionView-ButtonHeight.git. I used a version of the XAML you used and in .NET 8 it showed:
Image

But in .NET 9 it honored the minimum height request from the styles and showed this:
Image

So, all appears to be clear, the only question is what to do about the breaking change, which is no longer a problem for me now I know what's going on, but might still surprise others.

@spadapet
Copy link
Contributor

I thought 9.0.10 was the GA release, but it seems to be the SR1 with your change in it?

I'm not sure about exact GA vs SR1 version numbers for MAUI. Maybe they can differ compared to .NET 9.
But if I look at tags for the MAUI releases, my change was not in 9.0.0 and it is in 9.0.10.

Here's the exact commit:

@david-maw
Copy link
Author

What seems to have happened is 9.0.0 was shipped and is what a new MAUI app uses by default, but 9.0.10 is also available. Whether 9.0.10 was available at the time of GA I don't know, that's history now :-)

@mattleibow
Copy link
Member

mattleibow commented Nov 20, 2024

So I just want to check up on this... The issue is fixed now with at least 9.0.10 and the min height is respected.

@davidortinau maybe this is something that we can put in the changes doc from net8 to net9.

Sure, 9.0.0 also has the old behavior, but we can say it is as designed for net9. And then try get VS to use 9.0.10 ASAP.

For this issue specifically, is there more action needed?

@PureWeen PureWeen removed the s/needs-attention Issue has more information and needs another look label Nov 27, 2024
@PureWeen PureWeen added the s/needs-info Issue needs more info from the author label Nov 27, 2024
@PureWeen PureWeen modified the milestones: .NET 9 SR2, .NET 9 Servicing Nov 27, 2024
@david-maw
Copy link
Author

I'm assuming there isn't a question, just an over-zealous bot?

@dotnet-policy-service dotnet-policy-service bot added s/needs-attention Issue has more information and needs another look and removed s/needs-info Issue needs more info from the author labels Nov 27, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in MAUI SDK Ongoing Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-button Button, ImageButton i/regression This issue described a confirmed regression on a currently supported version platform/android 🤖 s/needs-attention Issue has more information and needs another look s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

6 participants