veliovgroup / flow-router

🚦 Carefully extended flow-router for Meteor
https://packosphere.com/ostrio/flow-router-extra
BSD 3-Clause "New" or "Revised" License
202 stars 29 forks source link

Using stop() still changes the URL to the next path #62

Closed jasongrishkoff closed 5 years ago

jasongrishkoff commented 5 years ago

When firing stop() inside of the global triggers.exit, the expected behavior is that the user is not taken to the next route. From what I can see, FlowRouter still updates all its internals to reflect that we're on the new route, even though the page doesn't actually change.

FlowRouter.triggers.exit([pageChange])
pageChange = function(context,redirect,stop) {                                                                        
   if (context.path.indexOf('step=') > -1                                                                         
        && !confirm('Are you sure you want to leave this page?'                                                       
            + ' You haven\'t finished yet.')                                                          
    ) {
        stop()
    }
}

This guy seems to have had a similar problem and solved it, but the pull request was never merged? https://github.com/kadirahq/flow-router/pull/681

dr-dimitru commented 5 years ago

Hello @jasongrishkoff ,

Very sorry for late reply. Could you describe in more details in more details, is the URL in the browser get changed? Is the new template get rendered?

Note: You can't call stop() from async callback

jasongrishkoff commented 5 years ago

The URL gets changed, but the template does not get rendered.

dr-dimitru commented 5 years ago

@jasongrishkoff do you have any errors or exceptions in a browser/server console? Sound's like routing was interrupted by something

dr-dimitru commented 5 years ago

@jasongrishkoff let me know if you have more details or it's got solved.

jasongrishkoff commented 5 years ago

There are no errors in the browser/server console.

dr-dimitru commented 5 years ago

@jasongrishkoff Minimal reproduction would help a lot. Marked [help wanted]

dr-dimitru commented 5 years ago

Hey @jasongrishkoff,

I believe right after you call .stop() you should redirect to other route or render another template. Described behavior matching expected as .stop() will interrupt routing chain of actions, in your case interruption happens between changing URI and rendering new template.

Feel free to close it in case if the issue is solved on your end.

jasongrishkoff commented 5 years ago

To me, "stop" would imply that I want to cancel my routing and stay where I am. The fact that it only fires after the URI has changed seems wrong. I've already used a workaround, so in a sense this is solved.

dr-dimitru commented 5 years ago

@jasongrishkoff would you share your workaround?

jasongrishkoff commented 5 years ago

I basically just trigger the back button.

FlowRouter.triggers.enter([loaderBar,pageChange])
pageChange = function(context,redirect,stop) {
  if (!confirm('Are you sure you want to leave this page?') {
    stop()
    setTimeout(() => { window.history.back() })
  }
}
dr-dimitru commented 5 years ago

@jasongrishkoff thank you.

We did additional checking, there is no easy way (or at least we haven't found any) to change this behavior, this part remained unchanged from original flow router.