-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix CollectionView header/footer not removed when set to null on Android with empty ItemsSource #32741
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
base: main
Are you sure you want to change the base?
Conversation
…moval fix Co-authored-by: kubaflo <[email protected]>
|
@ -0,0 +1,278 @@ PR Review: Fix CollectionView Header/Footer Removal on Android (#32741)SummaryThis PR successfully fixes a bug where CollectionView header and footer views remain visible on Android when set to Code ReviewChanges OverviewPlatforms affected: Android only (but includes UI tests for iOS, Android, Windows for cross-platform validation) Files modified:
Implementation AnalysisChange 1: Extended Property MonitoringFile: Before: if (property.Is(Microsoft.Maui.Controls.StructuredItemsView.HeaderProperty))
{
UpdateHasHeader();
NotifyDataSetChanged();
}
else if (property.Is(Microsoft.Maui.Controls.StructuredItemsView.FooterProperty))
{
UpdateHasFooter();
NotifyDataSetChanged();
}After: if (property.Is(Microsoft.Maui.Controls.StructuredItemsView.HeaderProperty) ||
property.Is(Microsoft.Maui.Controls.StructuredItemsView.HeaderTemplateProperty))
{
UpdateHasHeader();
NotifyDataSetChanged();
}
else if (property.Is(Microsoft.Maui.Controls.StructuredItemsView.FooterProperty) ||
property.Is(Microsoft.Maui.Controls.StructuredItemsView.FooterTemplateProperty))
{
UpdateHasFooter();
NotifyDataSetChanged();
}Why this works: Monitoring both Correctness: ✅ This change correctly addresses scenarios where headers/footers are set via templates. Change 2: Adapter Detach/Reattach for Empty ViewFile: New logic added: else if (showEmptyView && currentAdapter == _emptyViewAdapter)
{
if (ShouldUpdateEmptyView())
{
// Header/footer properties changed - detach and reattach adapter to force RecyclerView to recalculate positions.
SetAdapter(null);
SwapAdapter(_emptyViewAdapter, true);
UpdateEmptyView();
}
}
bool ShouldUpdateEmptyView()
{
if (ItemsView is StructuredItemsView structuredItemsView)
{
if (_emptyViewAdapter.Header != structuredItemsView.Header ||
_emptyViewAdapter.HeaderTemplate != structuredItemsView.HeaderTemplate ||
_emptyViewAdapter.Footer != structuredItemsView.Footer ||
_emptyViewAdapter.FooterTemplate != structuredItemsView.FooterTemplate ||
_emptyViewAdapter.EmptyView != ItemsView.EmptyView ||
_emptyViewAdapter.EmptyViewTemplate != ItemsView.EmptyViewTemplate)
{
return true;
}
}
return false;
}Why this works:
Root cause identified: The issue occurs specifically when EmptyView is active because the empty view adapter doesn't track header/footer changes and RecyclerView doesn't recalculate cached layouts without explicit detach/reattach. Performance consideration: The detach/reattach cycle only happens when:
This minimizes performance impact by avoiding unnecessary adapter swaps. Correctness: ✅ The fix correctly addresses the root cause without introducing unnecessary overhead. Test CoverageThe PR includes comprehensive UI tests: HostApp Test Page (
NUnit Tests (
Platform coverage:
Note: The Windows exclusion is acceptable and properly documented with a linked issue. TestingI tested this PR using the Sandbox app on Android to validate the fix in practice. Test SetupTest Scenario: CollectionView with empty ItemsSource, EmptyView, Header, and Footer. Test removal of header and footer by setting them to Environment: Android Emulator (emulator-5554) Test ResultsTest 1: WITHOUT PR (Baseline - Main Branch) Initial state:
After tapping "Remove Header":
After tapping "Remove Footer":
Test 2: WITH PR Changes Initial state:
After tapping "Remove Header":
After tapping "Remove Footer":
Visual EvidenceScreenshots captured:
The visual comparison clearly shows the PR resolves the issue. Edge Cases Tested
Additional Edge Cases to Consider (Not Tested)The following edge cases were not tested but should be considered:
Recommendation: These edge cases should ideally be tested, but the core fix logic appears sound for handling them. Issues Found🟡 Missing PR Description Template NoteThe PR description is missing the required note about testing PR builds: > [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you!Impact: Low - This is a documentation issue, not a code issue. Users won't be able to test PR artifacts. Recommendation: Add the note to the PR description. 🟢 Code Quality NotesPositive observations:
Minor observation:
Breaking Changes✅ No breaking changes - This is a bug fix that restores expected behavior. Documentation✅ XML documentation: No new public APIs, so no new documentation needed. ✅ Code comments: The PR includes a helpful inline comment explaining the adapter detach/reattach: // Header/footer properties changed - detach and reattach adapter to force RecyclerView to recalculate the positions.Related IssuesFixes: #31911 (CollectionView Header/Footer not removed when set to null on Android) Related: #32740 (Windows issue causing test exclusion - separate issue, properly documented) Recommendation✅ APPROVE with minor suggestion This PR successfully fixes the reported Android bug. The implementation is sound, well-tested, and follows MAUI coding patterns. Before merge:
Post-merge recommendations:
Summary of Changes✅ Code: Correctly implements adapter detach/reattach for empty view scenario The fix is ready for merge with the minor documentation improvement. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a bug where CollectionView header/footer views remain visible when set to null at runtime on Android, particularly when an EmptyView is active. The fix adds logic to detect when header/footer properties change while the empty view is displayed and forces RecyclerView to recalculate positions through an adapter detach/reattach cycle.
Key Changes
- Added detection logic to check if header/footer properties have changed while EmptyView is showing
- Implemented adapter detach/reattach to force RecyclerView position recalculation
- Added HeaderTemplate and FooterTemplate property change handling in StructuredItemsViewAdapter
- Added comprehensive UI tests with screenshots for both header and footer removal scenarios
Reviewed Changes
Copilot reviewed 4 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| MauiRecyclerView.cs | Added ShouldUpdateEmptyView() method and conditional logic to force adapter refresh when header/footer changes in empty view state |
| StructuredItemsViewAdapter.cs | Extended property change handlers to include HeaderTemplate and FooterTemplate changes |
| Issue31911.cs (HostApp) | Created test page with CollectionView demonstrating header/footer removal with empty ItemsSource |
| Issue31911.cs (Tests) | Added NUnit tests to verify header and footer removal behavior |
| Screenshot files | Added baseline screenshots for visual verification on iOS and Android |
| @@ -0,0 +1,34 @@ | |||
| #if TEST_FAILS_ON_WINDOWS // https://github.com/dotnet/maui/issues/32740 | |||
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests are disabled on Windows due to issue #32740. According to the PR description, the fix was validated on Windows. Consider investigating why the tests cannot run on Windows or document the specific limitation that prevents Windows test execution.
| bool ShouldUpdateEmptyView() | ||
| { | ||
| if (ItemsView is StructuredItemsView structuredItemsView) | ||
| { | ||
| if (_emptyViewAdapter.Header != structuredItemsView.Header || | ||
| _emptyViewAdapter.HeaderTemplate != structuredItemsView.HeaderTemplate || | ||
| _emptyViewAdapter.Footer != structuredItemsView.Footer || | ||
| _emptyViewAdapter.FooterTemplate != structuredItemsView.FooterTemplate || | ||
| _emptyViewAdapter.EmptyView != ItemsView.EmptyView || | ||
| _emptyViewAdapter.EmptyViewTemplate != ItemsView.EmptyViewTemplate) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method compares object references directly without null-safe equality checks. While this may work for detecting property changes from non-null to null (the main fix scenario), consider using a more robust equality comparison pattern for consistency and maintainability.
Co-authored-by: kubaflo <[email protected]>
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Issue Details
CollectionView Header/Footer views remain visible when set to null at runtime, particularly when an EmptyView is active.
Root Cause
When header/footer properties are set to null in CollectionView with an empty ItemsSource, Android's RecyclerView caches layout state and doesn't recalculate positions, causing removed header/footer views to remain visible on screen.
Description of Change
Added adapter detach/reattach cycle to force RecyclerView to recalculate positions when header/footer properties change in the empty view scenario.
Validated the behavior in the following platforms
Issues Fixed
Fixes #31911
Output ScreenShot
31911-BeforeFix.mov
31911-AfterFix.mov