Skip to content

Feature: Default sub-state for abstract states #1235

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

Closed
ProLoser opened this issue Jul 31, 2014 · 42 comments
Closed

Feature: Default sub-state for abstract states #1235

ProLoser opened this issue Jul 31, 2014 · 42 comments

Comments

@ProLoser
Copy link
Member

As per this discussion: https://github.com/ProLoser/AngularJS-ORM/issues/12

It would be cool if abstract states could have a default sub-state. Lets say project is an abstract state. However, instead of being unable to go directly to it, if you DO go directly to it, you automatically get pushed into a substate (such as project.tasks). This should probably handle either a string or an injectable callback function.

Otherwise I have to do crap like this:

function ProjectController($scope, $state) {
  $scope.$on('$stateChangeSuccess', function(event, toState) {
    if (toState.name === 'project') {
      $state.go('.tasks');
    }
  });
}

If you simply do the check once when the controller loads, then going from project.whatever to project doesn't re-execute the controller and therefore the check isn't performed again. If you put this logic earlier (such as in a resolve or onEnter) then the (async?) call to $state.go will restart the entire resolve resolution process since the route never finished loading, which causes an endless loop (unless you add toState checks in, yadda yadda).

Long story short, I think this is an excellent idea and I approve this message.

@timkindberg
Copy link
Contributor

I agree.

@cliechty
Copy link

cliechty commented Aug 4, 2014

I have also been trying to get this to work since I need to route to the first item in a collection returned from the server. I have it working with the above work around, however I'm not sure what the best way to handle routing back to the parent state when the user is already at a child state.

It also looks like you don't need to listen for route changes.
Example:

controller: function($scope, $state, recordList) {
    $scope.recordList = recordList;

    if (/parentState$/.test($state.current.name)) {
        $state.go('.childState', { id: recordList[0].id });
    }
}

@uberspeck
Copy link

+1

@marcfallows
Copy link

Thanks for raising this @ProLoser. I have now come across the need for this feature a few times and would be great to see in ui-router.

@marcghorayeb
Copy link

The idea is sound. I have also come across this requirement.

@Psvensso
Copy link

+1

@kbaltrinic
Copy link

Instead of, or perhaps in addition to having a default child state for an abstract state, I would like to see a $stateIsAbstract event similar to the $stateNotFound event. This would permit introducing logic to determine the correct alternate state, which in my case can vary depending on who the current user is, if they have landed on the intro page before or not (which they only need to see once) etc. This logic relies on application state data that is not available in the configuration block and so needs to be handled in an event or similar mechanism. My current solution is to set isAbstract false and handle the redirect in the $stateChangeStart event.

@ProLoser
Copy link
Member Author

ProLoser commented Oct 3, 2014

Perhaps, however I would normally prefer to see that logic located in a function defined as the value of the abstract key:

abstract: function(resolvedItem) {
  // do checks here on resolvedItem
  return 'abstractParent.defaultChild';
}

// or

abstract: 'abstractParent.defaultChild'

However there's no reason the event couldn't be added as a redundancy

@kbaltrinic
Copy link

The problem with the above is that the function is declared in the context of the module.config callback, into which a very limited set of services ($providers specifically and most notably no actual services) can be injected. Such a function would not have access to any of the normal application state that it would need in the course of executing its decision tree.

@ProLoser
Copy link
Member Author

ProLoser commented Oct 3, 2014

@kbaltrinic no idea what you're talking about, it would work almost exactly like how resolves work. In fact it'd probably fall under the resolve chain. Resolves fire at runtime, there's no reason this wouldn't either.

@ProLoser
Copy link
Member Author

ProLoser commented Oct 3, 2014

.state('parent', {
  // ...
  url: '/project/:id',
  resolve: {
    project: function($http, $stateParams) {
      return $http({ url: '/project/' + $stateParams.id });
    }
  },
  abstract: function(project){
    return (project.started) ? '.begin' : '.dashboard';
  }
})

@kbaltrinic
Copy link

Ah, I see what you are thinking now. It had not occurred to me that the abstract function would have access to the resolve dependencies or be injectable, though in retrospect I suppose it makes sense that it would be. Yes, in that case the function should work for all our cases.

@sricc
Copy link

sricc commented Oct 16, 2014

👍

KOBA789 pushed a commit to GehirnInc/ui-router that referenced this issue Nov 12, 2014
@raphaelpereira
Copy link

+1

2 similar comments
@jmaynier
Copy link

+1

@laurent-le-graverend
Copy link

+1

@inca
Copy link

inca commented Jan 2, 2015

would ❤️ to have it, too

@marktopper
Copy link

👍

1 similar comment
@felipemarcos
Copy link

👍

@inca
Copy link

inca commented Feb 6, 2015

Any progress made on this one?

@christopherthielen
Copy link
Contributor

in #1584 @acollard said:


This is how we handle redirects.

.config(['$stateProvider', '$urlRouterProvider',function($stateProvider, $urlRouterProvider) {
    $stateProvider
        .state('Account.Security', {
          url: '/security',
          redirectTo: 'Account.Security.ChangePassword' // OR
          //redirectTo: function($state){ $state.go('Account.Security.ChangePassword', { id: 'some id' }); }
        });
});

.run(['$rootScope', '$state', function($rootScope, $state){
      $rootScope.$on('$stateChangeStart', function(event, toState) {
        var redirect = toState.redirectTo;
        if (redirect) {
          event.preventDefault();
          if(angular.isFunction(redirect))
              redirect.call($state);
          else
              $state.go(redirect);
        }
      });
    }]);

The redirectTo function could do whatever logic you needed. Ex. get the id for the last item the user was viewing.


@acollard That's pretty close to what I'm thinking. In ui-router-extras I've added a similar option for default substate, using default for the key and { state: "", params: {} } as the value.

dsr: { 
  default: { state: "Account.Security.ChangePassword", params: { id: 123 } }
}

@christopherthielen
Copy link
Contributor

in #1584 @getvega said:


One problem though: state parameters. What if you had something like:

$stateProvider
    .state("posts", {
        url: "/posts",
        defaultChild: "posts.post"
    })
        .state("posts.post", {
            url: "/:id"
        })

Which id would we inject in posts.post?

That defaultChild would have to also specify which params, and maybe be an invokable similar to controllers :

$stateProvider
    .state("posts", {
        url: "/posts",
        defaultChild: "posts.post",
        defaultChildParams: ["posts", function(posts) {
                return {
                        id: posts[0].id
                };
        }]
    })

Not so clean anymore... What do you think?


@getvega Thanks, that's an interesting use case that I hadn't considered.

@acollard
Copy link

acollard commented Feb 9, 2015

@christopherthielen If this was built into the framework you wouldn't need the '$stateChangeStart' event hookup and you could fully support injectable services.

Personally I prefer "redirectTo" over "default". Since redirect better explains what is happening. "Default" could be useful for another function anyways.

My suggestion is to support something like this.

.config(['$stateProvider', '$urlRouterProvider',function($stateProvider, $urlRouterProvider) {
    $stateProvider
        .state('Account.Security', {
          url: '/security',
          redirectTo: 'Account.Security.ChangePassword', // OR
          redirectTo: ['$state', 'authenticationService', function($state, authenticationService){ 
            var currentUser = authenticationService.user();
            if(currentUser)
               $state.go('Account.Security.ChangePassword', { userId: currentUser.id });
            else
               $state.go('Account.Login');
            }]
        });
}]);

This is obviously a contrived example, I wouldn't put the userId in a stateParam. It would be pulled in as a resolve on the Account.Security.ChangePassword state.

@getvega
Copy link

getvega commented Feb 9, 2015

Great suggestions @christopherthielen & @acollard .

+1 on redirectTo over default. default would lead people into thinking that it's a flag on children, not on parent (childState.default = true)

But I tend to prefer @christopherthielen's usage ({state: XX, params: YY}). It provides more guidelines on how and when it should be used. Imho @acollard's would be a bit too generic and could be mixed up with onEnter...

@acollard
Copy link

acollard commented Feb 9, 2015

@getvega My thinking is it would support both string and function similar to ngRoute configuration (ex template: {string=|function()=}). But I like @christopherthielen idea for including the params too.

So maybe redirectTo could be 3 different options:

  • string: 'Account.Security.ChangePassword'
  • object: { state: 'Account.Security.ChangePassword', params: 123 }
  • function: ['$state', function($state){}] or function(){}

I agree with your point about the function being too close to onEnter. But my reason for keeping it is I would like redirectTo support on abstract states and onEnter shouldn't fire for an abstract state in my opinion.

@christopherthielen
Copy link
Contributor

I'd like some comments on 1) what meaning does abstract have? Should we should return URL for a state with a default substate, and if so, what should it look like? 2) preferred syntax for declaring default substates

what meaning does abstract have? how does this affect URL generation for abstract-states-with-default-substate?

Marking a state abstract states simply means that the state cannot be directly activated and its navigable reference is not filled in.

With default-substate redirect, it would be OK to transition directly to an abstract state $state.go(abstractstate), although the state still cannot be activated directly (due to the redirect). However, there would be no appreciable difference in behavior between an abstract state with redirect and regular state with redirect.

Abstract states currently have their navigable property undefined. This also means they do not get a URL calculated (for instance in ui-sref).

An abstract state with default substate could potentially have navigable filled in with the default substate/params. This is easy to implement, as long as the default substate is statically defined. Then, if a ui-sref to an abstract state is generated, the href would point to the substate.

However, if we implemented default-substates as a function (as proposed by both @acollard and @getvega), then maintaining an abstract state's navigable reference is overhead we wouldn't want. I agree with them both that dynamic calculation of the default substate should be allowed.

Therefore, a simpler option regarding the abstract state navigable problem is to set the abstract state's navigable reference to itself, just like a normal state. The href generated for a ui-sref link to one of these states would be the abstract state's href, and the redirect would happen dynamically. This makes the most sense to me, but I'd like to hear thoughts.

Back to my first thought, this would make the behavior of abstract states with redirect basically equivalent to a normal state with redirect. So, logically I wonder if we want to couple redirect logic with abstract at all.

preferred syntax for declaring default substates

Let's think about some conventions.
Here are some possibilities:

  1. { name: :"foo", abstract: "foo.bar" }
  2. { name: :"foo", abstract: true, redirect: "foo.bar" }
  3. { name: :"foo", redirect: "foo.bar" }
  4. { name: :"foo", redirectTo: "foo.bar" }
  5. { name: :"foo", default: "foo.bar" }
  6. { name: :"foo", redirect: { state: "foo.bar", params: { param1: 123 } }
  7. { name: :"foo", redirect: function injectableRedirectFn() { return { state: "foo.bar", params: { param1: 123 } }; } }

  1. couple default-substate with abstract declaration. meh.
  2. Mark state abstract, also declare a redirect. (abstract now serves no purpose, if we set navigable to itself)
  3. Declare redirect substate
  4. Declare redirectTo substate
  5. Declare default substate (default is a reserved word in JS, IE8 doesn't allow it as a literal object key)
  6. Same as 3, but an object is used with state and params keys
  7. An injectable function returns an object that looks like 6

I think I want to support 3, 6, and 7.

@christopherthielen
Copy link
Contributor

You guys are quick 👍 I hadn't even typed my questions up yet!

Paging @ProLoser and @nateabele

@acollard
Copy link

acollard commented Feb 9, 2015

@christopherthielen I'm with you on 3,6,7 per my last post. I prefer the name 'redirectTo' over just 'redirect'. More inline with ngRoute. To help all those people who move to ui-router from ngRoute...:smile:

@christopherthielen
Copy link
Contributor

@acollard fair enough. Truth is, I've never even looked at ngRoute, so I had no idea redirectTo was a thing.

@ProLoser
Copy link
Member Author

ProLoser commented Feb 9, 2015

Honestly, food for thought: what does having an 'abstract' flag gain you?
It's mostly a note-to-self for developers, and a slight improvement in
overhead (since their URLs aren't cached). Let me ask you this, if you did
away with abstract states entirely, what would you lose? Absolutely nothing
more than a little bit of overhead. In fact, it'd be entirely backwards
compatible.

I am pretty sure I had somewhere I was going with this...

I agree having abstract + redirect doesn't make sense, but I was thinking
in the big scheme of things, does even having abstract whatsoever even make
sense (since you could make essentially any state have the same
responsibilities as an 'abstract' one).

On Mon Feb 09 2015 at 9:07:55 AM Chris Thielen [email protected]
wrote:

@acollard https://github.com/acollard fair enough. Truth is, I've never
even looked at ngRoute, so I had no idea redirectTo was a thing.


Reply to this email directly or view it on GitHub
#1235 (comment)
.

@christopherthielen
Copy link
Contributor

@ProLoser That's my line of thinking too.

I think lots of people assume abstract has more semantics than it actually does, adding to the confusion. The one useful scenario I might continue to use it for is having a parent state which adds resolves and such to the state tree, but where redirection doesn't apply (such as a pseudo-root auth state, etc).

Once you mix in redirect, however, it becomes nothing more than a marker, as you mentioned.

@inca
Copy link

inca commented Feb 9, 2015

@christopherthielen 👍 on this scenario; sometimes we use abstract states only to define a common layout and context highlighting (auth is really good example). The whole idea of abstract states is pretty natural if you ask me.

I vote for keeping the abstract flag and adding the redirectTo (367 look good).

@raphaelpereira
Copy link

@christopherthielen I go with @inca

Keep abstract flag and add redirectTo or redirect as mentioned by you

@eddiemonge
Copy link
Contributor

@inca so you would have a state that you can never go to and it never redirects? That would make things like ui-sref harder methinks

@nalroff
Copy link

nalroff commented Feb 24, 2015

In the spirit of the parameter discussion, I also used to use something like this that was broken in 0.2.13 and am unsure how to work around it:

$urlRouterProvider.when(/^/users/edit/([0-9]+)$/i, '/users/edit/$1/personal');

Since the regular expression allowed me to pass the url parameter along to the next state, I'm unsure how to proceed with workarounds that are state-centric.

I realize also that this logic could be done using $stateParams inside the parent controller code, but it has some logic in it that I'd rather not run twice.

@christopherthielen
Copy link
Contributor

@nalroff Doesn't state-based redirection work for that use case?

.state('users.edit', { 
  url: '/edit/:id'
  redirectTo: 'users.edit.personal'
}

See this comment: #948 (comment)

@nalroff
Copy link

nalroff commented Feb 24, 2015

@christopherthielen That's perfect. Thanks! Not sure how I missed that.

@inca
Copy link

inca commented Feb 27, 2015

@eddiemonge Sorry, missed your comment (didn't receive an email).

Yup, just like @christopherthielen mentioned an abstract /auth context which can exist just for layout purposes, but does not actually expose "default" endpoints.

Another question arises, however: say, state with provides resolves and controllers, but redirects to non-inherited state. Will they be available in a state you've been redirected too? Or state redirection is implemented separately from inheritance?

@christopherthielen
Copy link
Contributor

@inca the concepts are separate. A redirect is essentially a totally different transition starting from the same "from" state. The redirect happens before resolves are even processed on the "redirecting state".

@inca
Copy link

inca commented Mar 12, 2015

Hate asking, but is there a milestone or pre-release which includes this fix? I desperately need it :)
Please forgive my nuisance :)

@acollard
Copy link

@inca I can't speak to when this feature will be added. But in the meantime you could use the implementation I described in #1584.

We've been using this for a while and it works great.

module.run(['$rootScope', '$state', '$injector', function ($rootScope, $state, $injector) {
    $rootScope.$on('$stateChangeStart',function (event, toState, toParams) {
      var redirect = toState.redirectTo;
      if (redirect) {
        if (angular.isString(redirect)) {
          event.preventDefault();
          $state.go(redirect, toParams);
        }
        else {
          var newState = $injector.invoke(redirect, null, { toState: toState, toParams: toParams });
          if (newState) {
            if (angular.isString(newState)) {
              event.preventDefault();
              $state.go(newState);
            }
            else if (newState.state) {
              event.preventDefault();
              $state.go(newState.state, newState.params);
            }
          }
        }
      }
    });

@nateabele
Copy link
Contributor

Closing this in the interest of consolidating discussion on #27 (but will keep it around for reference).

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

No branches or pull requests