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

Memoize tap GH state for sending proper value after releasing finger #478

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

Conversation

osdnk
Copy link
Contributor

@osdnk osdnk commented Feb 27, 2019

Motivation

Issue happens on iOS. #476.

After releasing finger in tap GH which is waiting for second GH and failure of second GH, values are being set zero and then zeros are send to JS instead on corrects values.

Changes

After reseting values are memoized and placed in nativeEvent. If this kind of situation is detected (no finger on screen) we fallback to memoized valued values instead of evaluating them.

Copy link
Member

@kmagiera kmagiera left a comment

Choose a reason for hiding this comment

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

I'd prefer if we always use memoized value for the event and memoize it in a place where we are certain the values are right. I'm not certain that the conditions you added in eventExtraData are going to work at all times (e.g. could there be a case that numberOfTouches is zero but the code where we memoize hasn't been triggered?)

@farfromrefug
Copy link

@kmagiera i think i fixed this the way you wanted.
I actually use your Native code for a Nativescript module.
Here is the commit if that is of any interest to you
nativescript-community/gesturehandler@6c3ed76

@kmagiera
Copy link
Member

This looks more like how I'd imagine. Thanks for sending @farfromrefug. Willing to send a PR with this change maybe?

@farfromrefug
Copy link

@kmagiera sorry i dont use your code nor react-native

@jkadamczyk
Copy link
Contributor

@kmagiera @jakub-gonet I can apply changes that @farfromrefug proposed.
Is it still relevant?

@jkadamczyk jkadamczyk self-assigned this Sep 1, 2020
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.

4 participants