11

DEMO

Consider the following example:

<input type="text" ng-model="client.phoneNumber" phone-number>
<button ng-click="doSomething()">Do Something</button>

.directive("phoneNumber", function($compile) {
  return {
    restrict: 'A',
    scope: true,
    link: function(scope, element, attrs) { 
      scope.mobileNumberIsValid = true;

      var errorTemplate = "<span ng-show='!mobileNumberIsValid'>Error</span>";

      element.after($compile(errorTemplate)(scope)).on('blur', function() {
        scope.$apply(function() { 
          scope.mobileNumberIsValid = /^\d*$/.test(element.val());
        });
      });
    }  
  };
});

Looking at the demo, if you add say 'a' at the end of the phone number, and click the button, doSomething() is not called. If you click the button again, then doSomething() is called.

Why doSomething() is not called for the first time? Any ideas how to fix this?

Note: It is important to keep the validation on blur.

Community
  • 1
  • 1
Misha Moroshko
  • 166,356
  • 226
  • 505
  • 746
  • 1
    How about this? `#wrapper {width: 220px;}`. I think it's better to keep your design stay as it is although dynamic factors, i.e., error messages, are added and removed. However it's ur call. – allenhwkim Jan 18 '14 at 04:37
  • 1
    Moving buttons around at the same time they are being used is very bad UI design. I would just leave the span there and change the blank (` `) text to say 'Error' when there's an error. – Jason Goemaat Jan 23 '14 at 22:01

6 Answers6

10

Explain

  1. Use click button, mousedown event is triggered on button element.
  2. Input is on blur, blur callback triggered to validate input value.
  3. If invalid, error span is displayed, pushing button tag down, thus cursor left button area. If user release mouse, mouseup event is not triggered. This acts like click on a button but move outside of it before releasing mouse to cancel the button click. This is the reason ng-click is not triggered. Because mouseup event is not triggered on button.

Solution

Use ng-pattern to dynamically validate the input value, and show/hide error span immediately according to ngModel.$invalid property.

Demo 1 http://jsbin.com/epEBECAy/14/edit

----- Update 1 -----

According to author's request, updated answer with another solution.

Demo 2 http://jsbin.com/epEBECAy/21/edit?html,js

HTML

<body ng-app="Demo" ng-controller="DemoCtrl as demoCtrl">
  <pre>{{ client | json }}</pre>
  <div id="wrapper">
    <input type="text" phone-number
           ng-model="client.phoneNumber"
           ng-blur="demoCtrl.validateInput(client.phoneNumber)">
  </div>
  <button ng-mousedown="demoCtrl.pauseValidation()" 
          ng-mouseup="demoCtrl.resumeValidation()" 
          ng-click="doSomething()">Do Something</button>
</body>

Logic

I used ng-blur directive on input to trigger validation. If ng-mousedown is triggered before ng-blur, ng-blur callback will be deferred until ng-mouseup is fired. This is accomplished by utilizing $q service.

Daiwei
  • 40,666
  • 3
  • 38
  • 48
  • The whole idea is to have validation on blur, not on every key press. I find it a bit annoying when error is displayed when user just started typing the phone number (e.g. if we validate that the phone number has exactly 10 digits). – Misha Moroshko Jan 19 '14 at 09:30
  • @MishaMoroshko Please see update 1. In update 1 element layouts is the same as before and validation is still triggered on `blur`. – Daiwei Jan 19 '14 at 14:17
  • I think AngularJS is opinionated framework, and there should be logical reason behind `ng-pattern` – TruongSinh Jan 22 '14 at 03:12
  • Awesome, thanks. So basically, on ng-mousedown, I made note that the save button was pressed. Onblur, if that flag is set, I make note of it. When the validation finishes (async), then I call the save method. Lame. – vbullinger May 12 '14 at 15:34
4

It is because the directive is inserting the <span>Error</span> underneath where the button is currently placed, interfering with the click event location. You can see this by moving the button above the text box, and everything should work fine.

EDIT:

If you really must have the error in the same position, and solve the issue without creating your own click directive, you can use ng-mousedown instead of ng-click. This will trigger the click code before handling the blur event.

Matt Way
  • 32,319
  • 10
  • 79
  • 85
  • Any ideas how to solve this without moving the button? – Misha Moroshko Jan 13 '14 at 06:35
  • It's a bit of a hack, but you could wrap the scope.$apply, in a short $timeout. It would only have to be longer than a general click speed. Otherwise you would probably have to create your own ng-click like directive that overcomes the locational problems. – Matt Way Jan 13 '14 at 06:56
  • 2
    @MishaMoroshko: I wonder what **would** be ideal ? Either something has to be moved (the input, the span or the button) or you could device a perfectly hacky workaround on your button (which will also be very "un-Angular-ish". What – gkalpak Jan 17 '14 at 02:24
  • Added another simple solution. – Matt Way Jan 17 '14 at 03:07
  • 1
    From UX design, the error there is never *ideal*. And I wonder Why @MishaMoroshko doesnt use `ng-pattern` – TruongSinh Jan 22 '14 at 03:09
  • +1 From UX design, the error there is NEVER ideal. Do NOT change contents, even it's just a position, without consent of user. – allenhwkim Jan 23 '14 at 15:46
4

Here: http://jsbin.com/epEBECAy/25/edit

As explained by other answers, the button is moved by the appearance of the span before an onmouseup event on the button occurs, thus causing the issue you are experiencing. The easiest way to accomplish what you want is to style the form in such a way that the appearance of the span does not cause the button to move (this is a good idea in general from a UX perspective). You can do this by wrapping the input element in a div with a style of white-space:nowrap. As long as there is enough horizontal space for the span, the button will not move and the ng-click event will work as expected.

<div id="wrapper">
    <div style='white-space:nowrap;'>
        <input type="text" ng-model="client.phoneNumber" phone-number>
    </div>
    <button ng-click="doSomething()">Do Something</button>
</div>
Trevor
  • 13,085
  • 13
  • 76
  • 99
1

Not a direct answer, but a suggestion for writing the directive differently (the html is the same):

http://jsbin.com/OTELeFe/1/

angular.module("Demo", [])
.controller("DemoCtrl", function($scope) {
  $scope.client = {
    phoneNumber: '0451785986'
  };
  $scope.doSomething = function() {
    console.log('Doing...');
  };
})
.directive("phoneNumber", function($compile) {

  var errorTemplate = "<span ng-show='!mobileNumberIsValid'> Error </span>";

   var link = function(scope, element, attrs) { 

    $compile(element.find("span"))(scope);

    scope.mobileNumberIsValid = true;
    scope.$watch('ngModel', function(v){
     scope.mobileNumberIsValid = /^\d*$/.test(v);
    });
  }; 

  var compile = function(element, attrs){
      var h = element[0].outerHTML;
      var newHtml = [
        '<div>',
        h.replace('phone-number', ''),
        errorTemplate,
        '</div>'
      ].join("\n");
      element.replaceWith(newHtml);
      return link;
    };

  return {
    scope: {
      ngModel: '='
    },
    compile: compile
  };
});
ed.
  • 2,696
  • 3
  • 22
  • 25
0

I would suggest using $parsers and $setValidity way while validating phone number.

app.directive('phoneNumber', function () {
    return {
        restrict: 'A',
        require: '?ngModel',
        link: function(scope, element, attrs, ctrl) { 
            if (!ctrl) return;
            ctrl.$parsers.unshift(function(viewValue) {
                var valid = /^\d*$/.test(viewValue);
                ctrl.$setValidity('phoneNumber', valid);
                return viewValue;
            });
            ctrl.$formatters.unshift(function(modelValue) {
                var valid = /^\d*$/.test(modelValue);
                ctrl.$setValidity('phoneNumber', valid);
                return modelValue;
            });

        }
    }
});

So, you will be able to use $valid property on a field in your view:

<form name="form" ng-submit="doSomething()" novalidate>
    <input type="text" name="phone" ng-model="phoneNumber" phone-number/>
    <p ng-show="form.phone.$invalid">(Show on error)Wrong phone number</p>
</form>

If you want to show errors only on blur you can use (found here: AngularJS Forms - Validate Fields After User Has Left Field):

var focusDirective = function () {
  return {
    restrict: 'E',
    require: '?ngModel',
    link: function (scope, element, attrs, ctrl) {
      var elm = $(element);
      if (!ctrl) return;
      elm.on('focus', function () {
        elm.addClass('has-focus');
        ctrl.$hasFocus = true;
        if(!scope.$$phase) scope.$digest();
      });
      elm.on('blur', function () {
        elm.removeClass('has-focus');
        elm.addClass('has-visited');
        ctrl.$hasFocus = false;
        ctrl.$hasVisited = true;
        if(!scope.$$phase) scope.$digest();
      });
      elm.closest('form').on('submit', function () {
        elm.addClass('has-visited');
        ctrl.$hasFocus = false;
        ctrl.$hasVisited = true;
        if(!scope.$$phase) scope.$digest();
      })
    }
  }   
};
app.directive('input', focusDirective);

So, you will have hasFocus property if field is focused now and hasVisited property if that field blured one or more times:

<form name="form" ng-submit="doSomething()" novalidate>
    <input type="text" name="phone" ng-model="phoneNumber" phone-number/>
    <p ng-show="form.phone.$invalid">[error] Wrong phone number</p>
    <p ng-show="form.phone.$invalid 
             && form.phone.$hasVisited">[error && visited] Wrong phone number</p>
    <p ng-show="form.phone.$invalid 
            && !form.phone.$hasFocus">[error && blur] Wrong phone number</p>
    <div><input type="submit" value="Submit"/></div>
</form>

Demo: http://jsfiddle.net/zVpWh/4/

Community
  • 1
  • 1
Pythonic
  • 486
  • 5
  • 9
0

I fixed it with the following.

<button class="submitButton form-control" type="submit" ng-mousedown="this.form.submit();" >
CountZero
  • 6,171
  • 3
  • 46
  • 59