Skip to content

fix [Navigator] Wrong scene transformation after pop #9485 - #9493

Closed
liusy182 wants to merge 2 commits into
react:masterfrom
liusy182:bug-fix-9485
Closed

fix [Navigator] Wrong scene transformation after pop #9485#9493
liusy182 wants to merge 2 commits into
react:masterfrom
liusy182:bug-fix-9485

Conversation

@liusy182

Copy link
Copy Markdown

fix for #9485

This bug can be reproduced by doing navigator transition FloatFromRight then FadeAndroid then do a navigator.popToTop().

The reason for the bug is because FloatFromRight specifies a translation for the scene but FadeAndroid does not (it only specifies opacity). As a result, navigator.popToTop() does not recover translation done by FloatFromRight properly, as the translation property will stay at last specified value (value of FloatFromRight.transformTranslate.to).

To fix this bug, we will provide a default transformTranslate property to FadeIn and FadeOut so that translation is able to recover.

@ghost

Copy link
Copy Markdown

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

@ghostghost 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 Aug 20, 2016
@ghost

Copy link
Copy Markdown

@liusy182 updated the pull request - view changes

@ghostghost 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 Aug 20, 2016
@mkonicek

Copy link
Copy Markdown
Contributor

@skevy Any opinions on this one?

@ghostghost 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 Sep 8, 2016
@mkonicek

mkonicek commented Sep 8, 2016

Copy link
Copy Markdown
Contributor

Is the issue only with the fade animations? With this fix, would the default transformTranslate transform have to be added to other animation configs added in the future?

The solution in #9516 seems more generic - what do you think?

@ghost

Copy link
Copy Markdown

It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @MengjueW as a potential reviewer. Could you take a look please or cc someone with more context?

@ghostghost 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 Sep 19, 2016
@hramos

Copy link
Copy Markdown
Contributor

Ping @liusy182

@liusy182

Copy link
Copy Markdown
Author

yes i think #9516 is a more proper fix.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis 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.

3 participants

@liusy182@mkonicek@hramos