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

Treat */Auto spans as resolvable finite measures rather than infinite #14648

Merged
merged 3 commits into from
Apr 19, 2023

Conversation

hartez
Copy link
Contributor

@hartez hartez commented Apr 19, 2023

Description of Change

When revising the Grid to handle more * column/row cases as resolvable from the outset (rather than having to measure them multiple times), combination values of Auto/* were treated as requiring unconstrained measures (because of the Auto component). This causes a couple of problems - for one, it means that Labels spanning Auto/* column combinations are always measured as if they will be allowed infinite width; this means they will never wrap. For another, we see problems like #14537 where the native Windows layout system only sees unconstrained width measures and assumes that the native control's DesiredSize is correct even when the container's width expands. This results in a situation where expanding the container does not expand the content, even though we request that it lay out at a larger width.

These changes make the cross-platform Grid treat constrained Auto/* combinations as resolvable (the size the Auto value + the size of the * value, if any) to a finite size and measure accordingly, rather then treating those values as unconstrained. With tests to that effect.

Issues Fixed

Fixes #14537

@hartez hartez force-pushed the fix-14537 branch 2 times, most recently from 970feeb to e1eeffb Compare April 19, 2023 01:40
@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@jsuarezruiz jsuarezruiz added area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter layout-grid labels Apr 19, 2023
@Redth Redth merged commit e0d1e1a into main Apr 19, 2023
28 checks passed
@Redth Redth deleted the fix-14537 branch April 19, 2023 15:57
@Redth
Copy link
Member

Redth commented Apr 19, 2023

/backport to net7.0

@github-actions
Copy link
Contributor

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

@Redth
Copy link
Member

Redth commented Apr 19, 2023

/backport to release/7.0.2xx-sr5

@github-actions
Copy link
Contributor

Started backporting to release/7.0.2xx-sr5: https://github.com/dotnet/maui/actions/runs/4745400816

@github-actions
Copy link
Contributor

@Redth backporting to release/7.0.2xx-sr5 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Treat */Auto spans as finite measurable values rather than infinite Fixes #14537
.git/rebase-apply/patch:171: trailing whitespace.
		public void AutoStarColumnSpanMeasureIsSumOfAutoAndStar(double determinantViewWidth, double widthConstraint) 
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M	src/Core/src/Layouts/GridLayoutManager.cs
M	src/Core/tests/UnitTests/Layouts/GridLayoutManagerTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Core/tests/UnitTests/Layouts/GridLayoutManagerTests.cs
CONFLICT (content): Merge conflict in src/Core/tests/UnitTests/Layouts/GridLayoutManagerTests.cs
Auto-merging src/Core/src/Layouts/GridLayoutManager.cs
CONFLICT (content): Merge conflict in src/Core/src/Layouts/GridLayoutManager.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Treat */Auto spans as finite measurable values rather than infinite Fixes #14537
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions
Copy link
Contributor

@Redth an error occurred while backporting to release/7.0.2xx-sr5, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

@Redth Redth added backport/suggested The PR author or issue review has suggested that the change should be backported. backport/approved After some discussion or review, this PR or change was approved to be backported. labels Apr 19, 2023
hartez added a commit that referenced this pull request Apr 19, 2023
hartez added a commit that referenced this pull request Apr 19, 2023
Redth pushed a commit that referenced this pull request Apr 19, 2023
@nor0x
Copy link
Contributor

nor0x commented Apr 30, 2023

@Redth when can we expect another "service release" with this fix?
it's kind of a severe issue and having to wait weeks for an update after a completed PR is not great!

@nll
Copy link

nll commented May 10, 2023

@nor0x A new service release was created a few hours ago: https://github.com/dotnet/maui/releases/tag/7.0.86

@hartez
Copy link
Contributor Author

hartez commented May 10, 2023

That service release does not have this fix in it.

@hartez
Copy link
Contributor Author

hartez commented May 10, 2023

As a workaround (if you don't want to wait for the service release), you can grab the updated Grid code from main and drop it into your app using a custom layout manager: https://github.com/hartez/CustomLayoutExamples#custom-layoutmanagers-for-existing-layouts

@baaaaif
Copy link

baaaaif commented May 11, 2023

That service release does not have this fix in it.

@hartez
SR5 was skipped according to the github release page and SR6 was released directly with the fix

@nor0x
Copy link
Contributor

nor0x commented May 11, 2023

@hartez

That service release does not have this fix in it.

it is mentioned in the bug fixes 🤔

@nll
Copy link

nll commented May 11, 2023

That service release does not have this fix in it.

The release notes clearly include a reference to the a backport of this pull request here.

Is "the process" not working again (#14520) ?

Screenshot 2023-05-11 at 09 29 55

@nor0x
Copy link
Contributor

nor0x commented May 11, 2023

i downloaded the assets of the release and indeed the changes from this PR are in there:
image

i'm not 100% sure if this is correct though, since the code from the release is already 2 weeks behind the current state - this whole process is such a mess!
image

and even after my VS and workload updates my app still has a broken layout

@hartez
Copy link
Contributor Author

hartez commented May 30, 2023

That service release does not have this fix in it.

I was wrong about this, I apologize. I must have been looking at the wrong branch, the commit is clearly there.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 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 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. layout-grid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[regression/7.0.81] Grid layouts are not working as they did in 7.0.59 (Windows)
6 participants