Fix CollectionView.Header is header is not scrollable in Android platform#31661
Fix CollectionView.Header is header is not scrollable in Android platform#31661SuthiYuvaraj wants to merge 9 commits intodotnet:mainfrom
Conversation
|
Hey there @@SuthiYuvaraj! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/rebase |
04fc615 to
d37685f
Compare
jsuarezruiz
left a comment
There was a problem hiding this comment.
@SuthiYuvaraj Could you rebase to fix the conflict?
d37685f to
9fbff30
Compare
🤖 AI Summary📊 Expand Full Review🔍 Pre-Flight — Context & Validation📝 Review Session — Create problems-report.html ·
|
| File:Line | Concern | Notes |
|---|---|---|
ItemContentView.cs |
IsHeaderOrFooterContent() called on every touch event (performance) |
Traverses logical parent tree on each touch |
ItemContentView.cs |
RequestDisallowInterceptTouchEvent(true) applied to ALL header/footer content on DOWN, not just scrollable |
Could affect non-scrollable headers |
ItemContentView.cs |
DispatchTouchEvent and OnInterceptTouchEvent become public API |
PublicAPI.Unshipped.txt updated, but are these appropriate as public API? |
ItemContentView.cs |
FindParentItemsView() traverses MAUI logical parent chain |
May not work with HeaderTemplate |
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #31661 | Override DispatchTouchEvent/OnInterceptTouchEvent in ItemContentView to call RequestDisallowInterceptTouchEvent for header/footer content | ⏳ PENDING (Gate) | ItemContentView.cs (+71), PublicAPI.Unshipped.txt (+2) |
Original PR |
🚦 Gate — Test Verification
📝 Review Session — Create problems-report.html · 05a9003
Result: ✅ PASSED
Platform: android
Mode: Full Verification
Test Filter: Issue22120
- Tests FAIL without fix ✅
- Tests PASS with fix ✅
The tests correctly catch the bug (CollectionView.Header not scrollable in Android) and confirm the PR's fix resolves it.
🔧 Fix — Analysis & Comparison
📝 Review Session — Create problems-report.html · 05a9003
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #31661 | Override DispatchTouchEvent/OnInterceptTouchEvent in ItemContentView to call RequestDisallowInterceptTouchEvent for header/footer content | ✅ PASS (Gate) | ItemContentView.cs (+71), PublicAPI.Unshipped.txt (+2) |
Original PR |
Exhausted: No (try-fix skipped due to rate limit errors on task agent - 429 Too Many Requests after 3 attempts)
Selected Fix: PR's fix - Gate confirmed tests pass. try-fix exploration blocked by environment rate limits.
📋 Report — Final Recommendation
📝 Review Session — Create problems-report.html · 05a9003
⚠️ Final Recommendation: REQUEST CHANGES
Summary
PR #31661 fixes Android-specific issue where CollectionView.Header containing scrollable content (e.g. ScrollView with ListView) was not scrollable due to RecyclerView intercepting all touch events. The fix uses RequestDisallowInterceptTouchEvent, which is the correct Android mechanism for nested scrolling. Gate verified tests fail without the fix and pass with it. However, several issues need to be addressed before merging.
Root Cause
Android's RecyclerView intercepts all touch events (via onInterceptTouchEvent) to handle its own scrolling, preventing nested scrollable views inside the header from receiving touch events. The fix overrides OnInterceptTouchEvent and DispatchTouchEvent in ItemContentView to call Parent?.RequestDisallowInterceptTouchEvent(true) on DOWN events for header/footer content, temporarily disabling RecyclerView's touch interception.
Fix Quality
The core approach (RequestDisallowInterceptTouchEvent) is the standard Android pattern for nested scrolling and is technically correct. However:
Issues Found
1. Unused Import (code quality)
using AndroidX.RecyclerView.Widget; // Added but never usedRecyclerView is imported but not referenced anywhere in the modified code. Should be removed.
2. HeaderTemplate Edge Case (functional limitation)
IsHeaderOrFooterContent() checks ReferenceEquals(View, structuredItemsView.Header). When HeaderTemplate is used instead of Header, structuredItemsView.Header returns null and the fix won't activate. This is an undocumented limitation that affects a common usage pattern. Should either be documented or handled.
3. Touch Event Logic Review
Setting RequestDisallowInterceptTouchEvent(true) unconditionally on DOWN for ALL header/footer content (not just when the content has scrollable children) is overly broad. A tap on a non-scrollable header button will also prevent RecyclerView from later receiving MOVE events until UP. While functionally correct (UP resets the flag), this is more aggressive than necessary and could potentially cause subtle interaction issues.
4. PR Is In DRAFT State
The PR is marked as draft. This may indicate the author is still working on it.
Test Coverage
Tests are well-structured:
Issue22120.cs(HostApp): Creates CollectionView with ScrollView header containing 15-item ListViewIssue22120.cs(SharedTests): Verifies scroll down and scroll up behavior with element visibility checks- Category:
UITestCategories.CollectionView✓ - Platform:
PlatformAffected.Android✓
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #31661 | Override DispatchTouchEvent/OnInterceptTouchEvent in ItemContentView to use RequestDisallowInterceptTouchEvent | ✅ PASS (Gate) | ItemContentView.cs (+71), PublicAPI.Unshipped.txt (+2) |
Original PR |
try-fix: Skipped due to environment rate limit (429 errors). PR's fix is the only validated candidate.
Changes Requested
- Remove unused import
using AndroidX.RecyclerView.Widget; - Document or fix
HeaderTemplatelimitation - add a comment or extend to handle templated headers - Mark PR as ready for review (remove draft status) when changes are complete
📋 Expand PR Finalization Review
Title: ✅ Good
Current: Fix CollectionView.Header is header is not scrollable in Android platform
Description: ⚠️ Needs Update
- Grammatical error: "is header is" (double "is")
- Missing platform prefix
[Android] - Verbose phrasing
✨ Suggested PR Description
[!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!
Root Cause
On Android, the RecyclerView that backs CollectionView intercepts all touch events to handle its own scrolling. When a scrollable control (e.g., ScrollView containing a ListView) is placed inside the CollectionView.Header or CollectionView.Footer, the RecyclerView consumes the touch events before they can reach the inner scrollable view, making the header/footer content unscrollable.
Description of Change
Added OnInterceptTouchEvent and DispatchTouchEvent overrides to ItemContentView (Android) to pass touch events through to scrollable header/footer content.
When the ItemContentView is rendering a header or footer (detected by comparing the logical view reference against StructuredItemsView.Header / StructuredItemsView.Footer):
OnInterceptTouchEvent(Down): CallsParent.RequestDisallowInterceptTouchEvent(true)to prevent the parentRecyclerViewfrom stealing touch events, then returnsfalseso child views can handle the gesture.DispatchTouchEvent(Up/Cancel): CallsParent.RequestDisallowInterceptTouchEvent(false)to restore normalRecyclerViewtouch interception after the gesture ends.
This is the standard Android nested scrolling pattern. Regular CollectionView items are unaffected — the overrides are no-ops when IsHeaderOrFooterContent() returns false.
Note: This fix applies when Header/Footer is set directly as a View. If HeaderTemplate/FooterTemplate is used (DataTemplate), the fix does not apply because the ReferenceEquals check compares against the template object, not the inflated view.
Issues Fixed
Fixes #22120
Platforms Tested
- Android
- Windows
- iOS
- Mac
Code Review: ⚠️ Issues Found
Code Review – PR #31661
🔴 Critical Issues
Unused using Directives
File: src/Controls/src/Core/Handlers/Items/Android/ItemContentView.cs
Problem: The PR adds two using directives that are not referenced anywhere in the new or existing code:
using AndroidX.Core.Widget; // unused
using AndroidX.RecyclerView.Widget; // unusedNeither NestedScrollView (from AndroidX.Core.Widget) nor RecyclerView (from AndroidX.RecyclerView.Widget) is referenced in the new logic. The IsHeaderOrFooterContent / FindParentItemsView methods work purely with MAUI logical types (IView, StructuredItemsView).
Recommendation: Remove both unused using directives before merge.
🟡 Suggestions
1. Fix Does Not Cover HeaderTemplate / FooterTemplate
File: src/Controls/src/Core/Handlers/Items/Android/ItemContentView.cs — IsHeaderOrFooterContent()
The ReferenceEquals check compares View (the MAUI virtual view) to structuredItemsView.Header:
return ReferenceEquals(View, structuredItemsView.Header) ||
ReferenceEquals(View, structuredItemsView.Footer);When Header / Footer is set directly as a View, this works correctly. However, when HeaderTemplate / FooterTemplate is used (a DataTemplate), structuredItemsView.Header holds the DataTemplate object, not the inflated view — so the check will always return false and the fix won't apply.
Recommendation: Either document this known limitation in a code comment and in the PR description, or extend the fix to handle the template case. At minimum, a // Note: Does not handle HeaderTemplate/FooterTemplate comment would prevent future confusion.
2. Extra Blank Line in DispatchTouchEvent
File: src/Controls/src/Core/Handlers/Items/Android/ItemContentView.cs
There is an extra blank line inside the outer if block:
public override bool DispatchTouchEvent(MotionEvent e)
{
if (IsHeaderOrFooterContent())
{
// ← extra blank line here
if (e.Action == MotionEventActions.Up || e.Action == MotionEventActions.Cancel)
{Recommendation: Remove the spurious blank line for consistency with the rest of the file's style.
✅ Looks Good
- Core fix logic is correct:
RequestDisallowInterceptTouchEvent(true)onMotionEventActions.Down/falseonUp/Cancelis the standard Android pattern for nested scroll containers and is widely used in the Android ecosystem. - Regular items unaffected:
IsHeaderOrFooterContent()returnsfalsefor non-header/footerItemContentViewinstances, so existing RecyclerView item behavior is unchanged. PublicAPI.Unshipped.txtupdated: Both new public overrides (DispatchTouchEvent,OnInterceptTouchEvent) are correctly added tonet-android/PublicAPI.Unshipped.txt.- UI test included:
Issue22120.cstests scroll-up and scroll-down on aScrollViewinsideCollectionView.Headerwith properAutomationIdattributes. The test uses[Category(UITestCategories.CollectionView)](single category, correct). - Logical tree traversal:
FindParentItemsView()traverses the MAUI logical parent chain (not the Android view hierarchy), which is the correct approach for finding the owningStructuredItemsView.
|
|
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 Description:
CollectionView.Header content is not scrollable on Android platform when it contains scrollable content like ScrollView with ListView.
Root Cause:
When the scrollable content is loaded inside the Header of the collectionview , touch is not passed to the internal scrollable control , which is handled by the TemplatedContentView in CollectionView.
Fix Description:
I have included the Touch Events in ItemContentView to handle the touch when the content is scrollable inside the header/ footer , Remaining cases it will not affects the previous behavior
Issues Fixed
Fixes #22120
Tested the behaviour in the following platforms
Output Screenshot
Before.mov
After.mov