DRY up logic in view as well as $routing

Question

I'm still learning Angular and have started to create a simple website. I have routing and some logic in my view to have an active class on the menu item when the url is on that page. The problem I am starting to see is if I change the url from / to /home I have to update it in the routing and view and also the logic to see if it should be active.

I was wondering is there a better way of doing this to make it DRY?

My app.js file looks like this:

    angular.module('simpleApp', ['ngRoute'])
  .config(['$routeProvider', '$locationProvider', function ($routeProvider, $locationProvider) {
    $routeProvider
      .when('/', {
        templateUrl: 'views/home/index.html'
      })
      .when('/about', {
        templateUrl: 'views/about/index.html',
        controller: 'AboutController',
        controllerAs: 'aboutCtrl'
      })
      .when('/services', {
        templateUrl: 'views/services/index.html',
        controller: 'ServicesController',
        controllerAs: 'servicesCtrl'
      })
      .when('/contact', {
        templateUrl: 'views/contact/index.html',
        controller: 'ContactController',
        controllerAs: 'contactCtrl'
      })
      .otherwise({
        redirectTo: '/'
      });
  }]);

and partial view of my index.html looks like this:

<nav class="navbar navbar-inverse" ng-controller="NavController as navCtrl">
          <div class="container-fluid">
            <!-- Brand and toggle get grouped for better mobile display -->
            <div class="navbar-header">
              <button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#bs-example-navbar-collapse-1" aria-expanded="false">
                <span class="sr-only">Toggle navigation</span>
                <span class="icon-bar"></span>
                <span class="icon-bar"></span>
                <span class="icon-bar"></span>
              </button>
              <a class="navbar-brand" href="/#/">SimpleApp</a>
            </div>
            <div class="collapse navbar-collapse" id="bs-example-navbar-collapse-1">
              <ul class="nav navbar-nav">
                <li ng-class="{ 'active': navCtrl.isActive('/')}">
                  <a href="/#/">Home</a>
                </li>
                <li ng-class="{ 'active': navCtrl.isActive('/about')}">
                  <a href="/#/about">About</a>
                </li>
                <li ng-class="{ 'active': navCtrl.isActive('/services')}">
                  <a href="/#/services">Services</a>
                </li>
                <li ng-class="{ 'active': navCtrl.isActive('/contact')}">
                  <a href="/#/contact">Contact Us</a>
                </li>
              </ul>
            </div>
          </div>
        </nav>

as you can see I call the isActive method on the nav controller but I have to always make sure that url I pass in is correct.

this is my nav controller:

    angular.module('simpleApp')
  .controller('NavController', function($scope, $location) {
    this.isActive = function (url) {
      return url === $location.path();
    };
  });

possible duplicate of [Set active tab style with AngularJS](http://stackoverflow.com/questions/12295983/set-active-tab-style-with-angularjs) — Long Nguyen, Aug 20 '15 at 18:13
I don't think its a duplicate as the question was answered with how I've written my code already. My question is how to make it dry. All be it one of the answers is to use a custom directive so will have a look at it. so thankyou for bringing this to my attention as I didn't know what to google/search on here to make sure I wasn't duplicating anything — saunders, Aug 20 '15 at 19:06
I was pointing at the accepted answer which is the best way to do it. Requires no extra directives or methods. You simply add a custom attribute to your route like so `.when('/', { templateUrl: 'views/home/index.html', activetab: 'home' })` and then in your html you can do this `
I am kind of doing that at the moment but its having to change it in two places where I'm trying to only change it in one place — saunders, Aug 20 '15 at 19:39
Ah I see, well then the directive is probably your best bet. I actually prefer the ui-router module instead of the ngRoute module for my routing. Its far more powerful and already has a built in directive to handle active tabs among many other things. Its definitely worth a look if you haven't already. — Long Nguyen, Aug 20 '15 at 19:42
I have been told its better and it is on my list to look at. but I thought it would be good to get a hand on the ngRoute to start with and then move on — saunders, Aug 20 '15 at 19:45
ui-router is basically ngRoute with more functionality. By learning ui-router you will also be able to work with ngRoute because the basics are the same. Up to you but it will probably save you a lot of time and headaches if you switch earlier :) — Long Nguyen, Aug 20 '15 at 19:47
okay cool. i'll move over now and try to figure it all out. thanks :) — saunders, Aug 20 '15 at 19:55
Rolled back because you shouldn't put the answer in your question — Jan, Aug 21 '15 at 15:27

score 1 · Answer 1 · answered Aug 20 '15 at 22:22

1

If you don't want to repeat yourself, why not just use an ng-repeat?

<ul class="nav navbar-nav">
  <li ng-repeat="item in navbarLinks" ng-class="{ 'active': navCtrl.isActive(item.link)}">
     <a href="/#{{item.link}}">{{item.text}}</a>
  </li>
</ul>

I mean, not having to repeat yourself is why it exists. So you don't repeat yourself. ng-repeat actually is really good for those cases when you don't want to repeat yourself. If you want to avoid repeating yourself, I'd suggest you use it. And by "it" I mean ng-repeat. To avoid repitition.

answered Aug 20 '15 at 22:22

Jan

5,688
3
27
44

This is actually quite a nice idea. I did fix it by using ui router but this is also a good method and nice to know. – saunders Aug 21 '15 at 06:24
@Jan Hi, I have one doubt. This code of yours is calling navCtrl.isActive function for all items. So let's say I have 50 items in navbarLinks, then isActive function is getting called 50 items. Is this elegant? – overlord Nov 29 '16 at 15:45
@overlord Well, it depends. What would the alternative be, to set a property on each navbar item? Then that property would need to be updated everywhere that the route could change. Would that be elegant? It depends on the rest of the code. – Jan Dec 09 '16 at 18:33

score 0 · Answer 2 · answered Aug 20 '15 at 17:59

0

Check out the ng-class directive.

https://docs.angularjs.org/api/ng/directive/ngClass

Basically you'd have an ng-class reference on your menu items that uses the expression portion to compare the menu item to the active item and then when that expression evaluates to true, the class will be applied to that item.

answered Aug 20 '15 at 17:59

Mike Feltman

5,160
1
17
38

Yes, not sure how I missed that before. I'm going to add a better answer below. – Mike Feltman Aug 20 '15 at 18:30
Okay no worries. Did your second answer get cut off? – saunders Aug 20 '15 at 19:04
Yes it did. I'm a nOOb here. I didn't realize that I needed a tag around it. – Mike Feltman Aug 20 '15 at 20:32

Mike Feltman · Answer 3 · 2015-08-20T20:31:42.660

0

This would be a good place for a custom directive. The directive could render the html just as it is above, but the actual html in your document would look something like <myNav url='about'>.

edited Aug 20 '15 at 20:31

answered Aug 20 '15 at 18:31

Mike Feltman

5,160
1
17
38

I have edited my question to use a directive but because I use / as my home page it doesn't make the home link active. any ideas? – saunders Aug 20 '15 at 19:33
That's pretty impressive work there. I believe you'd just have to check within the code for the blank path and when it's found, set the class on the home link. You could probably add an optional parameter to your directive that you only set on the home link to indicate it's the one to get the active class when the URL is simply /. – Mike Feltman Aug 20 '15 at 20:37
Thanks I did figure it out in the end but moved over to ui router which does this all for me. – saunders Aug 20 '15 at 20:43
Can you post the current HTML, just for completeness? I actually think I'm going to have a need to do something similar in a few days. – Mike Feltman Aug 20 '15 at 20:45
yer sure. I'll update my answer with the full html and directive – saunders Aug 20 '15 at 20:46

saunders · Accepted Answer · 2015-08-20T20:49:51.157

I finally figured out what I needed to do from the possible duplicate question but slightly changed it to work with the '/' url.

This is my index.html

<!DOCTYPE html>
<html>
<head>
  <title>Simple Angular App</title>
  <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.5/css/bootstrap.min.css">
  <link rel="stylesheet" type="text/css" href="css/styles.css">
</head>
  <body ng-app="simpleApp">
    <header>
      <div class="container">
        <nav class="navbar navbar-inverse">
          <div class="container-fluid">
            <!-- Brand and toggle get grouped for better mobile display -->
            <div class="navbar-header">
              <button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#bs-example-navbar-collapse-1" aria-expanded="false">
                <span class="sr-only">Toggle navigation</span>
                <span class="icon-bar"></span>
                <span class="icon-bar"></span>
                <span class="icon-bar"></span>
              </button>
              <a class="navbar-brand" href="/#/">SimpleApp</a>
            </div>
            <div class="collapse navbar-collapse" id="bs-example-navbar-collapse-1">
              <ul class="nav navbar-nav">
                <li>
                  <a href="#/" detect-active-tab="1">Home</a>
                </li>
                <li>
                  <a href="#/about" detect-active-tab="1">About</a>
                </li>
                <li>
                  <a href="#/services" detect-active-tab="1">Services</a>
                </li>
                <li>
                  <a href="#/contact" detect-active-tab="1">Contact Us</a>
                </li>
              </ul>
            </div>
          </div>
        </nav>
      </div>
    </header>
    <main>
      <div class="container">
        <div class="row">
          <div ng-view></div>
        </div>
      </div>
    </main>
    <footer>
      <div class="container">
        <div class="row">
          <div class="col-xs-12">
            <p class="footer-text">First attempt at an AngularJs site</p>
          </div>
        </div>
      </div>
    </div>
  </footer>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/2.1.4/jquery.min.js"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/3.3.5/js/bootstrap.min.js"></script>
    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.4.4/angular.js"></script>
    <script src="https://code.angularjs.org/1.4.4/angular-route.min.js"></script>

    <script src="js/app.js"></script>

    <script src="js/directives/shared/navdirective.js"></script>

    <script src="js/controllers/contact/ContactController.js"></script>
    <script src="js/controllers/about/AboutController.js"></script>
    <script src="js/controllers/services/ServicesController.js"></script>
  </body>
</html>

this is my app.js:

angular.module('simpleApp', ['ngRoute'])
  .config(['$routeProvider', '$locationProvider', function ($routeProvider, $locationProvider) {
    $routeProvider
      .when('/', {
        templateUrl: 'views/home/index.html'
      })
      .when('/about', {
        templateUrl: 'views/about/index.html',
        controller: 'AboutController',
        controllerAs: 'aboutCtrl'
      })
      .when('/services', {
        templateUrl: 'views/services/index.html',
        controller: 'ServicesController',
        controllerAs: 'servicesCtrl'
      })
      .when('/contact', {
        templateUrl: 'views/contact/index.html',
        controller: 'ContactController',
        controllerAs: 'contactCtrl'
      })
      .otherwise({
        redirectTo: '/'
      });

      // if(window.history && window.history.pushState) {
      //   $locationProvider.html5Mode({
      //     enabled: true,
      //     requireBase: false
      //   });
      // }
  }]);

and this is my navdirective file:

angular.module('simpleApp')
  .directive('detectActiveTab', function ($location) {
    return {
      restrict: 'A',
      link: function postLink(scope, element, attrs) {
        scope.$on("$routeChangeSuccess", function (event, current, previous) {
            // This var grabs the tab-level off the attribute, or defaults to 1
            var pathLevel = attrs.detectActiveTab || 1;

            if($location.path() === '/') {
              var pathToCheck = $location.path() ||
                  "current $location.path doesn't reach this level";
              var tabLink = attrs.href.split('#')[pathLevel] ||
                  "href doesn't include this level";
            } else {
              var pathToCheck = $location.path().split('/')[pathLevel] ||
                  "current $location.path doesn't reach this level";
                  // This var finds grabs the same level of the href attribute
              var tabLink = attrs.href.split('/')[pathLevel] ||
                  "href doesn't include this level";
            }

            if (pathToCheck === tabLink) {
              element.parent('li').addClass('active');
            }
            else {
              element.parent('li').removeClass('active');
            }
        });
      }
    };
  });

to use this I added detect-active-tab="1" to my a tag and then included the directive to my index.html

I have also had a look into ui router and it has made things a lot simpler as the routing is setup very similar to how ngRoute is set up but all you do is add ui-sref="/services" to my links and ui-sref-active="active" to my li's and it works out the active state for me which is quite nice.

I don't see that your code has improved at all. Instead of a few characters of instantly readable code, you have a hairy monster of a function that's way too convoluted for its simple purpose. You've only succeded in making your code less readable and maintainable. If you're gonna do anything to reduce duplication, change your entire navbar to an `ng-repeat` based on an array of objects. — Jan, Aug 20 '15 at 22:17
That was the answer to my problem I first asked. I changed it to ui route like i said underneath which means i get rid of the directive and its much easier to read. — saunders, Aug 21 '15 at 07:00
I'm not sure adding 10 times more code and more complex than what you were doing should be the answer to anything... — Jan, Aug 21 '15 at 10:03
There was many ways to skin this cat. The solution I put up solved my problem but gave me a complex issue which is why I chose ui router. — saunders, Aug 21 '15 at 11:30
There are always many ways to skin a cat. You could have solved this using regexps or Perl or installing a DLL or... Finding the BEST way to skin the cat is what programming's about. If you add 10 times more code, add complexity, harm readability and maintainability AND as you say it introduced OTHER issues, then it's a Bad Way to skin the cat. So even though it may be an alternative answer to your question it should not be considered the answer to your problem. — Jan, Aug 21 '15 at 12:30
But like I said in my answer I moved to ui router which solved all the problems and made it better — saunders, Aug 21 '15 at 14:50
Yeah I'm talking specifically about the `detectActiveTab` directive from the answer, that implementation as it looks now should IMO not even be considered as an alternative. That's all I'm saying. If you solved it by using `UI-router`, then post that as an answer. Or if you want to highlight the directive method as well, post two separate answers. — Jan, Aug 21 '15 at 15:31

score 0 · Answer 5 · answered Aug 21 '15 at 17:56

Although my first answer did solve my original question. It gave me other issues with it being very complex and hard to maintain. I decided to switch to UI router which made things a lot easier.

my app.js file looks like this now:

angular.module('simpleApp', ['ui.router'])
  .config(function ($stateProvider, $urlRouterProvider) {
    $urlRouterProvider.otherwise('/');

    $stateProvider
      .state('/', {
        url: '/',
        templateUrl: 'views/home/index.html'
      })
      .state('/about', {
        url: '/about',
        templateUrl: 'views/about/index.html',
        controller: 'AboutController',
        controllerAs: 'aboutCtrl'
      })
      .state('/services', {
        url: '/services',
        templateUrl: 'views/services/index.html',
        controller: 'ServicesController',
        controllerAs: 'servicesCtrl'
      })
      .state('/contact', {
        url: '/contact',
        templateUrl: 'views/contact/index.html',
        controller: 'ContactController',
        controllerAs: 'contactCtrl'
      });
  });

and my html looks like this:

    <!DOCTYPE html>
<html>
<head>
  <title>Simple Angular App</title>
  <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.5/css/bootstrap.min.css">
  <link rel="stylesheet" type="text/css" href="css/styles.css">
</head>
  <body ng-app="simpleApp">
    <header>
      <div class="container">
        <nav class="navbar navbar-inverse">
          <div class="container-fluid">
            <!-- Brand and toggle get grouped for better mobile display -->
            <div class="navbar-header">
              <button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#bs-example-navbar-collapse-1" aria-expanded="false">
                <span class="sr-only">Toggle navigation</span>
                <span class="icon-bar"></span>
                <span class="icon-bar"></span>
                <span class="icon-bar"></span>
              </button>
              <a class="navbar-brand" href="/#/">SimpleApp</a>
            </div>
            <div class="collapse navbar-collapse" id="bs-example-navbar-collapse-1">
              <ul class="nav navbar-nav">
                <li ui-sref-active="active">
                  <a ui-sref="/">Home</a>
                </li>
                <li ui-sref-active="active">
                  <a ui-sref="/about">About</a>
                </li>
                <li ui-sref-active="active">
                  <a ui-sref="/services">Services</a>
                </li>
                <li ui-sref-active="active">
                  <a ui-sref="/contact">Contact Us</a>
                </li>
              </ul>
            </div>
          </div>
        </nav>
      </div>
    </header>
    <main>
      <div class="container">
        <div class="row">
          <div ui-view></div>
        </div>
      </div>
    </main>
    <footer>
      <div class="container">
        <div class="row">
          <div class="col-xs-12">
            <p class="footer-text">First attempt at an AngularJs site</p>
          </div>
        </div>
      </div>
    </div>
  </footer>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/2.1.4/jquery.min.js"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/3.3.5/js/bootstrap.min.js"></script>
    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.4.4/angular.js"></script>
    <script src="js/lib/angular-ui-router.min.js"></script>

    <script src="js/app.js"></script>

    <script src="js/controllers/contact/ContactController.js"></script>
    <script src="js/controllers/about/AboutController.js"></script>
    <script src="js/controllers/services/ServicesController.js"></script>
  </body>
</html>

All I had to do was add the reference to UI router in my index.html and change the config as per documentation for UI router. Then added ui-sref and ui-sref-active="active" to the li to make it active and the magic of UI router did the rest.

DRY up logic in view as well as $routing

5 Answers5