writing a directive for validating image url

Question

this is the first directive I'm writing for AngularJS. I want to check if an image url inserted into a text input is valid.

this is what I have so far:

angular.module('directives', [])
.directive('imageUrlVerify', function() {
    return {
        restrict: 'A',
        require: 'ngModel',
        link: function(scope, element, attrs, ctrl) {
            var image = new Image();

            scope.$watch(function() {
                if (ctrl.$viewValue) {
                    image.src = ctrl.$viewValue;

                    if (image.complete) {
                        scope[attrs.imageValid] = true;
                    } else {
                        scope[attrs.imageValid] = false;
                    }
                } else {
                    scope[attrs.imageValid] = false;
                }
            });
        }
    }
})

and I use it:

<input type="text" ng-model="imageUrl" placeholder="Image URL..." data-image-valid="imageOk" image-url-verify>

This is actually working, but as my first directive I want to make sure that I'm doing things the right way...

am I missing something ?

EDIT: following @Josh-David-Miller 's answer I came up with this:

angular.module('directives', [])
.directive('imageUrlVerify', function() {
    return {
        restrict: 'A',
        replace: true,
        scope: { url: '=', imageValid: '=' },
        template: '<input ng-model="url" placeholder="Image URL..."/>',
        link: function(scope, element, attrs) {
            var image = new Image();

            scope.$watch('url', function() {
                scope.imageValid = false;
            });

            element.on( 'blur', function() {
                image.src = scope.url;
            });

            image.onload = function() {
                scope.$apply(function() {
                    scope.imageValid = true;
                });
            };

            image.onerror = function() {
                scope.$apply(function() {
                    scope.imageValid = false;
                });
            };
        }
    }
})

and use it like:

<input image-url-verify url="imageUrl" image-valid="imageOk" />

does it look better? anywhere else to improve?

Answer 1

I am not sure I follow the purpose your directive, but it can be cleaned up a bit.

First, reusable directives should have an isolate scope, so they can't mess with anything they're not supposed to. In your case, it actually makes the code simpler.

Second, the $watch statement isn't watching anything. $watch takes a first parameter that is either an string expression to be evaluated against the current scope, of a method to be executed with each digest to determine if a value changes. Your method doesn't return anything, so there's nothing to watch. Your code actually just executes the same piece of code on every digest. Perhaps you can elaborate on what you're going for here.

Lastly, your verification will occur with every keystroke, which is probably not very helpful. I changed it to run when the focus leaves the input element.

Here's the updated code:

angular.module('directives', [])
.directive('imageUrlVerify', function() {
  return {
    restrict: 'A',
    replace: true,
    scope: { url: '=', imageValid: '=' },
    template: '<input ng-bind="url" placeholder="Image URL..." valid="imageValid" />',
    link: function(scope, element, attrs) {
      var image = new Image();

      element.on( 'blur', function() {
        image.src = url;
        imageValid = url != "" && image.complete ? true : false;
      });
    }
  }
})

And then its usage is changed to:

<input image-url-verify url="imageUrl" image-valid="imageOk" />

Some of these design choices are preferences and some will depend on your specific usage, but absent either, this is how I would rewrite your directive.