2
\$\begingroup\$

I am showing templates in popups according to the user navigation links. It works for me, but, is this the correct way to do it?

var app = angular.module('plunker', []);

app.service('modalService', function( $rootScope ) {


  this.hide = function () {

    this.show = false;


  }

  this.showIt = function () {

    this.show = true;

  }

  this.setCategory = function ( category ) {

     return this.showPath = category+'.html'

  }

  this.showCategory = function (category) {

    this.setCategory( category )
    $rootScope.$apply();
  }

})

app.controller('header', function($scope) {

  $scope.view = "home view";

});

app.controller('home', function($scope, modalService) {

  $scope.name = 'World';
  $scope.service = modalService; 

});

//header directive

app.directive('headerDir', function( modalService) {

  return {
      restrict : "E",
      replace:true,
      templateUrl:'header.html',
      scope:{},
      link : function (scope, element, attrs) {


         element.on('click', '.edit', function () {
            modalService.showIt();
            modalService.showCategory('edit');
         });

         element.on('click', '.service', function () {
           modalService.showIt();
           modalService.showCategory('service');

         })

      }
  }

});

app.directive('popUpDir', function () {

  return {
    replace:true,
    restrict:"E",
    templateUrl : "popup.html"
  }

})

Click on the top link to show the popup, and click on the screen to hide the popup.

Live Demo

\$\endgroup\$
1
\$\begingroup\$
  this.hide = function () {

    this.show = false;


  }

  this.showIt = function () {

    this.show = true;

  }

You have extraneous lines everywhere, and extraneous space everywhere, trim it down.

this.hide   = function(){ this.show = false; }
this.showIt = function(){ this.show = true;  }

On the other hand, here, you're missing space (but also have extra space):

  this.setCategory = function ( category ) {

     return this.showPath = category+'.html'
                                   ^^^
  }

should be:

this.setCategory = function(category){ this.showPath = category + '.html'; }

The return is redundant, as the value goes nowhere in showCategory.


     element.on('click', '.edit', function () {
        modalService.showIt();
        modalService.showCategory('edit');
     });

     element.on('click', '.service', function () {
       modalService.showIt();
       modalService.showCategory('service');

This is redundant, pass the .service / .edit in as a variable.

\$\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.