Transition abort example in documentation doesn't work

[kimmobrunfeldt]

Example of aborting transition did not work. The willTransitionFrom function doesn't take a callback function. Specifying callback made it work for me.

[agundermann]

What do you mean exactly?

There's an example using the same method. As far as I can tell, it's working flawlessly.

Perhaps there's some confusion around window.confirm? Unlike custom dialog implementations, it's blocking/synchronous, so there's no need to use a callback in this case.

[kimmobrunfeldt]

You're right. I didn't remember it's synchronous call. Hmm, I'm not sure but implementing the willTransitionFrom without cb did not succesfully abort the transition for me

[kimmobrunfeldt]

This works:

statics: {
    willTransitionFrom: function willTransitionFrom(transition, component, cb) {
        // Check if user has written something to input fields
        var inputData = component._gatherInputData();
        var somethingWritten = _.any(_.map(inputData, function(value, key) {
            return Boolean(value);
        }));

        if (!somethingWritten) {
            // Safe to transition to next page
            cb();
            return;
        }

        var msg = 'You have unsaved information, are you sure you';
        msg += ' want to leave this page?';

        if (!window.confirm(msg)) {
            transition.abort();
        } else {
            cb();
        }
    }
},

This doesn't:

statics: {
    willTransitionFrom: function willTransitionFrom(transition, component) {
        // Check if user has written something to input fields
        var inputData = component._gatherInputData();
        var somethingWritten = _.any(_.map(inputData, function(value, key) {
            return Boolean(value);
        }));

        if (somethingWritten) {
            var msg = 'You have unsaved information, are you sure you';
            msg += ' want to leave this page?';

            if (!window.confirm(msg)) {
                transition.abort();
            }
        }
    }
},

"It doesn't work" means that when I press browser's back button and answer cancel to the confirmation message, it still navigates to the previous view.

[kimmobrunfeldt]

I'm using react-router 0.12.4.

[agundermann]

In your working example, you're not calling cb at all, if the transition is supposed to be aborted. Without the callback, you're essentially halting the transition indefinitely or until the next transition. I don't think that's a good idea. Does it still work if you call cb after transition.abort?

Your second example should work as far as I can tell. How does it fail? Does the dialog show up? Does it transition even though you clicked "No"? Do you get any errors in the console?

[kimmobrunfeldt]

Oh okay, I misunderstood the callback then. The second example fails like this: when I press browser's back button and answer cancel to the confirmation message, it still navigates to the previous view.

[kimmobrunfeldt]

You can test this here: http://designopen-board-qa.herokuapp.com/

• Click "Create new post"
• Write something to any input field
• Press back button

This is the code for this page: https://github.com/DesignOpen/board/blob/node/src/frontend/scripts/pages/NewPostPage.jsx

[kimmobrunfeldt]

I initialize react router here: https://github.com/DesignOpen/board/blob/node/src/frontend/scripts/main.jsx

[agundermann]

when I press browser's back button

I think this isn't your fault and that there's a bug in the way we handle aborted transitions: we always simulate clicking the back button if a transition is aborted. This works well when clicking a link (or going 1 forward in general), but behaves strangely in other cases (cancelling a transition from back button leads to going two pages back).

I assume this was implemented this way in order to keep the URL in sync with the content, without manipulating the browser history. I'm pretty sure though that this is actually impossible. Imagine the user going 3 pages back using the back button: we'd have to go three pages forward, but there's no way to determine that number.

Ideas:

• only allow aborting transitions resulting from push or replace. For the use case of not discarding form data, one could instead save the view state (e.g. in memory), and show a notification after the transition
• give up on keeping the browser history consistent by replacing the new URL with the old one
• give up on keeping the URL consistent by simply not rendering the new URL. This would be similar to stalling the transition by not calling the callback.

Also noteworthy: we can't possible abort transitions to other websites (e.g. when you directly navigate to the form page from google, then press back). The best we could do is to use window.onunload, but this would only allow setting the text of a browser-controlled dialog (synchronously), and would be triggered when refreshing the page as well.

[kimmobrunfeldt]

Ok. Thanks for your quick responses. For now, I think I'll just remove the willTransitionFrom implementation and save data to local storage instead.

[liouh]

@kimmobrunfeldt @taurose I ran into this issue today, and solved it by calling history.forward() right after transition.abort() in willTransitionFrom. Seems behave correctly for back, foward, and new link navigations.

Edit: Actually, it seems to have moved the problem from "back" to "forward" (if there is more than 1 entry in the forward history). Better than the more common back problem, but still not great.

[mindjuice]

I ran into the same problem in #1613.

The right thing for the router to do is nothing at all, not go back and then go forward again. The handleAbort() method in createRouter.js has a "do nothing" case when it receives an abortReason that is an instance of Cancellation.

This can be triggered by calling transition.cancel() instead of transition.abort() from your willTransitionFrom()

The cancel() method isn't documented, and the two function names are very similar in meaning. I don't know if this was intentional, but it would be much better if abort() knew whether it was being aborted from a willTransitionFrom() or elsewhere, so the correct action could be taken instead of requiring the developer to choose between two confusingly similar names.

[mindjuice]

So scratch that. Calling cancel() didn't actually quite work right either. The URL was updated to the previous route, but the component still thought it was in the original URL/route from before this.goBack() was called.

[ryanflorence]

I think the new history stuff that the router is buit on fixes this. @mjackson @taurose can you window.confirm this?

[ryanflorence]

closing, if I'm wrong please reopen.