Skip to content

Conversation

sooth-sayer
Copy link
Contributor

Fixes #9485

@ghost
Copy link

ghost commented Aug 22, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@@ -692,6 +701,11 @@ var Navigator = React.createClass({
this.refs['scene_' + sceneIndex].setNativeProps(enabledSceneNativeProps);
},

_clearTransformations: function(sceneIndex) {
const defaultStyle = flattenStyle([styles.defaultSceneStyle]);

Choose a reason for hiding this comment

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

react/no-string-refs: Using this.refs is deprecated.

@mkonicek mkonicek changed the title Fix wrong scene transformation after pop Navigator - Fix wrong scene transformation after pop Sep 8, 2016
@mkonicek
Copy link
Contributor

mkonicek commented Sep 8, 2016

Thanks for the great demo in #9485!

@ericvicenti Does this look good to you?

@@ -1087,6 +1101,7 @@ var Navigator = React.createClass({
var presentedRoute = this.state.routeStack[this.state.presentedIndex];
var popSceneConfig = this.props.configureScene(presentedRoute); // using the scene config of the currently presented view
this._enableScene(popIndex);
this._clearTransformations(popIndex);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add a comment why this is needed so people reading the code in the future understand why it's needed to reset the transform here?

@sooth-sayer
Copy link
Contributor Author

@mkonicek Is this explanation (10c5093) clear enough?)

@lacker
Copy link
Contributor

lacker commented Oct 19, 2016

@sooth-sayer can you fix the problem that eslint-bot pointed out?

@sooth-sayer
Copy link
Contributor Author

@lacker Ok, I will fix it

@facebook-github-bot
Copy link
Contributor

@sooth-sayer updated the pull request - view changes

@@ -726,7 +742,7 @@ var Navigator = React.createClass({
},

_setRenderSceneToHardwareTextureAndroid: function(sceneIndex, shouldRenderToHardwareTexture) {

Choose a reason for hiding this comment

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

no-return-assign: Arrow function should not return assignment.

@sooth-sayer
Copy link
Contributor Author

@lacker fixed

@ericvicenti
Copy link
Contributor

Ok this makes sense to me.

@facebook-github-bot shipit

@ericvicenti
Copy link
Contributor

@facebook-github-bot shipit

@lacker
Copy link
Contributor

lacker commented Oct 25, 2016

I think this needs a CLA to ship it. @sooth-sayer would you mind signing the CLA?

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 26, 2016
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@sooth-sayer
Copy link
Contributor Author

@lacker Ok, I signed it

@lacker
Copy link
Contributor

lacker commented Oct 26, 2016

@facebook-github-bot shipit

@facebook-github-bot facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. labels Oct 26, 2016
@facebook-github-bot
Copy link
Contributor

@lacker 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:
Fixes facebook#9485
Closes facebook#9516

Differential Revision: D4080618

Pulled By: lacker

fbshipit-source-id: 296c209a0438c4a06b3b3556d7084c5435d60c72
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. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants