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

Import ResponderEventPlugin changes from RN #5308

Merged
merged 1 commit into from
Dec 22, 2015

Conversation

sophiebits
Copy link
Collaborator

This imports the ResponderEventPlugin.js changes made in facebook/react-native@a0168a8 and facebook/react-native@5c1ac2a and updates tests to match.

@zpao
Copy link
Member

zpao commented Oct 28, 2015

👍 assuming you do a quick check to make sure this doesn't break anything. Doesn't looks like it should…

@zpao zpao mentioned this pull request Oct 28, 2015
@sophiebits
Copy link
Collaborator Author

Ugh, the tests for this in RN aren't running. Kicking this down the road…

@sophiebits sophiebits closed this Oct 28, 2015
@facebook-github-bot
Copy link

@spicyj updated the pull request.

@sophiebits
Copy link
Collaborator Author

This imports the ResponderEventPlugin.js changes made in facebook/react-native@a0168a8 and facebook/react-native@5c1ac2a.

Notably, the changes in the latter diff by @andreicoman11 (internally D2163934) required changes to ResponderEventPlugin-test (which sadly doesn't run in the RN repo) to match its the new behavior. In particular, this now calls responderGrant before calling responderTerminationRequest, whereas it should call responderTerminationRequest first and not dispatch the responderGrant event if responderTerminationRequest returns false. I am going to commit this changed test because it matches the current behavior that RN has been using since August – but this at least deviates from the original intended behavior of ResponderEventPlugin and may be worth revisiting so that responderGrant is one again called at the right times.

cc interested parties @vjeux @kmagiera @jordwalke

sophiebits added a commit that referenced this pull request Dec 22, 2015
Import ResponderEventPlugin changes from RN
@sophiebits sophiebits merged commit bae0f19 into facebook:master Dec 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants