Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have a very simple directive <my-card> that lists out some data.

Example Usage

my-card(ng-repeat="item in parentCtrl.items" item='item')

The only main functionality I have is a button that toggles some additional data.

Here is my directive/controller definition. Should I use the link function instead of creating a controller? The reason I moved the logic into the MyCardCtrl was so that I can easily unit test it.

MyCardCtrl

  function MyCardCtrl() {
    var vm = this;

    this.cardClass = 'my-card-' + vm.item.type;

    this.toggle = function() {
      vm.item.showMore = !vm.item.showMore;
    };

    this.init = function(element) {
      var img = angular.element(element[0].querySelector('.card-bg'));
      img.css('background-image', 'url(' + vm.item.image_url + ')');
    };
  }

myCard Directive

  function myCard() {
    return {
      restrict: 'E',
      replace: true,
      templateUrl: 'my-card.html'
      scope: {
        item: '='
      },
      link: function(scope, element, attrs, myCardCtrl) {
        myCardCtrl.init(element);
      },
      controller: 'MyCardCtrl',
      controllerAs: 'card',
      bindToController: true
    };
  }

  angular
    .module('components')
    .directive('myCard',      [myCard])
    .controller('MyCardCtrl', [MyCardCtrl]);

myCard template (written in Jade)

 .my-card(ng-class='class.cardClass')
    button(ng-click='card.toggleMeta()')
    .my-card-content(ng-show='card.item.showMore')
      // expose item object
      h1 {{::item.name}}
    .my-card-more(ng-show='!card.item.showMore')
    .my-card-bg
share|improve this question

Just get rid of the link function entirely. You can set the background-image in the template with ng-style, and then you have one less function to worry about:

.my-card(ng-style='{"background-image": "url(" + card.item.image_url + ")"}', ng-class='card.cardClass')
  button(ng-click='card.toggle()')
  .my-card-content(ng-show='card.item.showMore')
    h1 {{::card.item.name}}
  .my-card-more(ng-show='!card.item.showMore')
  .my-card-bg

If you want to build up the url(card.item.image_url) string in the controller for easier testing, you can still use ng-style to apply the resulting variable. For example:

function MyCardCtrl() {
  var vm = this;

  vm.backgroundUrl = 'url("' + vm.item.image_url + '")';

  vm.cardClass = 'my-card-' + vm.item.type;

  vm.toggle = function() {
    vm.item.showMore = !vm.item.showMore;
  };
}

Also, if you're on Angular 1.5+, this directive can be written even more simply as a component:

angular
  .module('components')
  .component('myCard', {
    bindings: {
      item: '<'
    },
    templateUrl: 'my-card.html',
    controller: 'MyCardCtrl',
    controllerAs: 'card'
  })
  .controller('MyCardCtrl', [MyCardCtrl]);
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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