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

fix(nav): ionViewCanLeave does not break navigation #8410

Merged
merged 1 commit into from
Oct 10, 2016

Conversation

manucorporat
Copy link
Contributor

@manucorporat manucorporat commented Oct 2, 2016

Right now, the NavController does the canLeave/canEnter challenge only when the navigation change requires a transition.

  1. Remove from stack
  2. Evaluate if a transition is required
    • if yes: canLeave/canEnter challenge -> continue transition
    • if no: we are done

This logic leads to an important bug when canLeave or canEnter returns false (i.e. the transition has been cancelled) but the pages have been already removed from the stack.

This PR introduces a new logic (that of course passes all the current unit tests):

  1. Evaluate if a transition is required
    • if yes: canLeave/canEnter challenge -> continue
    • if no: continue
  2. Remove from stack
  3. Evaluate if a transition is required (cached value from step one)
    • if yes: transition -> done!
    • if no: done

This way if canLeave returns false, we have not touched the stack yet.

This bug can be reproduced with the following code, the simulates returning false a couple of times, then return true:

    public trys = 0;
    ionViewCanLeave(): any {
        if (this.trys++ < 2) {
            return false;
        } else return true;
    }

It breaks the navigation 100% of the times

fixes #8408

@manucorporat manucorporat force-pushed the fix-canLeave branch 3 times, most recently from 1a953fb to 97496a7 Compare October 4, 2016 21:38
@manucorporat manucorporat added this to the 2.0.0-rc.1 milestone Oct 5, 2016
@manucorporat manucorporat force-pushed the fix-canLeave branch 3 times, most recently from 267d962 to f5815a0 Compare October 10, 2016 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ionViewCanLeave() breaks navigation on second call, causes blank screen
1 participant