Skip to content
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

Due to fix in 0.2.13 .when('/parent', '/parent/child') is not working with ui-sref #1584

Closed
Radim-Kohler opened this issue Nov 26, 2014 · 45 comments
Assignees
Labels
Milestone

Comments

@Radim-Kohler
Copy link

I would say, that the _"fix"_ in 0.2.13, related to: Avoid re-synchronizing from url after .transitionTo, change: 19715d15 introduced a new issue. Or at least, the .when('/parent', '/parent/child') is not working for ui-sref="parent"

Description of the "issue"

As documented here: How to: Set up a default/index child state small extract:

Or if you want the 'parent.index' url to be non-empty, then set up a redirect in your module.config using $urlRouterProvider:

$urlRouterProvider.when('/parent', '/parent/index');
$stateProvider
    .state('parent', {url: '/parent', abstract: true, template: '<ui-view/>'} )
    // url '/parent/index'
    .state('parent.index', {url: '/index'} )

NOTE: in the above cite, I replaced the home in url with parent to make it more consistent

This has stopped working. There are plunkers:

Examples

There are examples of the issue with different versions:

this is the change applied in "roll back" of a fix 19715d15 0.2.13 inside of the released angular-ui-router.js

1988   ...
1989   // RKö - rollback of a fix
1990   // if (ignoreUpdate) return true;
1991    ...

Workaround without change of 0.2.13

I described how to go around here Angular UI-Router $urlRouterProvider .when not working when I click <a ui-sref=“…”>, we need eventing:

var onChangeConfig = ['$rootScope', '$state',
 function ($rootScope, $state) {

  $rootScope.$on('$stateChangeStart', function (event, toState) {    
    if (toState.name === "parent") { 
      event.preventDefault();
      $state.go('parent.index');
    }
  });

}]

Maybe this is the way to go (or please, give me better). But as mentioned below, not sure about consistency...

Question, is it an issue? or not

I can imagine that this issue would be refuesd as proper behaviour.

BUT, is it really what we want - if this will result in 2 different states:

<a ui-sref="parent">  // goes to parent
<a href="#/parent">    // goes to parent.index
@dmitriynosenko
Copy link

+1, I have the same issue.

@ashjmcfox
Copy link

+1

As mentioned, this prevents redirecting URLs/states when using ui-sref. This is unfortunate as it now means I cannot upgrade past 0.2.12 since I require this functionality. I don't think we should have to clutter our $stateChangeStart event just to handle this, especially if we have multiple URL/state redirects in place. Hopefully this issue can be fixed in 0.2.14.

@adestis-ds
Copy link

+2
I have a "real world" example showing the same issue:

http://plnkr.co/edit/xyVPZePL2eV0lp8HxPBG?p=preview

it works with 0.2.12 but nOT with 0.2.13. If the new behaviour is intended I would like to know what is the recommended way for setting default states.

@christopherthielen christopherthielen added this to the 0.2.14 milestone Nov 27, 2014
@christopherthielen christopherthielen self-assigned this Nov 27, 2014
@christopherthielen
Copy link
Contributor

People who are using .when this way: do you really want "state-based redirection"? .when is for matching a url and redirecting, and/or doing some other action. If this issue affects you, it means you are navigating using states, not url. I think what you are doing is navigating to a state (using state.go/ui-sref), but you really want to navigate to some other state (usually a substate).

In UI-Router the state is the primary focus, and the URL that maps to that state is secondary. In my eyes a state-based redirect is preferable to a url based one. Url redirection is more very loosely coupled and feels sloppy when compared to state-based redirection.

Read my analysis of the "double transition" problem here: #1573. As noted in the issue, avoiding re-sync is a safety net to avoid double-transitions. However, the primary fix is to ensure that all state-to-url mappings are bi-directional.

As such, since state based redirection using $stateChangeStart in 0.2.x is rather clumsy, I'm going to comment out the "avoid resync" logic for 0.2.14. I'm going to reintroduce it in 1.0 when we have a better state-level redirection API. If there are any other use cases that aren't "state-level redirection", please inform me so I make sure to account for them.

@rguldener
Copy link

+1 same problem

As for the use case: Better (i.e. non-central) state forwarding would be very welcome, also using the urlRouterProvider based forward for what really should be a state forward. The parent state is just a placeholder for us (which when called should redirect to a default child state) and we reference it in ui-sref to get "automagically" correct setting of the active class in the navigation. Since we have this use case in many places of our app we do not want to clutter the $stateChangeStart function with state redirects and thus opted to use $urlRouterProvider.

@RobiFerentz
Copy link

👍

Same issue!

@yaru22
Copy link

yaru22 commented Dec 10, 2014

If this issue affects you, it means you are navigating using states, not url. I think what you are doing is navigating to a state (using state.go/ui-sref), but you really want to navigate to some other state (usually a substate).
@christopherthielen

For me, the reason I navigate to a parent state rather than its substate directly is because of ui-sref-active.

For example, I have a side menu item associated with, let's say, app.settings state that needs to be highlighted (with .active class) when I go to any of its substates (e.g. app.settings.a, app.settings.b, app.settings.c). app.settings state is just a common state encompassing those substates. So my html looks like:

<div ui-sref-active>
  <a ui-sref="app.settings">Settings</a>
</div>

When I click on Settings link, I want it to redirect me to the first substate (i.e. app.settings.a ) automatically. If I put app.settings.a directly in ui-sref above, Settings menu item will only be highlighted when the app is in app.settings.a but not when it's in app.settings.b or app.settings.c.

Maybe there is a better way of using ui-sref-active that I'm not aware of? Another workaround is using ng-class with $state.includes(), but I was hoping to make it work with ui-sref/ui-sref-active alone.

So if I want to keep <a ui-sref="app.settings">Settings</a> and have it redirect me to app.settings.a, I need $urlRouterProvider.when() work as before.

If there is a better way to handle the case above, I'd like to know as well. Thanks.

@fubupc
Copy link

fubupc commented Dec 11, 2014

@yaru22 +1

Exactly the same scenario here!

@jeverden
Copy link

+1 Same issue

@squadwuschel
Copy link

+1

@corbanbrook
Copy link

+1

@lazychino
Copy link

👍 I have the same issue, I use it like @yaru22 for ui-sref-active and for defining the default subroute with ui-router-extras deep state redirect

@Zacharias3690
Copy link

+1

@yahyaKacem
Copy link

@christopherthielen another use case besides the one presented by @yaru22
3 main states home, profile and settings like this:
qq

The 2nd state profile have 2 nested states profile.about and profile.main
qqq

I need that the profile state clickable on the index page loads the profile view and redirects to the default nested state which is profile.about here
qqqq

Here's a working plunker with what I got so far, with the fix mentioned by @Radim-Kohler.

@ehacke
Copy link

ehacke commented Jan 14, 2015

👍

4 similar comments
@acollard
Copy link

+1

@ui-michal-szwed
Copy link

👍

@goddyZhao
Copy link

+1

@summer-liu
Copy link

+1

@gabrielmaldi
Copy link

In my case the underlying issue is in fact a feature request: we need a way to specify default child states.

Given:

$stateProvider
    .state("settings", {
        abstract: true,
        url: "/settings"
    })
        .state("settings.general", {
            url: "/general"
        })
        .state("settings.notifications", {
            url: "/notifications"
        })
        .state("settings.advanced", {
            url: "/advanced"
        })

It would be nice if we could say something like:

.state("settings.general", {
    url: "/general",
    isDefaultChild: true
})

So that when we navigate to the abstract parent state:

$state.go("settings")

We would get redirected (or even better: go directly) to settings.general.

I used to do this using when but it would certainly be nicer to have a "isDefaultChild" property on each state.

What do you think?

Thanks

@yahyaKacem
Copy link

Also one more thing to be aware of when using the fix mentioned by @Radim-Kohler is that the order of defining the data object in the state change so when using this solution the child state's data gets defined before the parent state's data so the parent overwrite the child's data (a more detailed explanation could be found in this bug report) so if you'r using something that require you to pass data to the child state with the data attribute in the state definition object you should be aware of this bug, for me I think the best solution here is to define all the data for children in the parent state until this gets fixed.
@gabrielmaldi that's correct this is more of a feature request then a bug, and +1 for your suggestion also other possibility is to put the default child name in the parent so instead of having isDefaultChild: true on a child state we'll have a defaultChild: "childname" on the parent state, I think both are valid solutions.

@cs-NET
Copy link

cs-NET commented Jan 24, 2015

@yahyaKacem I agree that @gabrielmaldi suggestion should be in the parent state and would like to expand on that idea by adding another option childSticky: true so that when navigation occurs to another parent the state provider will keep the previously active child as the default instead of resetting back to the childDefault: "childName".

@gabrielmaldi
Copy link

I didn't really give the proposed syntax a lot of thought, just wanted to outline the idea of default child states. Perhaps it would be better to have it in the parent state, or maybe someone comes up with a better way.

@cs-NET I think you should look into UI-Router Extras which has Sticky States and Deep State Redirect.

@violet-day
Copy link

+1

@killbirds
Copy link

+1

@getvega
Copy link

getvega commented Feb 4, 2015

@yahyaKacem like these redirections defined on the parent - much cleaner than have it outside à-la-urlRouterProvider.
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?

@yahyaKacem
Copy link

@getvega I'm actually doing that differently, I just inject the posts service into the run function then get the posts[0].id from there and form the parameters object in the run function then when I call $state.go I pass the params there:

var onChangeConfig = ['$rootScope', '$state', 'posts', function ($rootScope, $state, posts) {
   $rootScope.$on('$stateChangeStart', function (event, toState) {
     if (toState.name === "posts") {
       event.preventDefault();
       $state.go('posts.post', {id: posts[0].id});
    }
  });
}];

as for the defaultChildParams as you said not so clean maybe the defaultChild should be a string with child name in case no route params are needed and in case of params, object like {child: 'childName', params: {id: 0}}, where params could be a simple variable or a function that takes some injectables and returns the params object.

@acollard
Copy link

acollard commented Feb 4, 2015

@yahyaKacem This may make your code a little more reusable. 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.

@gabrielmaldi
Copy link

@getvega perhaps the redirection should just forward any parameters it got (e.g. from $state.go()) to the child state; that'd reduce the complexity.

@christopherthielen
Copy link
Contributor

@gabrielmaldi @acollard @yahyaKacem @getvega Great conversation. Let's move the discussion to #1235 to keep this one about url-based redirects

@christopherthielen
Copy link
Contributor

For 0.2.13, here is a very simple recipe you can use to redirect to a default substate:

  $stateProvider.state('profile' , {
      url: "/about",
      templateUrl: "profile.html",
      redirectTo: 'profile.about'
  });

 app.run($rootScope, $state) {
    $rootScope.$on('$stateChangeStart', function(evt, to, params) {
      if (to.redirectTo) {
        evt.preventDefault();
        $state.go(to.redirectTo, params)
      }
    });
  }

See it in action in a fork of the above plunkr that @yahyaKacem posted:

http://plnkr.co/edit/dcN7Qra6nY9ZTOGjgCvJ?p=preview

@VictorioBerra
Copy link

@acollard

How can I gain access to resolved items in my redirectTo function? It would make sense that id need to resolve an item from the database and the state of that item could effect what params or states I need to redirect to.

http://plnkr.co/edit/6opdJIQe09ULqZxiGZOZ?p=preview

@acollard
Copy link

acollard commented Mar 3, 2015

@LordWingZero

Accessing parent resolves is a little difficult, the process of running the resolves happens inside the $state.transitionTo and are not accessible through the event. Accessing the toStates resolves will likely not be possible at all, since redirectTo is processed before the toStates resolves are run.

But if you can use a registered service instead of a parent resolve then below will work for you.

We've changed our method above to support dependency injection. My code above has been changed to:

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);
            }
          }
        }
      }
    });

Here is a sample of using it:

.config(['$stateProvider', '$urlRouterProvider',function($stateProvider, $urlRouterProvider) {
    $stateProvider
        .state('Account.Security', {
          url: '/security',
          redirectTo: ['auth', function(auth){ return auth.isLicensed() ? { state: 'Account.Security.ChangePassword', params: 'some id' } : 'AccessDenied';}]
        });
});

The ui-router team is looking to add this functionality natively #1235. My guess is parent resolves will still be unavailable. RedirectTo will likely occur before any resolves are run. Thoughts @christopherthielen?

@VictorioBerra
Copy link

@acollard

How would you handle redirectTo if auth.isLicensed() returned a promise instead of a bool?

http://plnkr.co/edit/6opdJIQe09ULqZxiGZOZ?p=preview

@acollard
Copy link

acollard commented Mar 3, 2015

@LordWingZero

I'm assuming the promise from your redirectTo would still return one of the two options: the state name, or { state, param } object.

I would probably change the listener to this to support promises:

module.run(['$rootScope', '$state', '$injector', '$q', function($rootScope, $state, $injector, $q) {
        $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) {
                event.preventDefault();
                $q.when(newState).then(function(result) {
                  if (angular.isString(result)) {
                    $state.go(result);
                  } else if (result.state) {
                    $state.go(result.state, result.params);
                  }
                });
              }
            }
          }
        });

RedirectTo would look like this:

redirectTo: ['auth', function(auth){ return auth.isLicensed().then(function(isLoggedIn){ return isLoggedIn ? { state: 'Account.Security.ChangePassword', params: 'some id' } : 'AccessDenied';});}]

Alternatively you could use a resolve to do the redirect. We do this as well, which already supports promises and injecting resolves (parents and self).

$stateProvider.state({ 
    name: 'home', 
    url: '/home', 
    resolve: {
      loginRequired: ['$q','$state','auth', function($q, $state, auth){
       return auth.isLoggedIn()
        .then(function(isLoggedIn){
         if(!isLoggedIn) 
          {
             $state.go('login'); 
        }
       }
      }]
    }});

@VictorioBerra
Copy link

@acollard

What do you think of:

else if(angular.isFunction(newState.then)){
    event.preventDefault();
    newState.then(function(newStateData){
        $state.go(newStateData.state);
     });
}

That way it handles $promise objects (if using $resource you will need to return $promise probably. not sure)
It handles strings, and it handles objects with a .state property.

@acollard
Copy link

acollard commented Mar 3, 2015

@LordWingZero

My option above supports promises, strings, and objects. $q.when() is used to wrap all three options and return a promise. If a non-promise is wrapped then it will run then() immediately.

Checking for a promise yourself may not work well, I find it is easier to leave it up to angular.

@gwin003
Copy link

gwin003 commented Mar 17, 2015

Has this been resolved yet? I implemented @christopherthielen 's solution for now (thanks for that!), but it would be great if this got fixed.

@VictorioBerra
Copy link

@gwin003 it will be fixed with the next MAJOR release of ui-router. Almost everything will be different...

@gwin003
Copy link

gwin003 commented Mar 17, 2015

Thanks @LordWingZero

@franklin-ross
Copy link

@acollard

I'm liking your solution, but I added some code to support object literals like redirectTo: { state: "a.b", params: { id: 0 } }. Don't think it supported that case before?

        //Redirect to string
        } else if (redirect.state) {
          event.preventDefault();
          $state.go(redirect.state, redirect.params);
        } else {
        //Redirect to injectable

@christopherthielen christopherthielen modified the milestones: 0.2.15, 0.2.14 May 19, 2015
@christopherthielen
Copy link
Contributor

This missed 0.2.14, sorry. I'll get this into 0.2.15 ASAP

@christopherthielen
Copy link
Contributor

UI-Router 1.0 preview is available: https://github.com/angular-ui/ui-router/tree/feature-1.0

@Radim-Kohler
Copy link
Author

I admire that... you are magicians. I have to touch it, start to test... upgrade... The idea about migrating to Typescript is MUST. We are on Typescript for 1,5 year... cannot imagine JS development without it anymore. Looking forward experience with the mighty 1.0 ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests