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

[iOS] Fixed HeaderFooterGrid to pass on CV1 and CV2 #26022

Merged
merged 7 commits into from
Dec 12, 2024

Conversation

devanathan-vaithiyanathan
Copy link
Contributor

@devanathan-vaithiyanathan devanathan-vaithiyanathan commented Nov 21, 2024

Issue details

The size of the CollectionView, including the header and footer, is not rendered correctly on the UI in CV1 but displays correctly in CV2.

Root cause

The GetSize() method calculated the size of the CollectionView based only on its content, ignoring the header and footer sizes. This caused incorrect height or width rendering when a header or footer was used.

Description of Change

The fix calculates the sizes of the header and footer using helper methods, This ensures the total size includes all elements, and the result is adjusted to fit within the CollectionView bounds.

Issues Fixed

Fixes #25773

Tested the behaviour in the following platforms

  • Android
  • Windows
  • iOS
  • Mac

Output Screenshot

Before After

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Nov 21, 2024
@jsuarezruiz jsuarezruiz added platform/iOS 🍎 area-controls-collectionview CollectionView, CarouselView, IndicatorView labels Nov 21, 2024
@devanathan-vaithiyanathan devanathan-vaithiyanathan marked this pull request as ready for review November 25, 2024 10:08
@devanathan-vaithiyanathan devanathan-vaithiyanathan requested a review from a team as a code owner November 25, 2024 10:08
@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Nov 25, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

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.

It s failing some tests..

https://dev.azure.com/xamarin/public/_apis/test/Runs/3418613/Results/100053/Attachments/14

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.

Could this be CV2 failing

image

image

@devanathan-vaithiyanathan
Copy link
Contributor Author

Could this be CV2 failing

image

image

Hi @rmarinho,

In CV1, It appears that the VerticalItemsSpacing in the GridItemsLayout is creating excessive spacing in the last section of the items layout. The build failures are due to these ItemSpacing discrepancies, resulting in snapshot mismatches.

The following Issue was previously identified and resolved, and a separate PR was created.

PR Link Reference => #25825

In this Focused PR, We have made targeted changes specifically to resolve the VerticalItemsSpacing issue for the last section of CV items. We expect the next CI run to pass with these updates.

Code Changes :
https://github.com/dotnet/maui/blob/0e5aabf62ed9ec4213066de39b090a20f93a414d/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewLayout.cs#L136C4-L141C5

Please review the PR and let us know if any further clarification.

I implemented a fix for the CV VerticalSpacing issue to address a test case failure. However, since the same changes were recently merged. I have reverted my changes.

PR => dotnet#25882
This reverts commit 47e9893.
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@devanathan-vaithiyanathan
Copy link
Contributor Author

@rmarinho,
image

We have attached the original and error snapshot images for reference. As highlighted by the difference between the two, there is a noticeable pixel error in the layout of the header and footer Grid in the CV1 snapshot.

Would it be possible to replace the header and footer Grid image for this case or are there other adjustments we can make to resolve this pixel discrepancy? Please share your thoughts on this.

Looking forward to your feedback! Thank you in advance.

@dotnet dotnet deleted a comment from azure-pipelines bot Dec 6, 2024
@rmarinho rmarinho added this to the .NET 9 SR3 milestone Dec 6, 2024
@rmarinho rmarinho self-assigned this Dec 6, 2024
@sheiksyedm sheiksyedm added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Dec 6, 2024
@rmarinho
Copy link
Member

rmarinho commented Dec 9, 2024

Can we try replace the image to check if it passes?

@devanathan-vaithiyanathan
Copy link
Contributor Author

Can we try replace the image to check if it passes?

Thank you for the suggestion. We will replace the header and footer Grid images to check if the build passes with the updated snapshot.

@rmarinho
Copy link
Member

rmarinho commented Dec 9, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@rmarinho
Copy link
Member

/rebase

@rmarinho rmarinho merged commit efa6895 into dotnet:main Dec 12, 2024
96 of 104 checks passed
rmarinho added a commit that referenced this pull request Dec 13, 2024
@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 Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView community ✨ Community Contribution fixed-in-net8.0-nightly This may be available in a nightly release! fixed-in-net9.0-nightly This may be available in a nightly release! partner/syncfusion Issues / PR's with Syncfusion collaboration platform/iOS 🍎
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[iOS] Fix HeaderFooterGrid to pass on CV1 and CV2
7 participants