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

Tap gesture with multiple pointers fail to start on 2.15.0 #2754

Closed
levibuzolic opened this issue Feb 9, 2024 · 7 comments · Fixed by #2755
Closed

Tap gesture with multiple pointers fail to start on 2.15.0 #2754

levibuzolic opened this issue Feb 9, 2024 · 7 comments · Fixed by #2755
Labels
Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided

Comments

@levibuzolic
Copy link

levibuzolic commented Feb 9, 2024

Description

Upgrading from 2.14.1 to 2.15.0 has stopped tap gestures with minPointers(2) from firing.

The tap gesture is setup as following:

const tap = Gesture.Tap()
  .minPointers(2)
  .runOnJS(true)
  .onBegin((event) => {
    console.log('onBegin', JSON.stringify({ event }));
  })
  .onStart((event) => {
    console.log('onStart', JSON.stringify({ event }));
  })
  .onEnd((event, success) => {
    console.log('onEnd', { event, success });
  });
  • On 2.15.0 only onBegin fires, no other event is triggered.
  • On 2.14.1 all 3 events are triggered as expected.
  • Switching to minPointers(1), the gesture works as expected

Minimal reproduction

https://github.com/levibuzolic/react-native-gesture-handler/blob/main/example/src/new_api/twoPointerTap/index.tsx

import React, { useState } from 'react';
import { Text, StyleSheet, View, ScrollView, Button } from 'react-native';
import { GestureDetector, Gesture } from 'react-native-gesture-handler';

export default function Home() {
  const [log, setLog] = useState<string[]>([]);

  const tap = Gesture.Tap()
    .minPointers(1)
    .runOnJS(true)
    .onBegin((event) => {
      setLog((prev) => [`onBegin: ${JSON.stringify({ event })}`, ...prev]);
    })
    .onStart((event) => {
      setLog((prev) => [`onStart: ${JSON.stringify({ event })}`, ...prev]);
    })
    .onEnd((event, success) => {
      setLog((prev) => [
        `onEnd: ${JSON.stringify({ event, success })}`,
        ...prev,
      ]);
    });

  return (
    <View style={styles.container}>
      <GestureDetector gesture={tap}>
        <View style={styles.container}>
          <ScrollView>
            <Button title="Reset" onPress={() => setLog([])} />
            {log.map((line, i) => (
              <Text key={i} style={[styles.log, i === 0 && styles.logNew]}>
                {line}
              </Text>
            ))}
          </ScrollView>
        </View>
      </GestureDetector>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    width: '100%',
    height: '100%',
    backgroundColor: 'white',
  },
  log: {
    fontSize: 12,
    color: 'grey',
    padding: 5,
  },
  logNew: {
    backgroundColor: '#e6ffec',
    color: 'black',
  },
});

Video

Kapture.2024-02-09.at.17.07.38.mp4

Steps to reproduce

  1. Set up a Gesture.Tap() with .minPointers(2)
  2. Perform a 2 pointer tap
  3. onStart and onEnd are not fired

Snack or a link to a repository

https://github.com/software-mansion/react-native-gesture-handler

See the TwoPointerTap example.

Gesture Handler version

2.15.0

React Native version

0.72.0

Platforms

iOS

JavaScript runtime

Hermes

Workflow

React Native (without Expo)

Architecture

Paper (Old Architecture)

Build type

Debug mode

Device

iOS simulator

Device model

No response

Acknowledgements

Yes

@github-actions github-actions bot added Missing repro Platform: iOS This issue is specific to iOS labels Feb 9, 2024
@levibuzolic
Copy link
Author

I'm in the process of looking through the diff between 2.14.1 and 2.15.0 to see if I can spot anything related.

@levibuzolic levibuzolic changed the title Tap gesture with multiple pointers fails to start on 2.15.0 Tap gesture with multiple pointers fail to start on 2.15.0 Feb 9, 2024
@github-actions github-actions bot added Repro provided A reproduction with a snack or repo is provided and removed Missing repro labels Feb 9, 2024
@levibuzolic
Copy link
Author

Adding onFinalize to the events, I can see that's getting called with success: false.

const tap = Gesture.Tap()
  .minPointers(2)
  .runOnJS(true)
  .onBegin((event) => {
    setLog((prev) => [`onBegin: ${JSON.stringify({ event })}`, ...prev]);
  })
  .onStart((event) => {
    setLog((prev) => [`onStart: ${JSON.stringify({ event })}`, ...prev]);
  })
  .onEnd((event, success) => {
    setLog((prev) => [
      `onEnd: ${JSON.stringify({ event, success })}`,
      ...prev,
    ]);
  })
  .onFinalize((event, success) => {
    setLog((prev) => [
      `onFinalize: ${JSON.stringify({ event, success })}`,
      ...prev,
    ]);
  });

@levibuzolic
Copy link
Author

Looks like theres a bit of a diff in the tap handler for macOS support, is suepct it's something there but haven't found the smoking gun yet. 👀

@m-bert
Copy link
Contributor

m-bert commented Feb 9, 2024

Hi @levibuzolic! I will take a look at it and get back to you as soon as I find something!

@m-bert
Copy link
Contributor

m-bert commented Feb 9, 2024

Okay, I've found out what causes this problem. Before macOS support NSSet touches contained UITouch elements and calling count resulted in getting actual amount of pointers. Now this set contains only one element and the real number of touches is hidden inside this element.

We will prepare a PR to fix that soon!

@levibuzolic
Copy link
Author

@m-bert awesome, thanks for looking into this! 🙇‍♂️

@m-bert
Copy link
Contributor

m-bert commented Feb 9, 2024

I've prepared PR that should fix this problem and I'd be happy if you could check if it helps!

m-bert added a commit that referenced this issue Feb 12, 2024
## Description

In 2.15.0 we released partial support for macOS. In `Tap` and `Rotation`, instead of passing original `touches` parameter  to keep iOS behavior, we changed that to `NSSet` with `event` object.  This means that instead of having multiple `UITouch` objects in that set, we only had one object which cumulates all touches. Because we use `[touches count]` to find number of touches, change mentioned above resulted in obtaining wrong values.

Fixes #2754

## Test plan
<details>
<summary> Tested on code from #2754 </summary>

```jsx
import React, { useState } from 'react';
import { Text, StyleSheet, View, ScrollView, Button } from 'react-native';
import { GestureDetector, Gesture } from 'react-native-gesture-handler';

export default function Home() {
  const [log, setLog] = useState<string[]>([]);

  const tap = Gesture.Tap()
    .minPointers(2)
    .runOnJS(true)
    .onBegin((event) => {
      console.log('onBegin', JSON.stringify({ event }));
      setLog((prev) => [`onBegin: ${JSON.stringify({ event })}`, ...prev]);
    })
    .onStart((event) => {
      console.log('onStart', JSON.stringify({ event }));
      setLog((prev) => [`onStart: ${JSON.stringify({ event })}`, ...prev]);
    })
    .onFinalize((event, success) => {
      console.log('onFinalize', { event, success });
      setLog((prev) => [
        `onEnd: ${JSON.stringify({ event })}; success:${success}`,
        ...prev,
      ]);
    });

  return (
    <View style={styles.container}>
      <Button title="Reset" onPress={() => setLog([])} />
      <GestureDetector gesture={tap}>
        <View style={styles.container}>
          {/* <ScrollView> */}
          {log.map((line, i) => (
            <Text key={i} style={[styles.log, i === 0 && styles.logNew]}>
              {line}
            </Text>
          ))}
          {/* </ScrollView> */}
        </View>
      </GestureDetector>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    width: '100%',
    height: '100%',
    backgroundColor: 'white',
  },
  log: {
    fontSize: 12,
    color: 'grey',
    padding: 5,
  },
  logNew: {
    backgroundColor: '#e6ffec',
    color: 'black',
  },
});

```

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants