Stopping tab change

[skidvd]

I have a scenario in which I am listening to the $stateChangeStart. In this case, I am verifying if the user has an unsaved changes and presenting a confirmation dialog to allow then to cancel (stay on the present tab and save the data) or continue (ignore their changes). All is working well with one important exception: event though I am calling event.preventDefault in response to the $stateChangeStart event, the tab is still be moved (i.e. the active tab class still changes) to the new tab the user clicked on. Is there a way to get ui-router-tabs to behave correctly in this situation (i.e. if the event is cancelled with preventDefault() then stay on present tab)? I have not seen anything in the docs or a quick review of the code, so would appreciate your assistance please.

[rpocklin]

There is some code for this in the link function which should ensure this works as you'd expect:

        var unbindStateChangeSuccess = $rootScope.$on('$stateChangeSuccess',
          function() {
            scope.update_tabs();
          });
          scope.$on('$destroy', unbindStateChangeSuccess);

This would delay updating the tabs until a successful state change. Is this along the lines of what you were thinking?

[skidvd]

Yes, that seems to be along the lines of what I was thinking. However, it does not appear to be working. I have verified that the URL is NOT updated in the case that i am doing event.preventDefault() - this confirms for me that the state change has been stopped. Yet, the tab set is still visibly updated to reflect the tab that the user clicked on (prior to the event.preventDefault())

I think the problem is related to the fact that the $scope.go function calls $state.go directly. Either that or something lower level, perhaps in bootstrap tabs - not sure.

[rpocklin]

Let me look into it, sounds like something easy to write a test for.

[skidvd]

Plunk is here: http://plnkr.co/edit/pGToXRrAOTxHlD3Ps5Rq .... (using version 1.3.0)

[rpocklin]

Great thanks. It seems to work as you would want it to. It does not load the tab 3 content, tested in Chrome and Firefox.

Not sure how I can help since it seems to work as intended.

I just created a testcase for this (not checked in) which I will add to the specs in master (here it is to give you an idea):

  it('should not change the route if a $stateChangeStart handler cancels the route change', function() {
    view = '<tabs data="tabConfiguration"></tabs>';
    renderView();

    var route = scope.tabConfiguration[2].route;

    root_scope.$on('$stateChangeStart', function(event) {
      event.preventDefault();
    });

    state.go(route);
    scope.$apply();

    expect(get_current_state()).not.toEqual(route);
  });

[skidvd]

Yes, it does not load the tab3 contents. BUT, it does render the tab 3 as selected. This rendering is the problem as the visual representation is now out of synch with the real data and the user is confused. The visual rendering should still depict the original tab that was selected before the click on tab 3 as selected (not tab 3 as it was prevented). Can you please take another look with this visual state in mind (this is not addressed by your test case).

[rpocklin]

Ok so that's the only issue then.

Forcing an update after $stateChangeStart does the trick (putting this and an unbind handler in the link phase):

        var unbindStateChangeStart = $rootScope.$on(
          '$stateChangeStart',
          function() {
            scope.update_tabs();
          }
        );

but i'm convinced you just want to disable the tab in certain situations. As luck would have it, I released v1.4.0 today which includes a disabled property. I'd recommend you use that instead, it will be a lot cleaner.

BTW since you are using a custom template, for v1.4.0 change your ng-click function from go(tab.route, tab.params, tab.options) to go(tab).

[skidvd]

Hello and thank you. Unfortunately, I do NOT want to disable the tab in this case. The user is free to say that they want to continue to the newly chosen tab if they wish (and ignore their previous changes) - see use case description at start of this thread. In this case, the state change will succeed and the new tab (tab 3) should be visually selected). If I disable, then they will not be able to do any of this as they cannot initiate a tab change to the specific tab - not what I need in this case.

I appreciate your code sample, but am unclear where I can place it in my code? It seems like it would need to be in your directive in order to have the proper scope? I have attempted several ways to work around this visual issue without success, so I will appreciate a fix for this issue in the directive (or a clear example how to handle in my code), please.

[rpocklin]

In that case why can't you use your custom template ng-click method to call a custom function, which shows the confirm and either calls $state.go('your-tab-route') or similiar, or does nothing? Catching a $stateChangeEvent part-way through then cancelling it feels a bit heavyweight, though I agree cancelling the state change should revert the active tab - no one else has asked for it.

[skidvd]

The reason I am using the $stateChangeEvent based approach is because I need to apply globally within the app - not only for tab navigation, but also for page-to-page navigation outside of the tabs. I am working on a directive that is intended to be general purpose for detecting potential unsaved changes, notifying the user and allowing them to choose next steps (i.e. cancel or continue/ignore) - all triggered by a stateChangeEvent. Additionally, the fact that I am using a custom template is coincidental in this case - but not in all cases I have, so the ng-click approach will not work in all cases.

If you agree that cancelling the state change should revert the active tab, then how do we get to that behavior?

[rpocklin]

Unfortunately there is no $stateChangeCancel event UI Router provides, and $stateChangeError is not triggered by cancelling in a $stateChangeStart event. If you can write a generic way to handle this i'm happy to review it in a PR.

[skidvd]

How about adding logic in the directive to detect that the active tab has been changed to something other than it should be an reverting it accordingly? This could be done regardless of any UI Router constructs.

[skidvd]

Here is my present workaround ... I'm sure there must be a better way and am open to your suggestions/improvements! In the $scope.go function, I added a check after a small timeout (needed to allow the state transition to complete (or not if cancelled)....

$scope.go = function(tab) {

          if (!currentStateEqualTo(tab) && !tab.disabled) {
            $state.go(tab.route, tab.params, tab.options);

            // need a small delay to allow state transition to be completed (or not if cancelled)
            setTimeout(function() {
                if (tab.route !== $state.current.name) {
                    $scope.update_tabs();
                }
            }, 250);
          }
        };

This seems to keep the tab visual state correct in my testing so far. Looking forward to your thoughts and input. BTW, this is with the 1.4.0 version.

[rpocklin]

That will never work if there are any async resolves on state change.

I can see you are very keen to solve this issue :)

What about manually triggering a $stateChangeError event if the user cancels the navigation? I can make ui-router-tabs update the tabs when it encounters this event.

See http://plnkr.co/edit/sXTMQT82B69gzW0thKvW

IMO ui-router could do with a $stateChangeCancel event for this purpose, but this is the best suggestion I can make given the circumstances.

[skidvd]

Indeed, I am keen to solve this issue. Thank you for both your assistance and this useful directive!

I share your opinion that a ui-router $stateChangeCancel would be helpful for these purposes. Not sure if you have any connections there, but it may be worth running by them to see if this has been requested before or could be easily added?

While I understand your perspective and proposed solution, I would prefer something more along the lines that I proposed (i.e. fully encapsulated in the directive) for the following main reasons:

• I am a proponent of full encapsulation within a directive of such related logic. Allowing too much knowledge of the inner workings to bleed out and subsequently become part of the application code baseline seems not like the right approach IMHO. What happens if the directive or its dependencies (ui-router in this case) change downstream ... it would be best, where possible, if this did not impact application code.
• (Mis?)-using the $stateChangeError event can have unforseen consequences as many apps (including ours) will attach additional logic an/or behavior to such an event. Manually triggering it would then need to have some special case logic added to determine which conditions are real errors and which are not and how to react appropriately.
• My personal preference is to limit use of broadcast approaches (although I understand that there are some times this is the best approach - I just personally always challenge myself to explore all other options before going that route).

What would you think about evolving the original approach I suggested into a watch based algorithm instead of the delay)? This would keep the logic entirely encapsulated in the directive, address your valid concern about async resolves (although I am honestly not really clear on the order of execution there - i.e. would resolves actually continue once the event is cancelled vi preventDefault?) and keep the use of $stateChangeError clean for its intended purposes. I'm also always cautious adding watches as this can have adverse performance impacts when used in excess. However, in this case the addition of a single watch seems to me to be worth if relative to the other potential trade offs we are considering. Looking forward to your thoughts.

[skidvd]

Actually, I just had a chance to do some testing here. It appears that async resolves are NOT even triggered if you event.preventDefault() in the $stateChangeStart. Based upon this and the above comments, I still prefer my original approach. What are your thoughts given that there is not an issue created by it for resolves?

[rpocklin]

Since this directive is based on ui-router, it uses the events to be informed of the state of routes. This is necessary, it isn't breaking encapsulation. I agree $stateChangeError is a workaround, so i've pinged the guys @ ui-router to review a PR for $stateChangeCancel - I suggest you explain the use case you've given me to them (angular-ui/ui-router#1090). I need it for another use case so if we can convince them to put it in then it's a win / win.

I'll take a further look to see if setTimeout(.., 0) fixes the problem (which would delay it for a digest cycle) even if resolves are used, but it's a hack - i'm not fond of hacks. You could also fork this directive or decorate it to extend the behaviour for your use case as a last resort.

I'm very mindful of not solving one special use case at the expense of all others - but if either of these are flexible enough to work I'll concede.

[skidvd]

In my testing setTimeout(..., 0) works and does NOT invoke async resolves when cancelled.

[rpocklin]

Ok, $stateChangeCancel has been accepted in ui-router but until it's released here's a workaround. It hooks onto the $state.go(..) promise and will update the tabs if the promise is rejected (ie. when preventDefault is called) rather than using $timeout.

Take a look at the plunkr here: http://plnkr.co/edit/sXTMQT82B69gzW0thKvW
the code is in the branch https://github.com/rpocklin/ui-router-tabs/tree/update-tab-header-on-state-cancel

If this is a suitable workaround i'll release it, with a view to using $stateChangeCancel event in a later release.

[skidvd]

Great news about $stateChangeCancel! That will be useful for these as well as other purposes.

I like your workaround. It appears to work and serve the purpose until $stateChangeCancel lands. Please let me know which version will have this workaround in it.

[skidvd]

Any updates here on the workaround version until $stateChangeCancel lands?

[rpocklin]

Released just now with v1.4.2 https://github.com/rpocklin/ui-router-tabs#history

I've added a donations link on the homepage https://gratipay.com/rpocklin/ if you feel this feature has helped you out.

[skidvd]

@rpocklin, I have been using the 'workaround' in 1.4.2 since you released it. It works much of the time. However, in some instances (hard to put a percentage on it, but it happens often enough to be problematic, the added catch and associated update_tabs() are called, but the visually represented tab still moves.

I know it feels hackish, but so far I have never had any similar 'failures' with this workaround:

setTimeout(function() {
if (tab.route !== $state.current.name) {
$scope.update_tabs();
}
}, 0);

Any idea what could be causing it, or if/when $stateChangeCancel will land?

[rpocklin]

UI Router has been released with $stateChangeCancel so i'll be reviewing PRs this week and aim for a release end of next week.

[rpocklin]

Changes have landed with 1.4.3