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

iOS two finger fix #12

Closed
wants to merge 1 commit into from
Closed

Conversation

jaisont07
Copy link

Added the uitouched to a list and fireevent with the points list to fix the zoom issue in ios

@janusw
Copy link
Member

janusw commented Jul 25, 2024

this is supposed to fix issue #10

@janusw
Copy link
Member

janusw commented Jul 25, 2024

Thanks for the PR, @jaisont07! I just tested it on iOS 17.5 (via the ExampleBrowser in the OxyplotMauiSample app, mostly using the example Annotations -> Tool tips) and made the following observations ...

Basically two-finger zooming seems to work, but it does not always work perfectly and sometimes seems to behave strangely. E.g. the plot jumps occasionally while doing a two-finger pinch gesture (or when putting down the second finger while the first one is already touching). This behavior seems similar to what I saw on my branch mentioned at #10 (comment).

@jaisont07 Can you reproduce these quirks, and do you have an idea what causes them?

@RobertStefancic Could you maybe also try the branch, and tell us how well it works for you? Does your implementation at #10 (comment) work any better, or does it suffer from the same problems?

The GHA builds actually ran through fine without any problems, which is good. I'll try to look through the code changes soon.

@janusw janusw added the OS:iOS label Jul 25, 2024
{
long id = ((IntPtr)touch.Handle).ToInt64();
CheckForBoundaryHop(touch);
if (idToTouchDictionary[id] != null)
{
FireEvent(idToTouchDictionary[id], id, TouchActionType.Moved, touch, true);
//FireEvent with all the points within the touch event
FireEvent(idToTouchDictionary[id], id, TouchActionType.Moved, touch, true, uitouches.IndexOf(touch) == uitouches.Count - 1);
Copy link
Member

Choose a reason for hiding this comment

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

I guess the same multi-point handling that you're using here should also be applied to the FireEvent calls in TouchesBegan and TouchesEnded ?

@janusw
Copy link
Member

janusw commented Sep 15, 2024

@jaisont07 @RobertStefancic Ping! Is there still interest in this feature? If yes, could you reply to my comments, so that we can move forward with this? I think it would be great to add proper zoom support for iOS ...

@jaison-t-john
Copy link

@jaisont07 @RobertStefancic Ping! Is there still interest in this feature? If yes, could you reply to my comments, so that we can move forward with this? I think it would be great to add proper zoom support for iOS ...

@janusw Yes this is required. But I am also able to reproduce the not so smooth zoom issue with all these fixes. Would be great if we could find a solution to this..

@RobertStefancic
Copy link
Contributor

RobertStefancic commented Sep 16, 2024

@janusw @jaisont07 I created a pull-request as well here: #15
This is the code that we used for the public release and we haven't had any complaints so far. The intention is to mimic how the Android touch events are dispatched, because on there it works fine.

Edit: First thing I noticed for the snappy behavior is that the touch events where coming rarely (which is different from Android, where they come often), hence I added the ShouldRecognizeSimultaneously => true.
Afterwards, I had to synchronize how many fingers are pressed at once - that is why I made it to only trigger the actual move event when the user has all the fingers in place. Otherwise, it will again appear snappy because the user touches will trigger at different locations and will move the zoom on the chart.

@janusw
Copy link
Member

janusw commented Sep 19, 2024

I'm closing this PR in favor of #15 (which was just merged).

@janusw janusw closed this Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants