1
\$\begingroup\$

I've a sample code to create a simple menubar here which works, but I'm not sure if that's the best way to code it. Can somebody please validate if there is a better way to do the same?

In short

  1. I've create a sample page with a directive called <menu-bar>

    <menu-bar></menu-bar>
    
  2. The directive merely points to a template code and a controller

    app.directive("menuBar", function () {
      return {
        restrict: 'E',
        templateUrl: 'main-nav.html',
        controller: 'navController',
        controllerAs: 'nc'
      }
    });
    
  3. The controller loads the menubar (which will eventually fetch the items from the DB

    app.controller('navController', ['$scope', '$location', function ($scope, $location) {
      $scope.navigationTabs = [
        new NAV("file","/file","File","fa fa-file-o", "active"),
        new NAV("edit","/edit","Edit","fa fa-edit", "inactive"),
        new NAV("view","/view","View","fa fa-search", "inactive"),
        new NAV("hist","/history","History","fa fa-history", "inactive"),
        new NAV("bmark","/bookmarks","Bookmarks","fa fa-bookmark-o", "inactive"),
        new NAV("hlp","/help","Help","fa fa-life-buoy", "inactive")
      ];
      ...
    }]);
    
  4. The controller defines a 'selectTab()' method which changes the url of the application when the user selects a menu-item. It also clears the active class on all the other items and sets the 'active' class on the one selected

        ...
        $scope.selectTab = function (nav) {
        $scope.navigationTabs.forEach(function (nav) {
            nav.active="";
        });
        console.log("navigating to url: " + nav.key);
        $location.path(nav.key);
        nav.active = "active";
    }
    
  5. The NAV item is defined in the app.js as follows:

    var NAV = function (id, key, value, icon, active) {
        this.id = id;
        this.key = key;
        this.value = value;
        this.icon = icon;
        this.active = active;
    };
    
\$\endgroup\$
1
\$\begingroup\$
$scope.navigationTabs = [
  new NAV("file","/file","File","fa fa-file-o", "active"),
  ...
]

All this piece of code does is define an instance of a NAV with the specified properties. It's not sharing methods, no inheritance, making the use of a constructor overkill. This is no different from doing:

$scope.navigationTabs = [
{ id: "file", key: "/file", value: "File", icon: "fa fa-file-o", active: "active" }

The above method throws away the need for a constructor, names your properties verbosely, and order of appearance doesn't matter. If you want defaults, a factory function that merges your object with a default one does the same thing. It may just be replacing your constructor, but you still keep the perks of using an object.

function createNav(settings){
  return Object.assign({
    // defaults here
  }, settings);
}

Taking this further, you can just store the objects in an array, and run map through it to create the tabs. This way, you avoid always calling the function and concentrate on just the data.

var navItems = [
  { id: "file", key: "/file", value: "File", icon: "fa fa-file-o", active: "active" },
  ....
];

$scope.navigationTabs = navItems.map(createNav);

console.log("navigating to url: " + nav.key);

Consider avoiding console.log when debugging. Use the browser's developer tools instead. Should you really want to log, use Angular's $log service as it safely detects the presence of console.


I would recommend that the directive be like a standalone component and written like:

<menu-bar menu-items="someVarWithMenuItems"></menu-bar>

In this manner, you make the directive portable, letting the enclosing controller decide what the menu items are. Another idea would be to have another directive and let the consumer write it this way:

<menu-bar>
  <menu-item ...props...>tab text</menu-item>
  <menu-item ...props...>tab text</menu-item>
  <menu-item ...props...>tab text</menu-item>
</menu-bar>

This way, you can just let the consumer drop in the scripts and not have to worry about them. They can just declare the tabs in the markup.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.