angular-ui / ui-router

The de-facto solution to flexible routing with nested views in AngularJS
http://ui-router.github.io/
MIT License
13.54k stars 3k forks source link

ui-sref-active not working correctly with abstract and child states #1431

Closed blah238 closed 8 years ago

blah238 commented 9 years ago

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

In the example above, clicking the Administration nav element shows a view with two tabs, Users and Roles.

These correspond to the states admin.users and admin.roles, while admin is an abstract state whose URL is inherited by admin.users. This all works fine except when setting the active class using ui-sref-active.

There are two problems in this scenario:

  1. The Administration nav element is not made active when on the Roles tab
  2. When refreshing the page (be sure to pop out the Plunker preview) while on the Roles tab, no elements get the active class set

I would like to avoid using $state.includes directly since ui-sref-active is supposed to work on child states since 0.2.11: https://github.com/angular-ui/ui-router/pull/927

Am I doing something wrong or is this a bug?

stoykostanchev commented 9 years ago
     <li ui-sref-active="active"><a ui-sref="admin.users">Administration Panel</a>

The active class 'ui-sref-active' activates upon is not the abstract 'admin', but the 'admin.users' :? I guess the way this is suppose to be done is - making the admin non-abstract, and setting the url of the default child state to "" (however, THAT does not work for me for some reason, trying to figure out right now if it's me or an issue) Switching to urls based navigation would work, but seems to me not to be the right intent

blah238 commented 9 years ago

@stoykostanchev Making admin non-abstract fixes the first problem, but not the second, and unfortunately adds a new one: the users tab is made active when it's not. See plunker: http://plnkr.co/edit/swUNUcvBE1DaITgWSWtD?p=preview

I feel like this really should be working the way it is, and that it's not is a bug.

blah238 commented 9 years ago

Well here's a workaround that fixes both issues for me: http://plnkr.co/edit/G5o6XkeCQfQId2XwUTXc?p=preview

This creates a flat object containing the the state names and their active status (true/false) into the root scope at startup, and updates it when $stateChangeSuccess fires.

I needed to do this instead of using $state.includes() directly because of issues with ui-bootstrap: https://github.com/angular-ui/bootstrap/issues/1741 https://github.com/angular-ui/bootstrap/issues/1539

If anyone has any better ideas I'm all ears. I wish these two libraries (ui-router and ui-bootstrap) integrated with each other better.

blah238 commented 9 years ago

@timkindberg could you take a look at this since it looks like you did the PR for this? Thanks!

SerhiiKozachenko commented 9 years ago

+1

dcolley commented 9 years ago

+1 I can't get any classes to work in the partial/sidebar.html

epelc commented 9 years ago

+1 this would help alot

adamalbrecht commented 9 years ago

+1

I run into this issue a lot and, off the top of my head, here are a couple ideas for how it could be fixed:

First, you could add additional options to ui-sref-active so that you can specify a route rather than just using the route provided to ui-sref. Maybe something like this:

<a ui-sref='admin.users' ui-sref-active="{class: 'active', state: 'admin'}">Link</a>

Another way that might also be generally useful would be to allow linking to abstract states if it provides a default child state. Then you could do the following:

For example:

      // ...
      .state('admin', {
        url: '/admin',
        abstract: true,
        defaultChild: 'admin.users',
        templateUrl: 'templates/adminPanel.html'
      })
      .state('admin.users', {
        url: '',
        templateUrl: 'templates/adminUsers.html'
      })
      // ...
<a ui-sref="admin" ui-sref-active="active">Administration Panel</a>
tagazok commented 9 years ago

I just had the same problem. In my app, I have an abstract 'users' state, and my menu link goes to users.list but then when selecting a user and going to users.details, I lost the ui-sref-active :/

I like the idea to have an object intp ui-sref-active to specify the associated state :)

epelc commented 9 years ago

@tagazok If your having the problem with a list/detail setup one way to get around it is to make your details state a child of your list state. This worked very well for me but it requires a little extra logic depending on if your details requires a second api call or if you already have the data from your list. You end up with users as an abstract state and users.list along with users.list.details so if you link to users.list then go to users.list.details you ui-sref-active will still work.

My problem is I have a settings state with multiple categories ie overview billing security teams states each with their own child states. So my above solution doesn't go very well with the above solution.

SerhiiKozachenko commented 9 years ago

@tagazok I have fixed this in my app by creating custom menu directive in which I have isActive func :

angular.module('Teach4Tech.Admin.Directives.Menu', [])
.directive('menu', [function(){
 return {
      restrict: 'A',
      template: '<li ng-repeat="item in menuItems" ng-class="{active: isActive(item.root)}"><a ui-sref="{{item.state}}"><span class="{{item.iconClass}}"></span>&nbsp;&nbsp;{{item.title}}</a></li>',
      controller: ['$scope', '$state', function ($scope, $state) {
        $scope.menuItems = [{
            title: 'Statistics',
              state: 'statistics',
              root: 'statistics',
            iconClass: 'glyphicon glyphicon-stats'
          }, {
              title: 'Videos',
                state: 'videos.list',
                root: 'videos',
            iconClass: 'glyphicon glyphicon-film'
          }, {
              title: 'Articles',
                state: 'articles',
                root: 'articles',
            iconClass: 'glyphicon glyphicon-pencil'
          }, {
              title: 'Courses',
                state: 'courses',
                root: 'courses',
            iconClass: 'glyphicon glyphicon-list-alt'
          }, {
              title: 'Messages',
                state: 'messages',
                root: 'messages',
            iconClass: 'glyphicon glyphicon-comment'
        }];

        $scope.isActive = function(root) {
          return $state.includes(root);
        };
      }]
    };
}]);
tagazok commented 9 years ago

thanks @Grievoushead this is exactly what I needed ! :)

squadwuschel commented 9 years ago

+1 Same Problem

nealtovsen commented 9 years ago

+1

rweng commented 9 years ago

+1, also like the idea of linking to abstract state if it provides a default child

tomwganem commented 9 years ago

+1

neaped commented 9 years ago

I have got the similar problem, what I have done:

Don't use abstract within state, coz ui-sref-active support nest child state, so I use $state.go('child state') within the controller to force the parent state to go to it's child. But this will bring up the other problem, parent state's active class will never gone :( this ui-sref-active-eq won't work coz I don't give child state specific url.

lucky7id commented 9 years ago

:+1: +1

f2net commented 9 years ago

+1 for the proposed form: ui-sref-active="{class: 'active', state: 'admin'}"

tristanz commented 9 years ago

+1

ghoullier commented 9 years ago

:+1:

jaman1020 commented 9 years ago

+1 on this, but in the meantime I've worked out a somewhat hacky solution that may or may not work for your use cases. ui-sref-active works with child states, but not 'sister' states, such as when you have two tabs. What I've done is converted the second tab to a child state, but changed the same views by using absolute view targeting. So -

    .state('user.home.main', {
       views: {
             view1@user.home: {}
             view2@user.home: {}
        }
    }
    .state('user.home.main.secondTab'. { // - because its part of user.home.main it retains state
      views: {
             view1@user.home: {}
             view2@user.home: {}
        }
    }

Again, depending on what you use your states for, this may or may not work for you, but at least it lets you preserve active state natively without having to create a directive or anything else.

santyclaz commented 9 years ago

+1

ruchevits commented 9 years ago

I use the following in my application:

$scope.menu = [
    {
        title: 'Home',
        state: 'home'
    },
    {
        title: 'Categories',
        state: 'category'
    },
    {
        title: 'Category One',
        state: 'category.one',
        root: 'category'
    },
    {
        title: 'Category Two',
        state: 'category.two',
        root: 'category'
    }
];
$scope.isActive = function(root) {
    return $state.includes(root);
};
<li ng-repeat="item in menu" ng-class="{active: isActive(item.root || item.state)}">
  <a ui-sref="{{item.state}}">{{item.title}}</a>
</li>

So, if root is not defined for a state, we just fall back to default behaviour.

almandot commented 9 years ago

+1 for either specifying the state for ui-sref-active or linking to abstract states if a default child is set... or both!

hdaws commented 9 years ago

+1 Just hit this need myself in an application where I need to be "active" on an abstract parent. Reordering my hierarchy has other large implications that I can't back out of in a timely fashion right now.

MichaelJCole commented 9 years ago

+1 @blah238 thanks for a simple solution.

uguryilmaz commented 9 years ago

hi, i have this exect problem. i have a sidebar menu with 3 level deep.

System User and Security Sample Form General Definitions Currency Country

i use states for routing. when i click Country menu item. it shows country page as expected. Country menu item highlited. no problem. then, when i click currency menu item, it show currency page but menu collapsed up to root. i analysed currency menu item via console, it has active class. i think parent elements lose their states somehow. i tried to create a plunker. i know menu doesnt collapsed at the beginning but look at the code side. it is the same code i use in my current project.

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

here is some screenshot:

first i click Ülke (Country) item from menu. it works normally. image

and then i clicked Para Birimi (Currency) item it shows currency index page but menu collapsed up to root.

image

budkin76 commented 9 years ago

+1. I'm running into issues as my parent state is abstract but I need it to be active for all its children.

budkin76 commented 9 years ago

Also, @blah238 thank you for that workaround! Using that for now.

sharpmachine commented 9 years ago

An easy workaround is just put your first view in the ui-view like so:

<!-- list.html -->
<div ui-view class="animated fadeIn">
  <toolbar></toolbar>
  <child-layout>
    <charge-card ng-if="vm.charges" ng-repeat="charge in vm.charges" dynamic-popover="vm.dynamicPopover" charge="charge" show-confirm="vm.showConfirm"></charge-card>
    <div ng-show="!vm.charges.length">There are no charges for {{$parent.$parent.vm.subject.basicInfo.firstName}}. Add charges by clicking the plus button.</div>
  </child-layout>
</div>

<!-- edit.html -->
<toolbar></toolbar>
<child-layout>
  <formly-form model="vm.charge" fields="vm.chargeForm">
    <button class="btn btn-primary btn-raised" ng-click="vm.updateCharge()">Save Changes</button>
    <button class="btn btn-inverse btn-inverse-danger" back-button>Cancel</button>
    <button class="btn btn-sm btn-danger pull-right" ng-click="vm.showConfirm($event)"><i class="fa fa-trash"></i></button>
  </formly-form>
</child-layout>

<!-- link to charges list -->
<li role="presentation" ui-sref-active="active"><a ui-sref="subject.charges({subjectId: subject.id})">Charges <paper-ripple fit></paper-ripple></a></li>
//Routing
// GET /subjects/1/charges
  .state('subject.charges', {
    url: '/charges',
    templateUrl: 'components/charges/list.html',
    controller: 'ChargesListCtrl',
    controllerAs: 'vm',
    resolve: {
      charges: function(Subjects, $stateParams) {
        return Subjects.one($stateParams.subjectId).getList('charges');
      },
      charge: function(Charges, $stateParams) {
        return Charges.one($stateParams.chargeId).get();
      }
    }
  })

  // GET /subjects/1/charges/1
  .state('subject.charges.edit', {
    url: '/:chargeId',
    templateUrl: 'components/charges/edit.html',
    controller: 'ChargeEditCtrl',
    controllerAs: 'vm',
    resolve: {
      charge: function(Charges, $stateParams) {
        return Charges.one($stateParams.chargeId).get();
      }
    }
  })

All your child views will still go through the original ui-view in the list.html file and your navigation actives will still be true whether you're on the parent or the child.

mykyta-shulipa commented 9 years ago

+1. Also having thins issue

eric-norcross commented 9 years ago

+1.

Building on @stoykostanchev's solution; it would be great if it could also support wildcards similar to $state.includes()

For instance:

<a ui-sref='admin.users' ui-sref-active="{class: 'active', state: 'admin.*'}">Link</a>

Which, I feel would be more clear than writing:

 <a ui-sref='admin.users' ng-class=" $state.includes('admin.*') ? 'active' : null">Link</a>
simmu commented 9 years ago

+1. I like @eric-norcross suggestion on the wildcards support. My temporary work around is to maintain the ui-sref pointing to the abstract state and use ng-click to disabled the default behavior and use $state.go to the default child view.

wlkns commented 9 years ago

+1

I fixed this using a custom directive:

app.directive('uiSrefActiveIf', ['$state', function($state) {
    return {
        restrict: "A",
        controller: ['$scope', '$element', '$attrs', function ($scope, $element, $attrs) {
            var state = $attrs.uiSrefActiveIf;

            function update() {
                if ( $state.includes(state) || $state.is(state) ) {
                    $element.addClass("active");
                } else {
                    $element.removeClass("active");
                }
            }

            $scope.$on('$stateChangeSuccess', update);
            update();
        }]
    };
}])

Then you use matching HTML ui-sref-active-if="parent" will add the class "active"

ailling commented 9 years ago

Thanks @arkin; works perfectly for me

domalaq commented 9 years ago

+1 to @arkin-

jancel commented 9 years ago

+1 @arkin Wonder if they could bring this in. Simple to integrate

@arkin You may want to add the $state injection.

...
['$scope', '$element', '$attrs', '$state', function ($scope, $element, $attrs, $state) {
...
alonronin commented 9 years ago

@jancel he injected it to the directive callback

jancel commented 9 years ago

Ah. Great. Thanks. I missed that one. Anyway, another +1.

On Monday, August 10, 2015, Alon Valadji notifications@github.com wrote:

@jancel https://github.com/jancel he injected it to the directive callback

— Reply to this email directly or view it on GitHub https://github.com/angular-ui/ui-router/issues/1431#issuecomment-129599583 .

Jeff Ancel

(314) 703-8829 - Main

www.jeffancel.com

hiquest commented 9 years ago

@arkin- thanks! I was about to write something like this myself, but you saved me the time!

sharpmachine commented 9 years ago

+1 @arkin- you're a genius. If I could I would buy you a water.

wlkns commented 9 years ago

You are all welcome ^ :)

If this is something the ui-router team want to implement, I'll happily submit a pull request.

Luddinus commented 8 years ago

@arkin- great!

celsomtrindade commented 8 years ago

@arkin- Thanks!! =D

jessicard commented 8 years ago

@sharpmachine shit like that makes women not want to contribute to open source

CoralineAda commented 8 years ago

The statement by sharpmachine is disgusting.

sharpmachine commented 8 years ago

@jessicard @CoralineAda You're right. I'm sorry. I was just excited kuz it took me forever to find a solution. I've edited the comment.

jessicard commented 8 years ago

@sharpmachine thank you!

CoralineAda commented 8 years ago

https://github.com/angular-ui/ui-router/pull/2309