3
\$\begingroup\$

I am working on angularjs, and I have created some directives with bunch of HTML elements within its templates. I have assigned a controller for those directives. Below is my code snippet:

directives.directive('ngLd',function()
{
    return {
        restrict : "AE",
        templateUrl:'partials/ld.html',
        scope:{},
        link : function(scope,element,attr)
        {
            var lbutton = element.find("span[class='lb-lk-comm']");
            var dlbutton = element.find("span[class='lb-dlk-comm']");
            if(lbutton && dlbutton)
            {
                lbutton.bind('click',function(event)
                {
                    scope.likeAction(lbutton.hasClass("lb-voted-comm"),dlbutton.hasClass("lb-voted-comm"));
                    scope.$apply(); 
                });
                dlbutton.bind('click',function(event)
                {
                    scope.dislikeAction(lbutton.hasClass("lb-voted-comm"),dlbutton.hasClass("lb-voted-comm"));
                    scope.$apply();
                });
            }
        },
        controller : "MyController"
    };
});  

In the above code, I have used a link function of bind the click event on required elements.
Another approach I could have followed to use the ngClick directive directly over the elements, but that is sort of exposing method to client end.

My question is, am I following a good approach by binding click event ? or should I use ngClick ?

I am using angular.bootstrap(document,"myApp"); to bootstrap the angular module to document, so I was wondering if I could have similar thing to dynamically bind those events (click, hover, change etc) to my HTML elements, without exposing them to HTML pages.

\$\endgroup\$
2
\$\begingroup\$

I think you did the right thing, if you used ngClick you would loose tooling: syntax highlighting, debugging, linting.

Other than that your code looks good, some minor nitpickings:

  • a single comma separate var statement is considered better:

    var lbutton = element.find("span[class='lb-lk-comm']"),
        dlbutton = element.find("span[class='lb-dlk-comm']");
    
  • You keep repeating "lb-voted-comm", you should use a constant for it.

  • I only understood after reading likeACtion/dislikeAction that lbutton/dbutton stand for likeButton, dislikeButton ( I thought it was left button, double click button or something ), you should spell out those variables names.
\$\endgroup\$
  • \$\begingroup\$ its lbutton for like button and dlbutton for dislike, its nothing just a span as I have mentioned. \$\endgroup\$ – agpt Mar 10 '14 at 15:26
  • 1
    \$\begingroup\$ @Aman I understand, if you actually called the variables likeButton and dislikeButton, then the code would be of higher quality/readability \$\endgroup\$ – konijn Mar 10 '14 at 15:29

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.