-
-
Notifications
You must be signed in to change notification settings - Fork 980
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
Add numberOfPointers
to LongPress
#3043
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor comment, looks great overall 👍
android/src/main/java/com/swmansion/gesturehandler/core/LongPressGestureHandler.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/gesturehandler/core/LongPressGestureHandler.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/gesturehandler/core/LongPressGestureHandler.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/gesturehandler/core/LongPressGestureHandler.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/gesturehandler/core/LongPressGestureHandler.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/gesturehandler/core/LongPressGestureHandler.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/gesturehandler/core/LongPressGestureHandler.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/gesturehandler/core/LongPressGestureHandler.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/gesturehandler/core/LongPressGestureHandler.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LongPress
permanently stops working after being cancelled by placing additional finger down while activated.
Tested on Android
with numberOfPointers(2)
, minDuration(2000)
and maxDistance(99999999)
Screen.Recording.2024-08-19.at.12.47.29.mov
That's strange, I've just tested it with parameters that you've provided and eI can't see this problem. What I do see though, is that we get Are you sure that it permanently stops working? |
Tested it again, it stops working for the very next to-be activation, but works again after that. Here's an updated video:Video Screen.Recording.2024-08-19.at.14.03.28.movAnd here's the full repro:Codeimport React from 'react';
import { StyleSheet, View } from 'react-native';
import {
Gesture,
GestureDetector,
GestureHandlerRootView,
} from 'react-native-gesture-handler';
export default function EmptyExample() {
const outer = Gesture.LongPress()
.numberOfPointers(2)
.minDuration(1000)
.maxDistance(99999999)
// .hitSlop(-100)
// .onTouchesUp(() => console.log('UP'))
.onTouchesDown(() => console.log('DOWN'))
.onStart(() => console.log('START'))
.onBegin(() => console.log('BEGIN'))
.onEnd(() => console.log('END'))
.onFinalize(() => console.log('FINALIZE'));
return (
<GestureHandlerRootView>
<View style={styles.container}>
<GestureDetector gesture={outer}>
<View style={styles.nested}>
{/* <View style={styles.slopper} /> */}
</View>
</GestureDetector>
</View>
</GestureHandlerRootView>
);
}
const styles = StyleSheet.create({
container: {
flex: 1,
justifyContent: 'center',
alignItems: 'center',
backgroundColor: '#F5FCFF',
},
text: {
width: 300,
height: 200,
backgroundColor: 'tomato',
},
nested: {
width: 300,
height: 300,
backgroundColor: 'plum',
},
slopper: {
width: 100,
height: 100,
margin: 'auto',
backgroundColor: 'tomato',
},
}); |
Another possibly related issue - given the following code: Codeimport React from 'react';
import { StyleSheet, View } from 'react-native';
import { Gesture, GestureDetector } from 'react-native-gesture-handler';
export default function EmptyExample() {
const outer = Gesture.LongPress()
.numberOfPointers(2)
.minDuration(1000)
.maxDistance(99999999)
.onTouchesUp(() => console.log('UP'))
.onTouchesDown(() => console.log('DOWN'))
.onStart(() => console.log('START'))
.onBegin(() => console.log('BEGIN'))
.onEnd(() => console.log('END'))
.onFinalize(() => console.log('FINALIZE'));
return (
<View style={styles.container}>
<GestureDetector gesture={outer}>
<View style={styles.nested} />
</GestureDetector>
</View>
);
}
const styles = StyleSheet.create({
container: {
flex: 1,
justifyContent: 'center',
alignItems: 'center',
backgroundColor: '#F5FCFF',
},
text: {
width: 300,
height: 200,
backgroundColor: 'tomato',
},
nested: {
width: 300,
height: 300,
backgroundColor: 'plum',
},
}); When gesture is cancelled by dragging the finger out-of-bounds, the next 2 attempts to activate it fail, before the 3rd one is successful. Issue only occurs on Android To reproduce:
Reproduction video: Screen.Recording.2024-08-19.at.14.14.09.mov |
Everything works fine in my case. @j-piasecki, could you check it if you can reproduce it? |
In my case only the first attempt fails, second one activates as expected. I'll check that. |
Okay, as I thought, To avoid much changes in general logic, I've set Let me know if that satisfies you, @j-piasecki @latekvo. Changes were made in this commit. |
I confirm all previously failing test cases now behave correctly on my end 👍 |
Wouldn't |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea if this looks better. Feel free to ignore it if you want.
android/src/main/java/com/swmansion/gesturehandler/core/LongPressGestureHandler.kt
Show resolved
Hide resolved
android/src/main/java/com/swmansion/gesturehandler/core/LongPressGestureHandler.kt
Show resolved
Hide resolved
You're right. Changed in d61ed7e. |
Description
LongPressGestureHandler
doesn't support multiple pointers. Since default value ofnumberOfTouchesRequired
on iOS is1
, handler gets cancelled if you put more than 1 pointer on it.This PR adds
numberOfPointers
modifier forLongPress
so that now users can specify how many pointers are needed forLongPress
to activate.Web
On
web
you could see similar problem, which was probably caused by callingcheckDistanceFail
insideonPointerMove
- if you added another pointer, start position was still at the first pointer position, not in the midpoint. Therefore it was easy to accidentally cancel handler.macOS
I'm not sure if this feature is required on
macOS
. In many places we just assume that handlers use only1
pointer (or2
in case ofpinch
/rotation
). I've added required property to recognizer, but I have not tested that 😅Test plan
Tested on slightly modified LongPress example from macOS:
Test code