angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.89k stars 27.53k forks source link

ng-model-options={updateOn: 'blur'} not working #7117

Closed kyrios closed 10 years ago

kyrios commented 10 years ago

Environment

Version: AngularJS v1.3.0-build.2602+sha.b10a437 Browser: Chrome (OS-X) 34.0.1847.116

Issue description

When using the 'blur' event the value is not saved to the model. Other events and 'default' works fine.

I'd like to use this feature in a ng-grid cell with a custom $parser. After my user enters some text into the cell I want to do one validation run before saving to the model.

I'm new to angular but investigated this a little. Maybe my investigation below helps a little narrowing the issue down.

Maybe it has something to do with event handling order since ng-grid also registers a blur handler.

Investigation

From my understanding ctrl.$setViewValue(value, event) should be called but isn't because of the $apply is a noop. (Line: 17246)

Unfortunately I don't understand the internals of angular enough to understand why $apply is a noop in my case.

if (ctrl.$viewValue !== value ||
        // If the value is still empty/falsy, and there is no `required` error, run validators
        // again. This enables HTML5 constraint validation errors to affect Angular validation
        // even when the first character entered causes an error.
        (validity && value === '' && !validity.valueMissing)) {
      if (scope.$$phase) {
        ctrl.$setViewValue(value, event);   //   <---- Neither this
      } else {
        scope.$apply(function() {
          ctrl.$setViewValue(value, event); //    <---- or this gets called
        });
      }

Plunker

http://plnkr.co/edit/IybN8jtCAdm03sCC2lxg?p=preview Does this help?

IgorMinar commented 10 years ago

If $apply is a noop then it usually means that the scope has been destroyed already.

shahata commented 10 years ago

@IgorMinar is right, the reason this doesn't work is that ng-grid destroys the input field when it loses focus. You can solve this easily by setting {updateOn: 'blur $destroy'}

http://plnkr.co/edit/JChErKEJm8aKGRejakf2?p=preview

kyrios commented 10 years ago

So in my case (using ng-grid) 'blur' simply is the wrong event? Maybe some hint in the documentation or an error on the console would help. Can't say if this is something I struggled with because I'm new to angular or it isn't that obvious.

Thanks @shahata for the plunker. Works fine now. Thanks @IgorMinar for the explanation.

ddimitrop commented 10 years ago

I have also seen that happening directly in the documentation.

The page https://docs.angularjs.org/guide/forms has a link to Plunker http://plnkr.co/edit/ngdoc:example-example102@snapshot?p=preview where the ng-model-options={updateOn: 'blur'} is not working.

The version where that happens (used in the Plunker code) is http://ajax.googleapis.com/ajax/libs/angularjs/1.3.0-beta.5/angular.min.js I double checked that this is not related to Plunker, I had the same behavior when I saved the page locally on my machine.

The problem does not happen on the version used in the main page ( https://docs.angularjs.org/guide/forms).

So even if this is a bug that has now been fixed in a later version, there is at least a problem in the documentation.

shahata commented 10 years ago

This obviously does not work in 1.3.0-beta.5 ngModelOptions was only added in 1.3.0-beta.6 :)

ggondim commented 10 years ago

A common confusion when reading docs is the selected version in the dropdown before the breadcrumb. I think it should be selected with the latest stable version and not latest beta.

alexfigueiredo commented 9 years ago

A common confusion when reading docs is the selected version in the dropdown before the breadcrumb. I think it should be selected with the latest stable version and not latest beta.

+1

TekTimmy commented 9 years ago

A common confusion when reading docs is the selected version in the dropdown before the breadcrumb. I think it should be selected with the latest stable version and not latest beta.

+1

wilk commented 9 years ago

A common confusion when reading docs is the selected version in the dropdown before the breadcrumb. I think it should be selected with the latest stable version and not latest beta.

+1

langdonx commented 9 years ago

Happy to see that this has landed in 1.3.0. I was curious if "custom events" are truly supported, or if only native-dom events are supported?

I'm thinking that this is not possible natively yet (because I can't piece together how I'd do it without my own directive), but I'm ultimately trying to only update the model once I get back a successfully persisted response from a $http or $resource call.

I have a shadow directive that I'm using all over my app that I'd love to change out for something native. An very early prototype so you can see functionally what I'm talking about doing: http://jsfiddle.net/langdonx/m4zaj/1/

JakobJingleheimer commented 9 years ago

A common confusion when reading docs is the selected version in the dropdown before the breadcrumb. I think it should be selected with the latest stable version and not latest beta.

+1