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.

Sign up for GitHub

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

Jump to bottom

Fix for FlexLayout padding issue #24195

Merged
merged 17 commits into from
Aug 30, 2024
Merged

Fix for FlexLayout padding issue #24195

merged 17 commits into from
Aug 30, 2024

Conversation

BagavathiPerumal
Copy link
Contributor

@BagavathiPerumal BagavathiPerumal commented Aug 13, 2024 •
edited by samhouts
Loading

Root cause

The FlexLayout doesn't include the padding value during its arrangement and measurement processes. As a result, the padding is not applied in the FlexLayout.

Description of Issue Fix

The fix involves updating the layout arrangement to include padding values, adjust bounds, and arrange children accordingly. The measurement process is adjusted to exclude padding during measurement, then add it back to determine the final size. This ensures accurate layout and measurement with proper padding.

Issues Fixed

Fixes #15991

Reference:

Referred code changes from HorizontalStackLayoutManager.cs file.

maui/src/Core/src/Layouts/HorizontalStackLayoutManager.cs

Line 12 in 210bc35

public override Size Measure(double widthConstraint, double heightConstraint)

Output

Before fix:

image

After fix:

image

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Aug 13, 2024
@jfversluis
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Aug 18, 2024 •
edited by jfversluis
Loading

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@BagavathiPerumal BagavathiPerumal marked this pull request as ready for review August 19, 2024 11:25
@BagavathiPerumal BagavathiPerumal requested a review from a team as a code owner August 19, 2024 11:25
@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@Redth
Copy link
Member

Redth commented Aug 22, 2024

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen
PureWeen requested changes Aug 25, 2024
View reviewed changes
Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this one, instead of doing a screenshot test can we move these tests to the "UnitTests" ?

Since none of the changes in here depend on anything platformspecific?

Here's an example of some tests against the new FlexLayout

maui/src/Controls/tests/Core.UnitTests/Layouts/FlexLayoutTests.cs

Line 11 in c156658

public class FlexLayoutTests

@PureWeen
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@BagavathiPerumal
Copy link
Contributor Author

For this one, instead of doing a screenshot test can we move these tests to the "UnitTests" ?

Since none of the changes in here depend on anything platformspecific?

Here's an example of some tests against the new FlexLayout

maui/src/Controls/tests/Core.UnitTests/Layouts/FlexLayoutTests.cs

Line 11 in c156658

public class FlexLayoutTests

@PureWeen I have updated the tests to Unit Tests, could you please validate once and let me know if there is any concerns

@samhouts samhouts added the area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter label Aug 27, 2024
@PureWeen
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen enabled auto-merge (squash) August 29, 2024 16:52
rmarinho
rmarinho approved these changes Aug 30, 2024
View reviewed changes
Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing test not related

@rmarinho rmarinho disabled auto-merge August 30, 2024 16:29
@rmarinho rmarinho merged commit ddab6b3 into dotnet:main Aug 30, 2024
95 of 97 checks passed
@samhouts samhouts added fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Sep 5, 2024
@samhouts samhouts added fixed-in-9.0.0-rc.2.24503.2 and removed fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Oct 14, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@PureWeen PureWeen PureWeen approved these changes

@rmarinho rmarinho rmarinho approved these changes

@jfversluis jfversluis Awaiting requested review from jfversluis jfversluis is a code owner automatically assigned from dotnet/dotnet-maui-reviewers

@jsuarezruiz jsuarezruiz Awaiting requested review from jsuarezruiz jsuarezruiz is a code owner automatically assigned from dotnet/dotnet-maui-reviewers

Assignees
No one assigned
Labels
area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter community ✨ Community Contribution fixed-in-9.0.0-rc.2.24503.2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FlexLayout Padding not working
6 participants
@BagavathiPerumal @jfversluis @PureWeen @Redth @rmarinho @samhouts