search

software-mansion / react-native-gesture-handler

Declarative API exposing platform native touch and gesture system to React Native.
https://docs.swmansion.com/react-native-gesture-handler/
MIT License
6.13k stars 982 forks source link

Allow Android Rotation Gesture to continue after second Pointer Lift and Land #3057

Closed DmitriyGalyanov closed 1 month ago

DmitriyGalyanov commented 3 months ago

Description

On iOS Rotation Gesture is not getting finished/cancelled when one of two Pointers lifts up, allowing the Gesture to continue when a new second Pointer arrives

On Android this behaviour differs – Rotation Gesture finishes whenever one of two Pointers lifts up, requiring to land 2 completely new Pointers in order to utilise Rotation Gesture again

I understand that this may be closer to default Android Gesture Recogniser Behaviour, but find it unexpected and inconvenient

Thus in this PR I’m adding Ability to tell Rotation Gesture Builder to enable iOS-like Behaviour, which allows Gesture Pause/Remain when second Pointer lifts/lands

Additionally, I added some local toggleable Logs, which I find useful for Debugging, but I’m not sure if it aligns with this Repo Style Guide (or could noticeably reduce Performance) – please notify me, if it’s prohibited

I think this Change may be usable for Gesture Handler Users, thus trying to push it to the Gesture Handler (not only my own Fork) and looking forward to any Feedback regarding both Code – Implementation/Naming and/or local Style Guide – and Docs, which I might’ve misaligned with

Attached Gifs display what I mean (first one shows current Behaviour, second one – with new Modifier)

Test plan

  1. Use RotationGesture without new Modifier (secondPointerLiftFinishesGesture(value: boolean)) and see if it works as expected (as before – Gesture finishes when one of two key Pointers is lifted)
  2. Use RotationGesture with new Modifier (secondPointerLiftFinishesGesture(value: boolean)) and see if it works as expected (Gesture doesn't finish when second Pointer is lifted and continues when second Pointer is landed again)
DmitriyGalyanov commented 2 months ago

@wcandillon

Guys, does somebody see this?

Could you please tell me if it's not needed / aligned with your opinion or will be overseen later

j-piasecki commented 2 months ago

Hi! Sorry, I didn't have much time for reviews lately. I should be able to spend more time on reviews soon(-ish, in a week, maybe two).

DmitriyGalyanov commented 1 month ago

@j-piasecki

Sorry for the delay on this 😅, there's been a lot going on recently.

Thank you for the reply! Hopefully will update PR accordingly to your review this week

DmitriyGalyanov commented 1 month ago

I'm not sure whether it's closer, as a native rotation recognizer doesn't exist on Android AFAIK. Moreover, I'd lean towards saying that it should've been working like this since the beginning and classify that as a bugfix. Instead of making it opt-in, I'd simply make it a default behavior. I'm not sure whether we even need to keep the old one behind a flag, current behavior seems very counterintuitive to me and I don't really see anyone relying on it.

Glad to see that! And, since both of us think it's closer to a bugfix, I've removed flag as redundant

DmitriyGalyanov commented 2 weeks ago

@j-piasecki

Hello!

Could you, please, tell me when (at least approximately) would it be released?

j-piasecki commented 2 weeks ago

@DmitriyGalyanov Most likely next week.