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

Dispatch native handled events to JS #10981

Conversation

janicduplessis
Copy link
Contributor

When native events where handled they were not sent to JS as an optimization but this caused some issues. One of the major one is touches are not handled properly inside a ScrollView with an Animated.event because it doesn't receive scroll events so it can't cancel the touch if the user scrolled.

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @ide and @ryangomba to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Nov 17, 2016
@ryangomba
Copy link
Contributor

Great, LGTM

@ryangomba
Copy link
Contributor

Separately, @janicduplessis I think we shouldn't rate limit native events - e.g.. ignore scrollEventThrottle when dispatching native event calls, but respect it when calling across the bridge.

@janicduplessis
Copy link
Contributor Author

It would be nice to do so but it will require changing quite a bit since at the moment we are hooking up to the JS event dispatcher to intercept native events so they are already throttled at this point, I think the best solution would be to have a generic way to throttle events inside the dispatcher instead of the adhoc implementation in ScrollView and have native animated events ignore that.

@mkonicek
Copy link
Contributor

Could sending events to JS that were handled break anything?

@mkonicek
Copy link
Contributor

cc @kmagiera, @astreet

@janicduplessis
Copy link
Contributor Author

@mkonicek It does not since native animated event won't do anything when it receives the event in JS. The issue happens because some components like ScrollView for example do some JS logic so they expect to still receive those events even when there is an animated event attached.

@astreet
Copy link
Contributor

astreet commented Nov 23, 2016

lgtm

@facebook-github-bot
Copy link
Contributor

@astreet has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

DanielMSchmidt pushed a commit to DanielMSchmidt/react-native that referenced this pull request Jan 4, 2017
Summary:
When native events where handled they were not sent to JS as an optimization but this caused some issues. One of the major one is touches are not handled properly inside a ScrollView with an Animated.event because it doesn't receive scroll events so it can't cancel the touch if the user scrolled.
Closes facebook#10981

Differential Revision: D4226403

Pulled By: astreet

fbshipit-source-id: 41278d3ed4b684af142d9e273b11b974eb679879
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants