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

Android: Pinch gesture not working properly in Carousel #18569

Open
unombun opened this issue Nov 7, 2023 · 16 comments
Open

Android: Pinch gesture not working properly in Carousel #18569

unombun opened this issue Nov 7, 2023 · 16 comments
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView migration-compatibility Xamarin.Forms to .NET MAUI Migration, Upgrade Assistant, Try-Convert p/2 Work that is important, but is currently not scheduled for release partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with platform/android 🤖 s/needs-attention Issue has more information and needs another look t/bug Something isn't working
Milestone

Comments

@unombun
Copy link

unombun commented Nov 7, 2023

Description

We want to have a carousel with images that can be zoomed. We want to swipe so the carousel to change the image, and with pinch and pan gesture we want to zoom in or out or move the zoomed image.

On Android if you have an ContentView with PinchGestureRecognizer added from the code, the pinch event is not triggered at all. In order to trigger the event you have to manually add the PinchGestureRecognizer from the XAML code directly to the Image you want to zoom and call your pinch event from the xaml.cs (as exemplified in the repro). Even though we managed to trigger the pinch event, the problem is that only on Android the event is not triggered correctly. The pinch values are sent chaotic and the image is zoomed very strange. Apart that the Pinch event can be triggered with just one finger which should not be possible.

This issue was found in .net7 we tested both on simulator and real device in debug and release mode.

We found out that if you set the PinchGestureRecognizer in the first Grid of the page, the event triggers correctly and smoothly, this should work in some cases where you have just one picture in the page but in our case we have a carousel-view with images and we want to zoom each image and block the scrolling while zooming.
In order to be able to trigger the events on the grid, the carousel-view needs to be set input transparent.
But the problem is if the carousel is transparent, we lose the functionality behind with the scrolling.

We tried different scenarios where we manually set the PinchGestureRecognizer directly to the carousell-view or the ZoomContentView but we encountered the same behaviour.

On iOS it works perfectly.

Steps to Reproduce

  1. Create new Project
  2. Add a carousell-view
  3. Implement a ZoomContentView with mathematical calculations behind for the pinch event
  4. In carousell-view data template add the ZoomContentView where we set the gestures
  5. Add an image in the content of the ZoomContentView
  6. Add PinchGestureRecognizer to the image control
  7. Call the implemented method from ZoomContentView when the PinchGestureRecognizer OnPinchUpdated is triggered.
  8. The pinch is not working smoothly and it can be triggered with one finger

Link to public reproduction project repository

https://github.com/flesarradu/AndroidPinchGestureIssue

Version with bug

7.0.96

Is this a regression from previous behavior?

Not sure, did not test other versions

Last version that worked well

Unknown/Other

Affected platforms

Android

Affected platform versions

No response

Did you find any workaround?

No

Relevant log output

No response

@unombun unombun added the t/bug Something isn't working label Nov 7, 2023
@mikeparker104 mikeparker104 added the partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with label Nov 7, 2023
@PureWeen PureWeen assigned mikeparker104 and BenBtg and unassigned mikeparker104 Nov 7, 2023
@PureWeen PureWeen added area-controls-collectionview CollectionView, CarouselView, IndicatorView platform/android 🤖 labels Nov 7, 2023
@PureWeen PureWeen added this to the .NET 8 SR1 milestone Nov 7, 2023
@BenBtg
Copy link
Contributor

BenBtg commented Nov 15, 2023

Having investigated this issue further I have found that the Pinch Zoom on Android using the above technique and as documented in Microsoft learn has the same issue with or without being contained in a Carousel view.
It is not clear yet if there is an issue with the gesture recogniser or the math used to do the scaling.
However using a dedicated Android SimpleOnScaleGestureListener in an Native DotNet Android app does work as expected. Suggest a potential workaround would be to use this via a custom Handler.

@BenBtg
Copy link
Contributor

BenBtg commented Dec 4, 2023

Upon further investigation it does appear that the CarouselView on Android does block pinch gestures when IsSwipeEnabled is true. When false Pinch Gestures work as expected allowing pinch to zoom.
As a workaround I have found a way to capture 2 finger touch events using th OnTouchEvent handler along with the DownTime property on the MotionEvent to distinguish betweeg single and two finger touch down events.
This information can then be used to disable the IsSwipEnabled when two fingers touch events occurs and enable during single finger touch events.
Ideally this logic should be baked into the MAUI Carousel so that the behaviour is consistent with other platforms.

@Redth Redth modified the milestones: .NET 8 SR2, Backlog Dec 5, 2023
@Redth Redth removed this from MAUI SDK Ongoing Dec 5, 2023
@ghost
Copy link

ghost commented Dec 5, 2023

We've added this issue to our backlog, and we will work to address it as time and resources allow. If you have any additional information or questions about this issue, please leave a comment. For additional info about issue management, please read our Triage Process.

@Redth
Copy link
Member

Redth commented Dec 5, 2023

Hey @BenBtg would you be able to share a bit of code with your workaround here?

We are moving this one to the Backlog again for future consideration as we're working on improving our processes and transparency around planning work for servicing.

@Redth Redth added the s/needs-info Issue needs more info from the author label Dec 5, 2023
@ghost
Copy link

ghost commented Dec 5, 2023

Hi @unombun. We have added the "s/needs-info" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@samhouts samhouts added the p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint label Dec 7, 2023
@ghost ghost added the s/no-recent-activity Issue has had no recent activity label Dec 12, 2023
@ghost
Copy link

ghost commented Dec 12, 2023

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

@ghost ghost closed this as completed Dec 15, 2023
@ramonB1996
Copy link

@BenBtg
Could you update your sample for the working solution by chance? I am facing the same issue.

@ghost ghost removed the s/no-recent-activity Issue has had no recent activity label Dec 19, 2023
@BenBtg
Copy link
Contributor

BenBtg commented Jan 8, 2024

Sorry for the delay. Just back from leave. The suggested workaround wasn't sufficient. The eventual approach taken was to navigate to a dedicated zoom page outside of the carousel.

@samhouts samhouts reopened this Jan 8, 2024
@ghost ghost added the s/no-recent-activity Issue has had no recent activity label Jan 12, 2024
@ghost
Copy link

ghost commented Jan 12, 2024

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

@BenBtg
Copy link
Contributor

BenBtg commented Jan 15, 2024

This issue needs to remain open. Current proposed workaround is not sufficient.

@ghost ghost added s/needs-attention Issue has more information and needs another look and removed s/needs-info Issue needs more info from the author s/no-recent-activity Issue has had no recent activity labels Jan 15, 2024
@jsuarezruiz
Copy link
Contributor

Progress here https://github.com/dotnet/maui/tree/fix-18569

@jsuarezruiz
Copy link
Contributor

The same sample running on Xamarin.Forms:
Issue18569.zip

Sometimes the pinch to zoom not works (for the same .NET MAUI issues, touch conflicts), but once works is working fine, panning works, etc. The behavior is better in Xamarin.Forms than in .NET MAUI.

@BenBtg
Copy link
Contributor

BenBtg commented Jan 19, 2024

Does that mean this issue should be treated as a regression?

@samhouts samhouts added the migration-compatibility Xamarin.Forms to .NET MAUI Migration, Upgrade Assistant, Try-Convert label Jan 19, 2024
@jsuarezruiz
Copy link
Contributor

Does that mean this issue should be treated as a regression?

Not in .NET MAUI itself, but is a different behavior from Xamarin.Forms.

@BenBtg BenBtg removed their assignment Feb 14, 2024
@BenBtg
Copy link
Contributor

BenBtg commented Feb 14, 2024

@BenBtg Could you update your sample for the working solution by chance? I am facing the same issue.
@ramonB1996
FYI here is a branch with a possible workaround I found using Platform Behaviours to handles zooming and disabling swipe on the caroursel. It's a bit convoluted but I think it's probably the best option in NET7.
https://github.com/BenBtg/AndroidPinchGestureIssue/tree/SwipeZoom

@jsuarezruiz jsuarezruiz self-assigned this Feb 15, 2024
@jsuarezruiz jsuarezruiz moved this from Todo to In Progress in MAUI SDK Ongoing Feb 15, 2024
@PureWeen PureWeen added p/2 Work that is important, but is currently not scheduled for release and removed p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint labels Jul 2, 2024
@jsuarezruiz jsuarezruiz removed their assignment Nov 28, 2024
@Odaronil
Copy link

The issue with Pinch and PanUpdated (CarouselView) on Android still persists.
MAUI: .NET 9.0.21 SR2.1
Visual Studio: 17.12.3
On iOS, everything works stably.

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 migration-compatibility Xamarin.Forms to .NET MAUI Migration, Upgrade Assistant, Try-Convert p/2 Work that is important, but is currently not scheduled for release partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with platform/android 🤖 s/needs-attention Issue has more information and needs another look t/bug Something isn't working
Projects
Status: In Progress
Development

No branches or pull requests

9 participants