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

CollectionView, the footer moves to the bottom of the page when the empty view or empty view template is enabled #24997

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

Vignesh-SF3580
Copy link
Contributor

@Vignesh-SF3580 Vignesh-SF3580 commented Sep 30, 2024

Root Cause

Android: The issue occurred when using HeaderTemplate and FooterTemplate in a CollectionView. The footer template was not arranged properly after switching from an empty view to an item view. This was because the empty view incorrectly occupied the entire available space, pushing the footer to the bottom of the screen. The incorrect behavior stemmed from the UpdateEmptyViewSize method, which did not properly measure and adjust the size of the empty view, especially when using templates. The footer template was never created because the RecyclerView did not allocate space for it due to the wrong measurement of the empty view.

Description of Change

The fix involved updating the UpdateEmptyViewSize method to accurately measure the height of the empty view, whether it was a direct IView or a DataTemplate. By ensuring the correct height was set for the RecyclerView, we allowed the footer template to be rendered correctly. This change ensures that the empty view does not take up the entire available space, and the footer can appear as expected when transitioning between item views and empty views.

Fix done at Items/ItemsViewHandler.Android.cs

Root Cause

iOS: The issue arose when Header, Footer, and EmptyView is used in collectionview. The DetermineEmptyViewFrame method was not properly calculating the frame for the EmptyView, resulting in the EmptyView taking up available amount of space. This caused footer view move to the bottom of the screen.

Description of Change

The fix involved updating the DetermineEmptyViewFrame method in the ItemsViewController class to accurately measure the height of the EmptyView element using _emptyViewFormsElement.Measure(). Additionally, in the StructuredItemsViewController, a check was added to ensure that if an EmptyView is present, the base DetermineEmptyViewFrame method is called to maintain consistent sizing logic. By ensuring the correct frame is set for the EmptyView, the issues with Footer were resolved, preventing the EmptyView from taking up all available space.

Fix done at Items/iOS/ItemsViewController.cs , Items/iOS/StructuredItemsViewController.cs.

Issues Fixed

Fixes #24966
Fixes #11847 (Android)

Tested the behaviour in the following platforms

  • Android
  • Windows
  • iOS
  • Mac

Screenshots

iOS:

Before Issue Fix After Issue Fix

Android:

Before Issue Fix After Issue Fix

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Sep 30, 2024
@jfversluis jfversluis added the area-controls-collectionview CollectionView, CarouselView, IndicatorView label Sep 30, 2024
@Vignesh-SF3580 Vignesh-SF3580 marked this pull request as ready for review September 30, 2024 11:33
@Vignesh-SF3580 Vignesh-SF3580 requested a review from a team as a code owner September 30, 2024 11:33
@rmarinho
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

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.

test failing

App.Tap("RemoveButton");
App.Tap("RemoveButton");
// Here we check for dynamic Emptyview and Footer proper proper alignment in view it should not be at the bottom of screen.
VerifyScreenshot();
Copy link
Contributor

Choose a reason for hiding this comment

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

On iOS the snapshot seems to be correct:
image
However, on Android, the items still appear. It should not happen after tapping the remove button.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On iOS the snapshot seems to be correct: image However, on Android, the items still appear. It should not happen after tapping the remove button. image

The fix for this issue is associated with PR 24830, we can get the correct image once the mentioned PR is merged

Copy link
Member

Choose a reason for hiding this comment

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

It was merged, we can rebase this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rebased

@Vignesh-SF3580 Vignesh-SF3580 marked this pull request as draft October 8, 2024 06:49
@rmarinho
Copy link
Member

rmarinho commented Oct 9, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@Vignesh-SF3580
Copy link
Contributor Author

/rebase

@Vignesh-SF3580 Vignesh-SF3580 marked this pull request as ready for review October 10, 2024 12:03
@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Oct 11, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@Vignesh-SF3580
Copy link
Contributor Author

/rebase

Copy link

Azure Pipelines successfully started running 3 pipeline(s).


if (_emptyViewFormsElement is not null)
{
emptyViewHeight = (nfloat)_emptyViewFormsElement.Measure(CollectionView.Frame.Width, double.PositiveInfinity).Request.Height;
Copy link
Member

Choose a reason for hiding this comment

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

For this can you cast _emptyViewFormsElement as an IView first before measuring? This won't be necessary once we're on net9 but for now it's still useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the code based on your feedback. Could you please review it and let me know if there are any concerns?

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).


[Test]
[Category(UITestCategories.CollectionView)]
public void CollectionViewFooterMovestoBottomWithEmptyvieworEmptyviewTemplate()
Copy link
Member

Choose a reason for hiding this comment

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

Test is failing

Copy link
Contributor

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since updating to .NET 9 and enabling CollectionViewHandler2 in the iOS testing projects, we’re encountering UI test failures due to layout differences between CollectionViewHandler(with our custom fix in .NET 8) and CollectionViewHandler2. In the updated handler, the empty view is positioned incorrectly, overlapping the header, which alters the expected UI layout and causes screenshot comparison failures in tests.
The previous alignment fix for the empty view does not work with CollectionViewHandler2 due to differences in rendering flow. We are currently working on fixing this issue to ensure similar behavior to CollectionViewHandler for consistent UI rendering across versions.

A separate issue has been created to track progress: #25606,

@praveenkumarkarunanithi
Copy link
Contributor

/rebase

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

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 platform/android 🤖 platform/iOS 🍎
Projects
None yet
6 participants