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

Don't cancel long press after activation on Android #574

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lnikkila
Copy link
Contributor

Makes long presses more consistent with iOS, which doesn’t cancel the gesture due to maxDist if it’s already in the ACTIVE state. This is also more consistent with TapGestureHandler, whose docs specifically say that maxDist fails if the “handler hasn't yet activated”.

Makes long presses more consistent with iOS, which doesn’t cancel the
gesture due to maxDist if it’s already in the `ACTIVE` state. This is
also more consistent with TapGestureHandler, whose docs specifically
say that maxDist fails if the “handler hasn't yet activated”.
@szholdiyarov
Copy link

Hi @lnikkila

thanks for PR.

how to reproduce this issue?

@lnikkila
Copy link
Contributor Author

We have a LongPressHandler that uses onHandlerStateChange and onGestureEvent together to first wait for a long press, and then observe drag touch events (XY-coordinates) until the user lets go. This handler also has a max distance limit so that the activating long press needs to be stationary.

On iOS, once the long press handler activates, you can then keep dragging arbitrary distances without the max distance limit interrupting the gesture. This makes sense since the long press has already activated.

On Android, the max distance limit terminates the gesture even after it has activated, and you can’t drag after the long press. This patch changes behaviour on Android to be consistent with iOS, and makes a “long press and drag” gesture possible without adding additional handlers like PanGestureHandler.

If you need a repro sample to review this, I can write one later this week.

@osdnk
Copy link
Contributor

osdnk commented May 22, 2019

@lnikkila
Would you like to add some example?

@lafiosca
Copy link
Contributor

lafiosca commented Dec 3, 2019

I'm finding this happens on Android even when the maxDist is set to a very large number like 1000 or Number.MAX_SAFE_INTEGER. As soon as I drag the touch outside of the bounds of the view that the gesture is wrapping, it switches from state 4 (ACTIVE) to state 3 (CANCELLED).

@lnikkila
Copy link
Contributor Author

lnikkila commented Dec 3, 2019

Sorry about the wait, I've been super busy.

Here's some pseudocode that should repro this issue, the expected behaviour is for the long press to not deactivate after it's first been activated even if you keep dragging, this allows observing drag events after the initial long press:

const onHandlerStateChange = ({ nativeEvent }) => {
  const { state } = nativeEvent;

  if (state === State.ACTIVE) {
    console.log('ACTIVE');
  } else if (state === State.END) {
    console.log('END');
  } else if (state === State.CANCELLED) {
    console.log('CANCELLED');
  }
};

const onGestureEvent = ({ nativeEvent }) => {
  // You should be able to observe these touch events after long pressing and
  // then dragging more than `maxDist` after the initial activation.
  console.log(nativeEvent);
};

const Component = () => {
  return (
    <LongPressGestureHandler
      onGestureEvent={onGestureEvent}
      onHandlerStateChange={onHandlerStateChange}
    >
      <View style={{ flex: 1, backgroundColor: 'red' }} />
    </LongPressGestureHandler>
  );
}

At the time I wrote this patch this worked as expected on iOS, but emitted the CANCELLED state after exceeding maxDist on Android. This might've changed since, we're still using an older version.

@lafiosca
Copy link
Contributor

lafiosca commented Dec 3, 2019

I have confirmed the behavior described by @lnikkila on the current version as well.

Regarding my earlier comment, I realized that setting shouldCancelWhenOutside to false addresses my problem. (Seems odd that this option is ignored on iOS.)

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.

5 participants