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

Fixed Header and Footer template is not visible in CollectionView #26844

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

NirmalKumarYuvaraj
Copy link
Contributor

@NirmalKumarYuvaraj NirmalKumarYuvaraj commented Dec 27, 2024

Issue Details:

When both the HeaderTemplate and FooterTemplate are declared for a CollectionView, neither template is visible in the view.

Root Cause:

In the StructuredItemsView handler, the UpdateHeader() and UpdateFooter() methods return null for the Header property, resulting in the switch case returning null as well. Additionally, both the Header and MauiContext are found to be null when mapped to the native ListViewBase.Header, causing the header to fail to render properly

Description of Change:

Fixed by recalculating the Header value to account for both the Header and HeaderTemplate. Furthermore, the MauiContext was explicitly passed as a parameter to the ItemTemplateContext during the mapping process. This ensures that the MauiContext is properly associated with ListViewBase.Header, enabling the header to render as expected. Processed same for footer template in UpdateFooter()

Tested the behavior in the following platforms.

  • Android
  • Windows
  • iOS
  • Mac

Issue not reproduced in iOS and macOS platforms as it is fixed by this PR #25418

Issues Fixed:

Fixes #22892

Screenshots

Before Issue Fix After Issue Fix

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Dec 27, 2024
@karthikraja-arumugam karthikraja-arumugam added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Dec 30, 2024
@NirmalKumarYuvaraj NirmalKumarYuvaraj marked this pull request as ready for review January 3, 2025 04:36
@NirmalKumarYuvaraj NirmalKumarYuvaraj requested a review from a team as a code owner January 3, 2025 04:36
@PureWeen
Copy link
Member

PureWeen commented Jan 6, 2025

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@@ -108,7 +108,7 @@ protected virtual void UpdateHeader()
_currentHeader = null;
}

var header = ItemsView.Header;
var header = ItemsView.Header ?? ItemsView.HeaderTemplate;
Copy link
Member

Choose a reason for hiding this comment

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

If we just left this as var header = ItemsView.Header

And then just deleted the
case null:

would that do the same thing?

We might still want it to fall through to the default case

That way it will set both Header properties on the ListViewBase to null

Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine as is but I would change this to be a boolean, HasHeader and change switch statement. for example. ON the null I would also would make sure we clear the ListViewBase.HeaderTemplate just in case

This comment was marked as outdated.

Copy link
Member

Choose a reason for hiding this comment

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

Previous comment was wrong :)

Having the switch here is fine, but the default case still having to check the header is a bit redundant? I would think it should just be a case DataTemplate headerTemplate:

And then we would not need the null and default would be the null where we null out both.

Copy link
Member

Choose a reason for hiding this comment

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

I think we still need the null if you clear the template and we want to clear on the native side too.

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.

Needs some tweaks

@@ -108,7 +108,7 @@ protected virtual void UpdateHeader()
_currentHeader = null;
}

var header = ItemsView.Header;
var header = ItemsView.Header ?? ItemsView.HeaderTemplate;
Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine as is but I would change this to be a boolean, HasHeader and change switch statement. for example. ON the null I would also would make sure we clear the ListViewBase.HeaderTemplate just in case

@@ -108,7 +108,7 @@ protected virtual void UpdateHeader()
_currentHeader = null;
}

var header = ItemsView.Header;
var header = ItemsView.Header ?? ItemsView.HeaderTemplate;

This comment was marked as outdated.

@rmarinho rmarinho requested a review from PureWeen January 7, 2025 14:04
@rmarinho rmarinho added the area-controls-collectionview CollectionView, CarouselView, IndicatorView label Jan 7, 2025
@rmarinho rmarinho merged commit 4c42078 into dotnet:main Jan 7, 2025
111 checks passed
@rmarinho rmarinho modified the milestones: .NET 9 SR3, .NET 9 SR4 Jan 7, 2025
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 partner/syncfusion Issues / PR's with Syncfusion collaboration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS/Windows] CollectionView HeaderTemplate and FooterTemplate don't appear to work
6 participants