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

[regression/8.0.0-preview.5.8529] Grid with a star and an empty auto column leads to an incorrect layout #16334

Closed
ivan-todorov-progress opened this issue Jul 25, 2023 · 6 comments · Fixed by #16488
Labels
area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter fixed-in-7.0.96 Look for this fix in 7.0.96 SR8! fixed-in-7.0.100 fixed-in-7.0.101 fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 i/regression This issue described a confirmed regression on a currently supported version layout-grid p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with partner Issue or Request from a partner team platform/android 🤖 platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst platform/windows 🪟 t/bug Something isn't working
Milestone

Comments

@ivan-todorov-progress
Copy link

ivan-todorov-progress commented Jul 25, 2023

Description

When using a Grid layout with * and Auto columns and there is no content in the Auto column, the result layout is incorrect.

For example, consider the following Grid layout:

<Grid ColumnDefinitions="*, Auto" RowDefinitions="Auto, *">
    <Label Grid.Row="0" Text="First Name" />
    <Entry Grid.Row="1" />
</Grid>

The above Grid contains children in the * column only, while the Auto is empty. The following screenshot demonstrates how the layout appears initially:

image

After resizing the window, the layout appears correctly:

image

The above screenshot is on Windows, but similar problems can be observed on the other platforms as well.

Steps to Reproduce

  1. Run the sample project from the provided link.
  2. Observe the incorrect initial layout.
  3. Resize the window or rotate the phone/tablet.
  4. Observe the correct layout after resizing.

Link to public reproduction project repository

https://github.com/telerik/ms-samples/tree/main/Maui/GridStarAutoEmptyIncorrectLayout

Version with bug

8.0.0-preview.5.8529

Last version that worked well

7.0.86

Affected platforms

iOS, Android, Windows, macOS

Affected platform versions

N/A

Did you find any workaround?

No response

Relevant log output

No response

@ivan-todorov-progress ivan-todorov-progress added the t/bug Something isn't working label Jul 25, 2023
@Stedy59
Copy link

Stedy59 commented Jul 25, 2023

You specified two columns, but the grids children have no grid column value. The documentation for Maui Grid states... You must specify row and/or column if more than one. Xamarin would "kind of" figure that out for you, but moving forward, Maui doesn't. Hope that helps.

@ivan-todorov-progress
Copy link
Author

ivan-todorov-progress commented Jul 25, 2023

@Stedy59 If Grid.Column is not explicitly specified, it defaults to 0. Please, keep in mind that this is a repro project, stripped down for simplicity. The actual use case is more sophisticated than that, but I have omitted some unrelated details.

For example, I have the following requirement:

  • On desktop, put the Label to the left of the Entry.
  • On mobile, put the Label above the Entry.

One way to accomplish this is to duplicate the entire view for desktop and mobile, just to specify a different placement for that specific Label, but that would be too tedious and difficult to maintain. Another option is to use a Grid with sufficient rows and columns, in combination with the OnIdiom markup extension:

<Label Grid.Column="{OnIdiom Default=0, Phone=1}" Grid.Row="{OnIdiom Default=1, Phone=0}" />

That is just one example. Another case is to have a child in an Auto column, which is shown or hidden based on a condition. I can give you more such examples, but I believe you get the idea.

The main problem is that when we have an Auto column, which does not have a visible content, the Grid ignores the sizes of the rest of its children and simply evaluates the entire row height to 0. And that is done initially only. Resizing the window or rotating the device recomputes the layout, correctly this time.

@Stedy59
Copy link

Stedy59 commented Jul 25, 2023

Try this...

<VerticalStackLayout
Spacing="10">
<StackLayout
Spacing="10"
Orientation="{OnIdiom Default='Horizontal', Phone='Vertical'}">
<Label
Text="First Name:"/>
<Entry
MinimumWidthRequest="200"/>
</StackLayout>
<StackLayout
Spacing="10"
Orientation="{OnIdiom Default='Horizontal', Phone='Vertical'}">
<Label
Text="Last Name:"/>
<Entry
MinimumWidthRequest="200"/>
</StackLayout>
</VerticalStackLayout>

Make the inner StackLayout a global style, and use everywhere.

@ivan-todorov-progress
Copy link
Author

ivan-todorov-progress commented Jul 25, 2023

@Stedy59 Thanks for the suggestion. I have a workaround for my particular scenario, but it is more involved. I am reporting a regression in the Grid layout that worked well with the latest official version, but was broken in the latest preview, in hope Microsoft would have enough time to fix it before .NET 8 goes official.

@samhouts samhouts added area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter i/regression This issue described a confirmed regression on a currently supported version layout-grid labels Jul 26, 2023
@samhouts samhouts added this to the .NET 8 GA milestone Jul 26, 2023
@samhouts samhouts added the partner Issue or Request from a partner team label Aug 1, 2023
@mikeparker104 mikeparker104 added the partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with label Aug 1, 2023
@samhouts samhouts changed the title Grid with a star and an empty auto column leads to an incorrect layout [regression/8.0.0-preview.5.8529] Grid with a star and an empty auto column leads to an incorrect layout Aug 1, 2023
@Redth
Copy link
Member

Redth commented Aug 1, 2023

Just to add a bit more context, the sample project repros but the sample in the issue description does not.

It seems like the condition here is when there's an auto/unbound measure involved only. If you just put the one Grid (First Name with entry) in a page, it doesn't repro, but as soon as I nest it inside a VerticalStackLayout (or even inside a ) it repros.

So, without going into the repro project, you can simply use this XAML:

<VerticalStackLayout Margin="20">
    <Grid ColumnDefinitions="*, Auto" RowDefinitions="Auto, *" ColumnSpacing="10" RowSpacing="10">
        <Label Grid.Row="0" Text="First Name" />
        <Entry Grid.Row="1" />
    </Grid>
</VerticalStackLayout>

@hartez
Copy link
Contributor

hartez commented Aug 1, 2023

So this and #16368 are the same bug.

@samhouts samhouts added the p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint label Aug 2, 2023
rmarinho pushed a commit that referenced this issue Aug 10, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 3, 2023
@samhouts samhouts added the fixed-in-7.0.96 Look for this fix in 7.0.96 SR8! label Oct 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter fixed-in-7.0.96 Look for this fix in 7.0.96 SR8! fixed-in-7.0.100 fixed-in-7.0.101 fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 i/regression This issue described a confirmed regression on a currently supported version layout-grid p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with partner Issue or Request from a partner team platform/android 🤖 platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst platform/windows 🪟 t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants