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

[Navigator] Pop route from stack when dismissed via gesture. #1018

Closed
wants to merge 1 commit into from

Conversation

mwilc0x
Copy link
Contributor

@mwilc0x mwilc0x commented Apr 25, 2015

This PR fixes an issue described in #1014 where a route is not being removed from the stack when dismissed via a pop gesture.

@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 Apr 25, 2015
@mwilc0x mwilc0x changed the title [Navigator] Remove a route from the stack with a pop gesture. [Navigator] Pop route from stack when dismissed via gesture. Apr 26, 2015
@brentvatne
Copy link
Collaborator

@mjw56 - just to clarify: When you pull down to dismiss a view that was introduced with FloatFromBottom it currently does not pop the route from the stack but does dismiss the view. This pull request also removes it from the route stack. Is this correct?

@mwilc0x
Copy link
Contributor Author

mwilc0x commented Apr 29, 2015

@brentvatne hey, yeah that's correct. It will remove it from the stack and call the dismissed component's componentWillUnmount lifecycle hook.

@ericvicenti can you help verify this is the right fix?

@@ -736,6 +736,7 @@ var Navigator = React.createClass({
if (transitionVelocity < 0 || this._doesGestureOverswipe(releaseGestureAction)) {
this._transitionToFromIndexWithVelocity(transitionVelocity);
} else {
this._handlePop();
Copy link
Contributor

Choose a reason for hiding this comment

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

I probably wouldn't do it this way. This will theoretically start the transition twice, and _handlePop also sometimes calls pop on child navigators instead of itself

I have refactored this code already and I think I have this issue fixed. The new code will land soon and we can double check

@mwilc0x
Copy link
Contributor Author

mwilc0x commented Apr 29, 2015

Ok, thanks for your reply @ericvicenti! I will close this and let it be handled in your refactored code.

Cheers

@mwilc0x mwilc0x closed this Apr 29, 2015
@vitch
Copy link

vitch commented Apr 29, 2015

I've just run into the same problem. Any ETA on when the refactored code will land?

Also, #1025 can probably be closed along with this?

@skevy
Copy link
Contributor

skevy commented May 18, 2015

@ericvicenti running into this issue as well...any updates on the refactor?

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.

7 participants