RFC: Add a ui-sref-resolve directive similar to ui-sref-active(-eq)

[lordnox]

The purpose of this is a loading state icon on navigation elements while fetching all data through resolve.

If a user clicks a button in the navigation to change the state (to: app.user.profile) the app needs to fetch the profile data first (resolve: { profile: function($http) { $http.get... } }). During the latency of the network request I'd like to have the button pressed (<a class="btn btn-primary">) to get an extra class ("btn-loading") to show the user an indicator that the click was registered.

The way this currently has to be done:

<nav>
  <a class="btn btn-primary" ng-class="{'btn-loading': changingState}" ui-sref="app.user.profile">Profile</a>
</nav>

function Controller($scope, $state) {
  $scope.changingState = false; // simplified here would need one for each button
  $scope.go = function(state) {
    $scope.changingState = true;
    $state.go(state).then(function() { $scope.changeingState = false; })
  }
}

Proposed change:

<nav>
  <a class="btn btn-primary" ui-sref-resolve="btn-loading" ui-sref="app.user.profile">Profile</a>
</nav>

[jhiemer]

Need something quite similar over here.

+1

[nateabele]

This seems like a very specific solution to a very general problem. Why not something more like this? (Warning: off-the-cuff, untested):

app.directive("myLoader", () => {
  return {
    restrict: "A",
    scope: { myLoader: "=", myLoaderClass: "@" },
    link: (scope, element) => {
      scope.$watch("myLoader", (oldVal, newVal) => {
        if (!newVal || newVal === oldVal) return;
        element[0].classList.add(scope.myLoaderClass);
        newVal.then(() => element[0].classList.remove(scope.myLoaderClass));
      });
    }
  }
});

3 lines of directive linking code that aren't hard-coded to anything. In fact, they're completely composable with any other thing that supports promises.

Then in your controller/template you could do:

$scope.transition = $state.go("app.user.profile");

and:

<a
  class="btn btn-primary"
  my-loader="transition"
  my-loader-class="btn-loading"
  ui-sref="app.user.profile"
>Profile</a>

What I would consider doing is exposing some kind of event hook on uiSref that exposes the promise returned from $state.go(). Would that maybe work for you?

[lordnox]

If I am reading your code right I'm pretty sure it would not work as $state.go would go directly to the state and I'd have to wrap every ui-sref into ng-clicks which in turn reduces the uses of ui-sref to states where I explicitly know that there are no time-costly resolve queries.

The possibility of an exposed $scope.go promise would be what is needed.

The simplest solution might be just to have the code in my PR just expose an callback instead of setting the class, maybe both with type-detection.

I think the better solution would be to convert the ui-sref link function to be an actual controller. Then I'd be able to write my ui-sref-resolve and require the controller.

How does that sound?

[lordnox]

I did not know what the preferred way is...

Here is a method triggering a event on the current scope. It's not cleaned up yet, but up and running.

Pro:

• it introduces just a few lines, easily tested
• it enables every parent-scope to handle the event
• might solve Apply a custom animation to ui-view during resolve #456

Contra:

• it triggers an extra event that needs documentation

ToDo:

• Decide on event name
• clean up code
  • remove controller again
  • add toState / fromState ( and other useful data )

        var promise = $state.go(state, params, options);
        if(promise) {
          $scope.$emit('$uiSref', {
            element: element,
            promise: promise
          });
        }

Would this be a solution you'd merge?

[nateabele]

I think the better solution would be to convert the ui-sref link function to be an actual controller. Then I'd be able to write my ui-sref-resolve and require the controller.

I think this is a good idea, and not necessarily mutually exclusive with the event idea. I'd go for something like this:

diff --git a/src/stateDirectives.js b/src/stateDirectives.js
index 2715927..1b7fb0f 100644
--- a/src/stateDirectives.js
+++ b/src/stateDirectives.js
@@ -134,7 +134,8 @@ function $StateRefDirective($state, $timeout) {
         if ( !(button > 1 || e.ctrlKey || e.metaKey || e.shiftKey || element.attr('target')) ) {
           // HACK: This is to allow ng-clicks to be processed before the transition is initiated:
           var transition = $timeout(function() {
-            $state.go(ref.state, params, options);
+            var $transition = $state.go(ref.state, params, options);
+            if (attrs.uiSrefOn) scope.$eval(attrs.uiSrefOn, { $transition: $transition });
           });
           e.preventDefault();

Btw, sorry, I realize I wasn't using very clear language. When I said 'event', I meant 'event handler' vis-a-vis something like ng-click. With the above, you can do, i.e.:

<a
  ui-sref="mystate({ foo: 'bar' })"
  ui-sref-on="ui.loadState = $transition"
  my-loader="ui.loadState"
>...</a>

That lets you handle the transition inline with no extra controller code (unless you need it). If that works for you, and you want to write up some tests & docs for it, I'll merge it (otherwise I'm happy to do it myself as soon as I have time, but I'm betting you'll get to it before I do 🐢).

[nateabele]

Anyway, to conclude, if you want to resubmit a separate PR that just factors out the uiSref link function into a controller, that'd be fine, too. Then you'll have a more flexible API for making directives that goes beyond what's available in the core if you need to do really specific stuff.

(Also, finally, please keep whitespace changes in separate PRs as changes are too hard to track otherwise. Thanks!)

[lordnox]

I still think that a real event might be better as you can handle it from anywhere, solving the ui-view issue. But this will work for me.

Thanks

[nateabele]

@lordnox Patch looks good. Please squash the commits and fix the message per the contributor guideline, i.e. something like:

feat(uiSref): add ui-sref-on to expose transition promise

Fixes #1863

Thanks!

[lordnox]

Arg... need to practice my git-fu ... Hope this is like you need it

[nateabele]

@lordnox This looks perfect. Open a PR with just that commit and I'll merge it (I closed the other one for you).

[stale]

This issue has been automatically marked as stale because it has not had
recent activity. It will be closed if no further activity occurs.

This does not mean that the issue is invalid. Valid issues
may be reopened.

Thank you for your contributions.