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

Fix onUp in Fling #2709

Merged
merged 1 commit into from
Jan 8, 2024
Merged

Fix onUp in Fling #2709

merged 1 commit into from
Jan 8, 2024

Conversation

m-bert
Copy link
Contributor

@m-bert m-bert commented Dec 22, 2023

Description

Fling gesture ends when pointer was moved farther than minimum distance. The problem is that onUp method calls endFling after pointer was removed from tracker. This means that calling this.tracker.getLastX(this.keyPointer) (the same for Y position) inside tryEndFling will return undefined.

It is rather cosmetic change - it doesn't really affect behavior since we call tryEndFling inside onPointerMove.

This PR also changes minimum distance the pointer has to travel, since 160 is way to big.

Test plan

Tested on the following code
import React from 'react';
import { View, StyleSheet } from 'react-native';
import { Gesture, GestureDetector } from 'react-native-gesture-handler';

export default function App() {
  const f = Gesture.Fling()
    .onEnd((e) => console.log(e))
    .onFinalize((e, s) => console.log(e, s));

  return (
    <View style={styles.container}>
      <GestureDetector gesture={f}>
        <View style={styles.box} />
      </GestureDetector>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
    backgroundColor: '#ecf0f1',
    padding: 8,
  },
  box: {
    width: 500,
    height: 500,
    backgroundColor: 'blue',
  },
});

@@ -5,7 +5,7 @@ import { AdaptedEvent, Config } from '../interfaces';
import GestureHandler from './GestureHandler';

const DEFAULT_MAX_DURATION_MS = 800;
const DEFAULT_MIN_ACCEPTABLE_DELTA = 160;
const DEFAULT_MIN_ACCEPTABLE_DELTA = 32;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even though it should be fine (and also this functionality can be implemented using Pan), I think it would be better if users could change this value. What do you think @j-piasecki?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, having it set to 160 is a bit much. We could make it also use velocity to determine whether to activate (if the distance was shorter than the minimum delta, but the gesture was fast). Anyway, that's something for another PR, if we want to do this at all.

@m-bert m-bert requested a review from j-piasecki January 2, 2024 07:43
@m-bert m-bert merged commit f22ada3 into main Jan 8, 2024
1 check passed
@m-bert m-bert deleted the @mbert/fix-fling-pointer-up branch January 8, 2024 10:43
j-piasecki pushed a commit that referenced this pull request Jan 12, 2024
## Description

Fling gesture ends when pointer was moved farther than minimum distance.  The problem is that `onUp` method calls `endFling` after pointer was removed from tracker. This means that calling `this.tracker.getLastX(this.keyPointer)` (the same for `Y` position) inside `tryEndFling` will return undefined.

It is rather cosmetic change - it doesn't really affect behavior since we call `tryEndFling` inside `onPointerMove`. 

This PR also changes minimum distance the pointer has to travel, since `160` is way to big.

## Test plan

<details>
<summary> Tested on the following code </summary>

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

export default function App() {
  const f = Gesture.Fling()
    .onEnd((e) => console.log(e))
    .onFinalize((e, s) => console.log(e, s));

  return (
    <View style={styles.container}>
      <GestureDetector gesture={f}>
        <View style={styles.box} />
      </GestureDetector>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
    backgroundColor: '#ecf0f1',
    padding: 8,
  },
  box: {
    width: 500,
    height: 500,
    backgroundColor: 'blue',
  },
});

```

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants