[Android] Fixed CarouselView behaves strangely when swiping vertically in view #31790
Conversation
|
Hey there @@Dhivya-SF4094! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
dotnet-policy-service
bot
added
the
partner/syncfusion
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jsuarezruiz
added
platform/android
area-controls-collectionview
|
/rebase |
github-actions
bot
force-pushed
the
fix-22507
branch
from
0965507 to
28ea8c6
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where CarouselView incorrectly intercepts vertical swipe gestures on Android, preventing proper scrolling of nested scrollable content like CollectionView. The fix improves touch event handling by comparing horizontal and vertical movement to determine whether gestures should be handled by the CarouselView or delegated to child views.
Key changes:
- Modified touch interception logic in MauiCarouselRecyclerView to distinguish between horizontal and vertical swipes
- Added comprehensive UI tests to validate the fix across platforms
- Ensured nested scrollable content can scroll vertically without unwanted CarouselView item transitions
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| MauiCarouselRecyclerView.cs | Enhanced OnInterceptTouchEvent method with directional swipe detection logic |
| Issue22507.cs (HostApp) | Created test page with CarouselView containing nested CollectionView for reproducing the issue |
| Issue22507.cs (Tests) | Added automated UI test to verify vertical scrolling works correctly within CarouselView |
| bool ShouldDelegateToChild(float deltaX, float deltaY) | ||
| { | ||
| float absDeltaX = Math.Abs(deltaX); | ||
| float absDeltaY = Math.Abs(deltaY); | ||
|
|
||
| return IsHorizontal ? absDeltaY > absDeltaX : absDeltaX > absDeltaY; | ||
| } |
There was a problem hiding this comment.
The ShouldDelegateToChild method lacks documentation explaining its logic for determining when to delegate touch events to child views. Consider adding a summary comment explaining that it compares horizontal vs vertical movement magnitudes to determine gesture direction.
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| float absDeltaX = Math.Abs(deltaX); | ||
| float absDeltaY = Math.Abs(deltaY); | ||
|
|
||
| return IsHorizontal ? absDeltaY > absDeltaX : absDeltaX > absDeltaY; |
There was a problem hiding this comment.
Should only evaluate direction after a minimum movement threshold?
There was a problem hiding this comment.
There was a problem hiding this comment.
@jsuarezruiz, I have updated the logic to ensure the scroll direction is only evaluated after the user’s touch movement exceeds a minimum threshold. To achieve this, used ViewConfiguration.Get(context).ScaledTouchSlop, which provides the platform-defined distance in pixels that indicates a meaningful gesture.
| // Wait for the test page to load | ||
| App.WaitForElement("Issue22507Label"); | ||
|
|
||
| App.ScrollDown("Issue22507CollectionView"); |
There was a problem hiding this comment.
Can include another test that start scrolling vertical and then horizontally?
There was a problem hiding this comment.
@jsuarezruiz, Added an additional test that starts with a vertical scroll followed by a horizontal swipe to validate gesture handling
Dhivya-SF4094
force-pushed
the
fix-22507
branch
from
127bc37 to
b27f6a7
Compare
|
|
||
| List<View> _oldViews; | ||
| CarouselViewOnGlobalLayoutListener _carouselViewLayoutListener; | ||
| float _touchSlop; |
There was a problem hiding this comment.
While Android touch events typically fire on UI thread, could be accessed from different threads?
Can use local variables to avoid holding state across events or use volatile.
| _touchSlop = ViewConfiguration.Get(context).ScaledTouchSlop; | ||
| } | ||
|
|
||
| public bool IsSwipeEnabled { get; set; } |
There was a problem hiding this comment.
IsSwipeEnabled property gates the entire gesture logic but isn't documented. Can include a comment?
2 participants
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:
When a CarouselView is used with nested scrollable content (e.g., CollectionView), swipe gestures are not handled correctly. By default, the CarouselView intercepts all touch events, including vertical swipes, even when the intent is to scroll the child view.
Root Cause
By default, MauiCarouselRecyclerView overrides OnInterceptTouchEvent and returns true, meaning it intercepts touch gestures before child views (like CollectionView) can process them.
As a result, Vertical swipes however, are also partially intercepted by the Carousel, even when the intent is to scroll inside the nested CollectionView.
This causes a conflict: vertical gestures intended for the inner CollectionView sometimes trigger an unwanted item transition in the CarouselView.
Description of Change
The touch interception logic was updated to properly distinguish swipe directions. Instead of intercepting all gestures, the implementation now compares horizontal and vertical movement to decide whether the event should be handled by the parent control or delegated to the child.
Validated the behaviour in the following platforms
Issues Fixed:
Fixes #22507
Screenshots
22507_Android_BeforeFix.mov
22507_Android_AfterFix.mov