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

Make layout honor MaxWidth and MaxHeight requests #15022

Merged
merged 4 commits into from May 11, 2023
Merged

Conversation

jknaudt21
Copy link
Contributor

@jknaudt21 jknaudt21 commented May 10, 2023

Description of Change

Layouts were not taking into account a view's MaxWidth/Height if the layout's alignment policy was set to Fill. This caused the frustrating behavior that you would specify an element's MaxWidth/Height but have these values completely ignored.

The fix consists on making the ComputeFrame method and the AlignHorizontal/AlignVertical methods to take into account these properties. The changes can be summarized as follows:

If an element has an unset WidthRequest but has a MaximumWidthRequest, then its consumed width should be the minimum between the MaximumWidthRequest and the available space (i.e bounds.Width). This type of logic should also be used in the alignment calculations iff the WidthRequest is not set.

Below are screenshots of the before and after of running this code on different platforms:

    <StackLayout>
            <Button Text="Some random text that I've typed to occupy space"
                    BackgroundColor="AliceBlue"
                    TextColor="Black"
                    MaximumWidthRequest="100"
                    />
    </StackLayout>
Before After
image
image
image

Notice how the buttons are now smaller and obey their MaximumWidthRequests.


Las but not least, some of the existing unit tests were modified to take into account MaximumWidth/Height since the substitutes would return 0 for these values. I've also added tests to ensure that layouts behave as expected when the Width and Height are not set but their Maximum counterparts are.

Issues Fixed

Fixes #15002

If Width is not explicitly set but MaximumWidth is, we should still use that as part of the arranging calculations
@jknaudt21 jknaudt21 added the area/layout 🔲 StackLayout, GridLayout, ScrollView, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter label May 10, 2023
@github-actions
Copy link
Contributor

Thank you for your pull request. We are auto-formatting your source code to follow our code guidelines.

@mattleibow mattleibow merged commit 773f8c2 into main May 11, 2023
28 checks passed
@mattleibow mattleibow deleted the dev/jd/maxwidth branch May 11, 2023 04:29
@mattleibow mattleibow added the backport/suggested The PR author or issue review has suggested that the change should be backported. label May 11, 2023
rmarinho pushed a commit that referenced this pull request May 30, 2023
* Ensure MaximumWidth is used when arranging

If Width is not explicitly set but MaximumWidth is, we should still use that as part of the arranging calculations

* Account for alignment

* Add tests

* Auto-format source code

---------

Co-authored-by: GitHub Actions Autoformatter <autoformat@example.com>
@hartez hartez added the backport/approved After some discussion or review, this PR or change was approved to be backported. label May 31, 2023
@hartez
Copy link
Contributor

hartez commented Jun 1, 2023

/backport to net7.0

@github-actions
Copy link
Contributor

github-actions bot commented Jun 1, 2023

Started backporting to net7.0: https://github.com/dotnet/maui/actions/runs/5138858004

@zwikk
Copy link

zwikk commented Jun 21, 2023

Awesome fix! Found a bug though, see #15785

@github-actions github-actions bot locked and limited conversation to collaborators Dec 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/layout 🔲 StackLayout, GridLayout, ScrollView, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter backport/approved After some discussion or review, this PR or change was approved to be backported. backport/suggested The PR author or issue review has suggested that the change should be backported.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Button not honoring MaximumWidthRequest on iOS, Mac, and Android
4 participants