feat(input): Allow custom events and timeouts to trigger model updates #2129

Closed
wants to merge 3 commits into
from

Projects

None yet
@lrlopez
Contributor
lrlopez commented Mar 8, 2013

By default, any change on the content will trigger an immediate model update and form validation. With this PR you can override this behavior using the ng-update-model-on attribute to bind only to a comma-delimited list of events.

I.e. ng-update-model-on="blur" will update and validate only after the control loses focus.

If you want to keep the default behavior and just add new events that may trigger the model update and validation, add "default" as one of the specified events.

I.e. ng-update-model-on="default,submit"

Also, a ng-update-model-debounce attribute will allow defering the actual model update after the last triggered event. This feature is not available in radio buttons.

I.e. ng-update-model-debounce="500" for 500ms

Custom timeouts for each event can be set for each event if you use an object in ng-update-model-on. I.e. ng-update-model-on="{default: 500, blur: 0}"

You can specify both attributes in any tag so they became the default settings for any child control, although they can be overriden.

Closes #1285

@lrlopez
Contributor
lrlopez commented Mar 9, 2013

BTW, I already signed the CLA

@lrlopez
Contributor
lrlopez commented Mar 9, 2013

One note about the missing support for timeouts in radio buttons: Radios are, in fact, different input controls that are tied to the same model. The timeout timer gets reset & launched on every input content change, but it can't know about the other radio controls' timers.

This has an unpleasant side effect: imagine you have 3 radio buttons bound to the same model with a timeout of 1 second. If you click the three options one after the other fast enough, after (approx.) one second there will be 3 timeouts that will trigger a model update three times in a row instead of just one.

If this is acceptable for you, I can also support timeouts in radio buttons.

@symblify
symblify commented Mar 9, 2013

I like the sound of this a lot. Take, for example, a search input box. At the moment as the user types it keeps triggering the update of the model, and if you have a filter based on that, it could be a lot of work between key presses, slowing the UI down. The ability to use different events and a timeout would make this scenario more performant.

It would be useful if there was some way to set globally the event (and perhaps timeout too) used for model updates so that consistency can be achieved without having to add attributes to every input, although I suppose you could create a directive which applies the attributes across the app.

@lrlopez
Contributor
lrlopez commented Mar 9, 2013

Thanks for the suggestion @symblify. I've updated the PR so you can specify both ng-update-model-on and ng-update-model-timeout in a <form> tag and it will be set as default to any child control or subform that inherites the form scope. You can override them if necessary.

Also, if you set $updateModelOn or $updateModelTimeout in the root scope, settings will be app-wide default.

I've added the new bits of documentation and a new spec that tests the feature. Enjoy!

@symblify
symblify commented Mar 9, 2013

Very nice! Thanks for taking on board my suggestion @lrlopez, this looks like a great solution. Hopefully this will get merged in soon.

@IgorMinar
Member

I'm not too crazy about pulling the global default from the rootScope. I need to review the rest

@lrlopez
Contributor
lrlopez commented Mar 12, 2013

@IgorMinar, well... In fact it actually pulls the value from the current scope, but due to scope inheritance if options get set into the root scope it will be available everywhere unless they are overridden. This allows granularity when applying defaults (i.e. app-wide, controller-wide, etc.)

Is that considered a bad practice?

@IgorMinar
Member

primarily I'm concerned about what is the correct behavior when you have components with isolated scopes.

@lrlopez
Contributor
lrlopez commented Mar 12, 2013

Hmmm... You are right, but then there is not much to do unless you use a global config service, import $rootScope using DI (which would ignore any intermediate override) or start pulling info directly from the DOM (which seems way too bad).

May be having a standard AngularJS global config provider would be nice for handling these cases.

Anyway, wouldn't make sense that behavior for elements inside an isolated scope component? They idea behind isolation is to prevent "interferences" from the outside...

BTW, if the Angular team thinks that having defaults this way is not desirable, I can undo it in order to have the basic functionality merged.

We can support defaults later once a better approach is found. Just tell me what you do...

@lrlopez
Contributor
lrlopez commented Mar 16, 2013

Just an update. I've been using this feature on a couple of personal projects and it seems to work ok. Also, most of my students applied ng-update-model in the <form> tag. None of them had the urge of using global appwide defaults.

Like @IgorMinar, I'm still concerned about isolated scopes, but I fear there's not much we can do about it unless we define & use a global configuration service or mess with the DOM, which doesn't sound too Angular. But I might be wrong...

@symblify

Perhaps to solve the issues @IgorMinar refers to, the feature should be limited to individual controls but the default should come from a provider ($modelUpdateProvider?) and could be set in the same way that html5mode is set up in $locationProvider, to give an example.

I think it is more likely you would want to have consistency across all forms in an app which suggests making it a global configurable setting rather than having to set it up on all forms. Plus it sorts the issues with scopes and $rootScope.

@lrlopez
Contributor
lrlopez commented Apr 10, 2013

@symblify, thanks for the proposal... it's a good idea too.

I've been thinking on this during the weekend and found another way to implement defaults in an intuitive way: What about extending ng-update-model-on and ng-update-model-timeout so they can be stated in any tag? All the input tags inside will use those settings and could also be overridden. This way you could put the defaults you want in the <body> tag and would be global without needing to pollute any scope from a controller.

Lots of possibilities... I don't know what to do! Now that AngularJS 1.1.4 has launched may be we can claim some time from the core team in order to throw some light into the approach they like... 😜

@symblify

I agree, I would like to hear how the core team would approach this. Model updating is a core feature, and at the moment there isn't much control over how it happens, and it would be good to have more flexibility.

As an aside, It is actually possible to change the way model updating works in current Angular builds by using a decorator to change the default behaviour. See my example plunk here: http://plnkr.co/edit/nMruiQdhdSS6B2j1XT8f?p=preview

@lrlopez
Contributor
lrlopez commented Apr 14, 2013

I've rebased the PR to latest master and also fixed some doc typos. Reference to $rootScope has been replaced into a generic explanation about setting default values from the scope. Directives that choose to have an isolated scope will have the same problems using other core features, like $index on a ng-repeat, so I think this should be expected.

May be it's enough. Let's wait for the team response...

@vojtajina vojtajina commented on an outdated diff May 1, 2013
src/ng/directive/form.js
@@ -48,6 +48,15 @@ function FormController(element, attrs) {
form.$valid = true;
form.$invalid = false;
+ // allows specifying default values for ng-update-model
+ if (attrs.ngUpdateModelOn != undefined) {
@vojtajina
vojtajina May 1, 2013 Contributor

use isDefined()

@vojtajina vojtajina commented on an outdated diff May 1, 2013
src/ng/directive/input.js
- if (!timeout) {
- timeout = $browser.defer(function() {
- listener();
- timeout = null;
- });
+ // Allow adding/overriding bound events
+ if (!isEmpty(eventList)) {
+ defaultEvents = false;
+ // bind to user-defined events
+ angular.forEach(eventList.split(','), function(ev) {
@vojtajina
vojtajina May 1, 2013 Contributor

you can use just forEach

@vojtajina vojtajina commented on an outdated diff May 1, 2013
src/ng/directive/input.js
if (element[0].checked) {
scope.$apply(function() {
ctrl.$setViewValue(attr.value);
});
}
+ };
+
+ // make the name unique, if not defined
+ if (isUndefined(attr.name)) {
+ element.attr('name', nextUid());
+ }
+
+ // bind to user-defined/default events
+ angular.forEach(eventList.split(','), function(ev) {
@vojtajina
vojtajina May 1, 2013 Contributor

same as above

@vojtajina
Contributor

I like this, it's a valuable feature.

I think it's fine, if you can't change this for isolate components. If they want to allow you to do so, they can export a way of doing it (either attribute or some config service).

@petebacondarwin petebacondarwin commented on an outdated diff May 2, 2013
docs/content/guide/forms.ngdoc
@@ -125,6 +125,19 @@ This allows us to extend the above example with these features:
- SAVE button is enabled only if form has some changes and is valid
- custom error messages for `user.email` and `user.agree`
+By default, any change on the content will trigger a model update and form validation. You can override this behavior using the ng-update-on
@petebacondarwin
petebacondarwin May 2, 2013 Member

typo: ng-update-on -> ng-update-model-on

@petebacondarwin petebacondarwin commented on an outdated diff May 2, 2013
src/ng/directive/form.js
@@ -48,6 +48,15 @@ function FormController(element, attrs) {
form.$valid = true;
form.$invalid = false;
+ // allows specifying default values for ng-update-model
@petebacondarwin
petebacondarwin May 2, 2013 Member

typo: ng-update-model -> ng-update-model-on

@petebacondarwin
Member

@lrlopez - regarding this comment: #2129 (comment).

I think that this is actually the way forward. We should have a new pair of directives, ngUpdateModelOn and ngUpdateModelTimeout, which can each be attached to any element. Each of these will create a controller, ngUpdateModelOnController and ngUpdateModelOnTimeoutController. The values for how and when to update the model will be stored on these controllers. The ngModel directive will then optionally require these two controllers from its ancestors:

  require: ['^?ngUpdateMode', '^?ngUpdateModeTimeout'],

If ngModel finds one or both of these controllers then it will delegate updating the model to these controllers.

This removes the need to put anything onto the scope, which is nasty, and supports both adding to individual inputs, forms, groups of inputs (inside a div, say) or even the whole page (say on the body element). It also removes the need to create a global provider.

I think it would also make the implementation cleaner as the the new stuff is hidden nicely in the new directves, no changes to form would be necessary and minimal changes to the input directives.

@lrlopez, @IgorMinar , @vojtajina and @symblify : what do you think of this?

@lrlopez
Contributor
lrlopez commented May 2, 2013

Pete, I really like your approach. Once I get home I'll start working on it... Thanks!

@IgorMinar
Member

how about creating a generic $debounce service instead of the custom timeout code?

@petebacondarwin
Member

Like this :-) ?
https://github.com/angular-ui/angular-ui/blob/debounce/modules/services/debounce/debounce.js

On 3 May 2013 18:48, Igor Minar notifications@github.com wrote:

how about creating a generic $debounce service instead of the custom
timeout code?


Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/pull/2129#issuecomment-17408364
.

@lrlopez
Contributor
lrlopez commented May 3, 2013

@IgorMinar, can you elaborate the idea?

@lrlopez
Contributor
lrlopez commented May 3, 2013

Ok, I got it. It's an interesting Idea, I'll split the timeout code into a generic service

@lrlopez
Contributor
lrlopez commented May 3, 2013

Oops, just saw Pete's reply. I can reuse that code, reference it or implement a new one, whatever you want...

@lrlopez
Contributor
lrlopez commented May 5, 2013

Instead of creating a new service, I modified $timeout so it can be used to reset the timer. This allows an easy implementation of debouncing behaviors when needed, as it's the case with the custom model updates.

I also followed Pete's advice and created two new directives ngUpdateModelOn and ngUpdateModelTimeout that can be applied to any element. Current values are extracted from the controllers, delegating upwards if necessary.

All these features (debouncing+custom model updates) combined add 1103 bytes to the minified build. I've splitted both enhancements into two different pull requests in order to ease the creation of the changelog.

I hope everything is ok now...

@symblify
symblify commented May 5, 2013

I'm no expert but this looks good to me.

@lrlopez
Contributor
lrlopez commented May 5, 2013

PR updated. I forgot to change one angular.forEach :)

@petebacondarwin petebacondarwin and 1 other commented on an outdated diff May 7, 2013
src/ng/directive/input.js
link: function(scope, element, attr, ctrl) {
- if (ctrl) {
- (inputType[lowercase(attr.type)] || inputType.text)(scope, element, attr, ctrl, $sniffer,
- $browser);
+ if (ctrl[0]) {
+ (inputType[lowercase(attr.type)] || inputType.text)(scope, element, attr, ctrl[0], [ctrl[1], ctrl[2]], $timeout,
@petebacondarwin
petebacondarwin May 7, 2013 Member

Is there any benefit in wrapping these two "update" controllers in an array just to unwrap them again later? Wouldn't it be simpler just to pass two parameters?

@lrlopez
lrlopez May 7, 2013 Contributor

Just some byte-savings... May be it's not worth it, I'll expand the parameter

@petebacondarwin
Member
@lrlopez
Contributor
lrlopez commented May 7, 2013

Oops. I'll have a look tonight... Thanks!

@lrlopez
Contributor
lrlopez commented May 12, 2013

I'm still working on this. I don't know why it's failing on our good & old friend IE8, so I'm downloading a Internet Explorer Application Compatibility VPC Image to do some debugging, but it could take some time... I'm really sorry!

@lrlopez
Contributor
lrlopez commented May 12, 2013

Ok, I think I've found the culprit. It seems that browserTrigger for IE<9 unconditionally switches the checked attribute for radio buttons and checkboxes (look here) so both tests fail when we 'emulate' a blur event on them.

In order to avoid spamming for those who are watching this PR, I'll keep this post updated with my latests findings.

UPDATE: Problem is solved if we only switch checked when emulating click events. In order to fix this I need to modify browserTrigger.js from ngScenario. I'll add another commit with the change...

@lrlopez
Contributor
lrlopez commented May 21, 2013

@petebacondarwin Pete, can you check if IE is happy now?

@symblify

@lrlopez I've added a comment to your ngScenario change, there is a = missing. With that fixed, I have been able to get all tests to pass in IE8.

@petebacondarwin @IgorMinar I see the 1.2 release is imminent, assuming the above fix can be made quickly, is it likely this feature will make it in? I would really like to see it in! Thanks.

@lrlopez
Contributor
lrlopez commented May 25, 2013

Typo fixed...

@lrlopez
Contributor
lrlopez commented May 30, 2013

Rebased to latest master as there was a conflict with 041f118

Edit: Does anybody know what's wrong with Travis?

@brodin
brodin commented Jun 3, 2013

Whats the status of this PR? Can we count on it soon?

@lrlopez
Contributor
lrlopez commented Jun 3, 2013

We're still waiting for an answer from the core team... ping @IgorMinar @petebacondarwin

@rzschech
Contributor

@lrlopez I tried out your PR but have an issue when combining both ng-update-model-timeout="500" and ng-update-model-on="blur". I would like the behavior to be that if the timeout or the blur event occurs the model is updated. However in this case it seems that update-model-on="blur" is preventing the timeout from being scheduled.

@symblify

Have you tried ng-update-model-on="default,blur"?

@rzschech
Contributor

Trying ng-update-model-timeout="500" and ng-update-model-on="default,blur" causes the model to be updated when the timeout occurs but not when the blur occurs.

@lrlopez
Contributor
lrlopez commented Jun 16, 2013

Hmmm... it works for me. Be aware of the 500ms timeout: in your example it also applies on the blur event. This means that the model will update 500ms after the input loses focus.

If it doesn't work that way for you, please create an example using http://plnkr.co/ and share it with us so we can have a look.

@rzschech
Contributor

Ahh, that explains it. My interpretation was to update the model on the timeout or on blur. The I need a ng-update-model-on-timeout.

@petebacondarwin
Member

How about we have two valid syntaxes for ng-update-model-on="expression", similar to ng-class:

  • string : (e.g. ng-update-model-on="default,blur") -> comma separated list of events to observe
  • object: (e.g. ng-update-model-on="{ default: 500, blur: 0 }" -> key is event name, value is timeout

Then ng-model-update-timeout is not needed and your template becomes simpler. From:

<input ng-model-update-on="blur" ng-model-update-timeout="500" ng-model="myModel">

To:

<input ng-model-update-on="{blur:500}" ng-model="myModel">

@lrlopez et al: What do you think?

@petebacondarwin
Member

You could also still keep the ng-model-update-timeout directive and it could provide a default timeout to any child ng-model-update-on directives that do not specify a timeout explicitly.

@L42y
L42y commented Jul 3, 2013

This is great, hope this can merge ASAP, thank you for all the great work, @lrlopez.

@lrlopez
Contributor
lrlopez commented Jul 3, 2013

@petebacondarwin,
I coded your suggestion, but found a problem:
In order to parse the object, I need to call $scope.$eval because eval is unsafe. This means that if you use a comma-delimited string expression, additional quotes will be needed:
ng-update-model-on="'default,blur'" (notice the surrounding ')
ng-update-model-on="{ default: 500, blur: 0 }"

On the other side, the parameter now could be any Angular expression, including function calls:
ng-update-model-on="getEvents(2+2)" (will bind to any event returned from calling $scope.getEvents(4))

If you think it is okay, I will update the PR with the new enhancements...

@petebacondarwin
Member

I was thinking that we could do a manual regex on the expression to ascertain whether it was a straight comma separated list or an object that needed evaluating.

@petebacondarwin
Member

I.E. check for wrapping {...}

@petebacondarwin
Member

This would not allow a generic getEvents() style expression but could allow { default: getTimeout() }

@lrlopez
Contributor
lrlopez commented Jul 3, 2013

Thanks, will do this way. I hope is not too inconsistent with the rest of AngularJS...

@petebacondarwin
Member

As an aside, you could have a third option, which is a single number, that would be a general timeout value:

ng-update-model-on="500" would effectively be what you had before with ng-update-model-timeout="500"

@petebacondarwin
Member

By the way @jeffbcross is working on something that is related and so we should get his input here too.

@lrlopez
Contributor
lrlopez commented Jul 4, 2013

Ok. It is all done but the third option (I've just read the idea). I've had to fix a function call in angular-mocks as there was a missing parameter.

Hope the PR is good enough to be merged. By the way, passes all the tests on IE8 too.

@L42y
L42y commented Jul 4, 2013

👍 @lrlopez great work!

@lrlopez
Contributor
lrlopez commented Jul 4, 2013

I've thinking about the idea of using ng-update-model-on for setting the default timeout (the third option Pete mentioned above). That would replicate what ng-update-model-timeout does... I'd prefer not to overload the directive with duplicate features, but I'm open to anyone to convince me :)

@ia3andy
ia3andy commented Jul 4, 2013

Nice work!
This feature has to be merged as soon as possible. This is a must have.
On mobile devices (iphone 4 especially), watching key events is too slow (nearly not usable at all).
Thanks again.

@lrlopez
Contributor
lrlopez commented Jul 4, 2013

I've updated the documentation to reflect the new object syntax and added a new test on checkboxes. Testing on my IE8 VM right now.

Update: IE8 tests pass.

@Foxandxss
Member

I love it, it provides a lot of good stuff in one place. Hope to see this merged for the next version :) 👍

@petebacondarwin
Member

PR Checklist (Major Feature)

  • Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name)
  • Feature is focused on a core value of the framework
  • PR is approved by 2+ senior team members and no committer is blocking the change
  • API is compatible with existing Angular apis and relevant standards (if applicable)
  • Breaking change check
    • No breaking change
    • Intentional breaking change
      • Approved by 2+ senior team members
      • The breaking change is documented (including migration steps)
  • PR contains unit tests
  • PR contains e2e tests (if suitable)
  • PR contains documentation update
  • PR passes all tests on Travis (sanity)
  • PR passes all tests on ci.angularjs.org (cross-browser compatibility)
  • PR is rebased against recent master
  • PR is squashed into one commit per logical change
  • PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
  • All changes requested in review have been implemented
@lrlopez
Contributor
lrlopez commented Jul 9, 2013
  • I've already signed the CLA.
  • Tests for the new features are also included.
  • Travis CI seems to like this PR :)
  • The PR has been rebased against a recent master.
  • There is one commit per logical feature. Their messages follow the guidelines.
@lrlopez
Contributor
lrlopez commented Jul 13, 2013

About the "2+ senior team members" approbation... should I bring their attention on this PR or just wait?

@petebacondarwin
Member

Hi Luis
Sorry about the wait. The team are aware of this PR but because it is
quite significant it needs to be checked thoroughly. Just hold on for a
little longer.
Pete

On 13 July 2013 13:01, Luis Ramón López notifications@github.com wrote:

About the "2+ senior team members" approbation... should I bring their
attention on this PR or just wait?


Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/pull/2129#issuecomment-20918976
.

@DerAlbertCom

Just my 0.02 cents, i think the naming of ng-update-model-timeout is misleading. It is not really timeout, is is the time after the model is updated after the last event occurs. this behavior can be used to throttle the incoming events, like scrolling, clicks, keys so that not on every event the function is called.

so ng-update-model-throttle is IMHO a better name which leads to the expected behavior.

@lrlopez
Contributor
lrlopez commented Jul 20, 2013

Yeah, it may be better. I don't mind changing the naming, but I think it is wiser to wait for the team opinion on this PR first as they may request further changes...

@lrlopez
Contributor
lrlopez commented Jul 27, 2013

I'm not sure if ng-update-model-throttle is the right name. We are implementing debouncing, not throttling... so ng-update-model-debounce may be better...

Anyway, someone landed the fix in angular-mocks some hours ago so this PR may conflict when rebased to current master. I'll try to rewrite the history so no clash takes place once I arrive home.

It's been 4 months since I opened this PR, I hope it gets reviewed soon so I don't have to invest more time in keeping it on-sync with master... Sigh.

@L42y
L42y commented Jul 28, 2013

someone please take some time review and merge this PR

@lrlopez
Contributor
lrlopez commented Jul 28, 2013

Ok... rebased to current master. Also, I fixed a typo on the debouncing and custom events documentation. While we are waiting for review, we could discuss two minor details:

  1. What would be the best naming for ng-update-model-timeout? It's not really a fixed timeout so Albert proposes ng-update-model-throttle, although I think that ng-update-model-debounce may be better. ng-update-model-debounce-timeout is too long IMHO, but it is the one that best describes the feature.
  2. Would it be better to specify the event list using a space as the delimiter? This way it would be more jQuerish as the syntax would be comparable to $.on()...

Any thoughts?

@lrlopez
Contributor
lrlopez commented Aug 5, 2013

Ok. I've decided to keep the comma-delimited list but rename ng-update-model-timeout to ng-update-model-debounce. Still waiting for the PR review...

@lrlopez
Contributor
lrlopez commented Aug 7, 2013

@petebacondarwin,
Pete, any news on this?

By the way, I realized this is the most popular open PR and the third in the longest-running list. Where are the prizes? 😃

@ia3andy
ia3andy commented Aug 8, 2013

Hi guys,
Just a comment on something I went across recently, If you use change and blur event on an input (ng-model), you need to blur all input before (or onclick) on the form submit button in order to set the value.
It sounds logical, but some people may come across this issue soon..
Thanx for your work :)

@lrlopez
Contributor
lrlopez commented Aug 8, 2013

@ia3andy Yeah, you're right. I haven't tried this, but you could also include the submit event in ng-model-update-on with zero timeout in order to update the model instantly when the user presses the submit button...

@lrlopez
Contributor
lrlopez commented Aug 13, 2013

@IgorMinar Igor, the release of 1.2.0rc1 means that there is no chance for this feature to be available in the next stable branch?

@symblify

I was disappointed to see this hadn't made the release candidate, which by its very name suggests that the 1.2.0 branch is "feature complete" and should have only bug fixes between now and final release. But I'd be pleased to hear otherwise!

@L42y
L42y commented Aug 14, 2013

@IgorMinar would you please consider about including this pr in 1.2.0?

@lgersman

@IgorMinar I need it too for https://github.com/lgersman/jquery.orangevolt-ampere . when using a search input for filtering data using angular performance gets worse without throttling/debouncing...

@djtarazona

This is great! Disappointed to see it didn't make it into 1.2.0 RC. Let's get it in!

@mdeinum
mdeinum commented Aug 15, 2013

We also need this (we need throttling and a different event to update the model). So please, pretty please, lets get this into the 1.2 version!.

@lgersman

After googling around for a throttle/debounce solution of updating an angular model i hacked together a very easy solution seamlessy integrating into angularjs.

Debounced / throttled model updates for angularjs : http://jsfiddle.net/lgersman/vPsGb/3/

Its basically a small piece of code consisting of a single angular directive named "ng-ampere-debounce" utilizing http://benalman.com/projects/jquery-throttle-debounce-plugin/ which can be attached to any dom element. The directive reorders the attached event handlers so that it can control when to throttle events.

You can use it for throttling/debouncing

  • model angular updates
  • angular event handler ng-[event]
  • jquery event handlers

Have a look : http://jsfiddle.net/lgersman/vPsGb/3/

The directive will be part of the Orangevolt Ampere framework (https://github.com/lgersman/jquery.orangevolt-ampere).

@Foxandxss
Member

@lgersman yeah but the idea is to not use jquery :P

@lgersman

@Foxandxss Agreed, that would be the ideal solution.

But until this feature is integrated in AngularJS, its good to know that its easy to workaround this requirement using the proposed code.

@lrlopez
Contributor
lrlopez commented Aug 15, 2013

I've polished the docs and added a demo in the forms section. I've also rebased to latest master.

By the way, I found a problem with the docs generation: This decorator function wraps the deferred model update function into an $apply which causes an $apply already in progress error when debouncing is used.

The directive is not to blame but the workaround pushed by @mhevery to support code prettify (c4fa487). I just avoided to show an example which implements debouncing, but solving this would imply to check if we're in a digest phase before using $apply in the directive, which I think it is not advisable...

@lrlopez
Contributor
lrlopez commented Aug 17, 2013

Ehh... Does anybody know if the Angular team check long-standing open PRs?

EDIT: Sorry for being childish in the original rant. I acknowledge there are thousand of things more important than this PR right now. I'm rather frustrated with the wait but I should have cooled down before pressing the Comment button...

@L42y L42y referenced this pull request in yeoman/generator-angular Aug 17, 2013
Closed

Add connect-modrewrite for URL rewriting #132

@IgorMinar
Member

@lrlopez we do, but there is a reason why some PRs are taking longer to process, they are big/hard changes that require significant effort and time.

we close lots of PRs every week including some long standing (http://angular.github.io/angular-prs/). In fact one of the reasons why 1.2 is taking so long is that there are just so many PRs that we need to look at and most of them require a few hours or even a few days of work before they can be merged. I'm afraid that this one is not going to make it into 1.2, but we do want to address it soon after 1.2.

Thanks for your patience.

@lrlopez
Contributor
lrlopez commented Aug 18, 2013

Thanks to you Igor, I really appreciate the update...

@lgersman

The "ng-update-model-debounce" directive from @lrlopez solves the need of throttled model updates, thats fine.

but how about throttled event directives (ng-change, etc) - are there any solutions ?

@lrlopez
Contributor
lrlopez commented Aug 20, 2013

Yes. One of the enhancements included into this PR is debouncing support for $timeout. If lrlopez@186c3d8 gets merged, you could do this:

var debounce;
var doRealSave = function() {
   // Save model to DB
}
$scope.save = function() {
   // debounce call for 2 seconds
   debounce = $timeout(doRealSave, 2000, debounce);
}

And in the form:

<input type="text" ng-model="name" ng-change="save()">

That will call doRealSave() 2 seconds after the last ng-change takes place.

@tonywok
tonywok commented Sep 16, 2013

Very awesome. This is exactly what I was looking for - glad I found it. Will be very cool when this gets in.

@schmurfy
schmurfy commented Oct 6, 2013

i am curious about the status on this one, I understand it may get time to get things right but man that's 7 months now according to github xD

@lrlopez
Contributor
lrlopez commented Oct 6, 2013

The only thing we can do for now is wait...

@IgorMinar
Member

We brainstormed about this today with @auser and @mhevery and considered many syntaxes and implementation approaches, here is the recap:

1/ everything in ng-model syntax

<input ng-model="{ path: 'book.title', updateOn: 'blur', debounce: 200 }">

Pros:

  • terse and readable
  • extensible

Cons:

  • will make error checking worse because <input ng-model="some.path"> where path evaluates to an object (currently throws a non-writable expression error) will be indistinguishable from the config object.
  • according to @mhevery, ngModel is not responsible for handling input events, so the event config should not be part of ngModel config

2/ explicit ng-model-* attributes

<input ng-model="book.title"
       ng-model-update-on="blur"
       ng-model-debounce="200">

Pros:

  • explicit
  • doesn't make error-checking worse

Cons:

  • verbose
  • hard to extend
  • verbose
  • per @mhevery, the same ngModel responsibility issue as 1/
    -verbose

3/ similar to 2/ but event name is defined via ng-input specific attribute

<input ng-model="book.title"
       ng-model-debounce="200"
       ng-input-update-model-on="blur">

Pros:

  • makes @mhevery happy
  • otherwise the same as 2/

Cons:

  • possibly non-intuitive from app developer's perspective
  • the same as 2/
  • verbose

4/ the best option (hint hint)

<input ng-model="book.title"
       ng-model-options="{ updateOn: 'blur', debounce: 200 }">

The NgModelController in this case just parses the config object and provides it to input and other directives that use ngModel without interpreting the meaning of the config (maybe except for debounce which should be handled by ngModel)

Pros:

  • terse and readable
  • doesn't affect implementation of ngModel
  • extensible
  • clearly the best option

Cons:

  • doesn't make @mhevery happy
  • updateOn option doesn't make sense for ngModel on custom components (like a map widget)

What do you guys think?

@tynman
tynman commented Mar 11, 2014

I like that 4/ keeps ng-model isolated, and that there's only one more
directive to learn.

Q1/ In this design, could updateOn be an array?

Q2/ Continuing your map widget example: Is updateOn a required field all
the time, or would you put it only where it makes sense (like
elements)?

Q3/ Assuming I made a map widget, there could be a few different "events"
that it uses internally. onPan, onRelease, onZoom, onPinDrop, etc. In this
case, none of those events are actually browser-based and I would have to
handle my model updates internally. In other words, there's no
NgModelController API change that goes with the onUpdate. Is that correct?

Q4/ With my shiny new map widget, assume . Does NgModelController handle
debouncing for me automatically, or do I need to tinker with
NgModelController to make that work?

-- Tyler

On Tue, Mar 11, 2014 at 3:33 PM, Igor Minar notifications@github.comwrote:

We brainstormed about this today with @auser https://github.com/auserand
@mhevery https://github.com/mhevery and considered many syntaxes and
implementation approaches, here is the recap:

1/ everything in ng-model syntax

Pros:

  • terse and readable
  • extensible

Cons:

  • will make error checking worse because where path evaluates to an object (currently throws a non-writable
    expression error) will be indistinguishable from the config object.
  • according to @mhevery https://github.com/mhevery, ngModel is not
    responsible for handling input events, so the event config should not be
    part of ngModel config

2/ explicit ng-model-* attributes

Pros:

  • explicit
  • doesn't make error-checking worse

Cons:

3/ similar to 2/ but event name is defined via ng-input specific attribute

Pros:

Cons:

  • possibly non-intuitive from app developer's perspective
  • the same as 2/
  • verbose

4/ the best option (hint hint)

The NgModelController in this case just parses the config object and
provides it to input and other directives that use ngModel without
interpreting the meaning of the config (maybe except for debounce which
should be handled by ngModel)

Pros:

  • terse and readable
  • doesn't affect implementation of ngModel
  • extensible
  • clearly the best option

Cons:

What do you guys think?

Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/pull/2129#issuecomment-37352321
.

@mgol
Member
mgol commented Mar 11, 2014

@IgorMinar I like 4/. While ngModel is not responsible for input events by itself, the declaration is in a separate directive (how about naming it ng-model-update or ng-model-update-options, though?), it's intuitive for developers + what you said.

@petebacondarwin
Member

Have you accounted for the fact that the directive can be applied to any element, not just input elements? The obvious use case being:

<form ng-model-options="...">
</form>

Since this would be a directive of its own, which ngModel and other inputs would simply require, it is not necessary for ngModel to parse it and pass it to input directives.  They would get it themselves.  NgModelController would require it and take what it needs, i.e. debounce stuff.  The inputs would get it themselves if they need it.

@lrlopez
Contributor
lrlopez commented Mar 11, 2014

I agree with Pete. Being able to apply the options to any element and its descendants is a nice feature as it adds flexibility and allows being less verbose. In fact this is already done in the PR.

Regarding the 4th approach, I concur with @tynman: we should set a way to specify more than one triggering event. We could allow an array in the updateOn option while keeping the default event for those cases when the user want to add new events instead of replacing them. We may even allow specifying an object with a custom timeout for every event, if it doesn't convolute too much the syntax.

As Pete said, custom components could (optionally) require ng-model-options and retrieve what it makes sense for them.

@wardbell

What is the interaction with ngChange? Will that fire when the value is written through (after debounce) or as the user types? Both?

@auser
Contributor
auser commented Mar 12, 2014

@tynman

For /1, it could be an array, but I'd be afraid that that's less readable than an object with descriptive keys. I'm wondering if it's worth it to maintain a secondary option at the risk of being more complex in use... what do you think?

2/ I don't think we'd want to make a second directive a requirement. We can assume a reasonable default and allow a second directive to allow modifications of those. I think ng-model-options (or ng-model-update-options) would be completely optional. If we were to require it, we'd have to go with /1 option, which is pretty ugly (IMHO).

3/ Regardless of the event type that gets fired, NgModelController won't be updated. I think if we want to include debounce as an option, it makes sense to be placed in NgModelController. This makes it reasonable to think that the directive's primary responsibility is to collate the options and provide other directives access to these options. For events like onZoom, onPan, etc., these only make sense for the map directive.

@mzgol I agree, ng-model-update-options make sense considering we are interested in updating ngModel only on updates. ng-model-options might be more extensible for use cases we're not considering right now.

@petebacondarwin +1

@lrlopez
Contributor
lrlopez commented Mar 17, 2014

Any news on this?

@petebacondarwin
Member

@lrlopez - @auser and I are going to pair on this, this week. Watch this space.

@auser
Contributor
auser commented Mar 18, 2014

Following on from the brain-storming and feedback, we’re proposing the final API for ngModelOptions. Unless there are any serious concerns with this, we would like to get the initial implementation complete this week.

<input ng-model="book.title"
       ng-model-options="{ updateOn: 'blur', debounce: 200 }">

Premise: ngModelUpdateOptions will contain an extensible collection of options for a directive that requires ngModel. The directive will be responsible for containing a list of options to handle updating ng-model.

@name ngModelOptions
@param Object

The object options:

updateOn: {string|array|object}:

The updateOn key parameter defines what events will update the ngModel of the directive. If updateOn is not defined, ngModel will update follow the current default behavior and update immediately.

  • When a string, it will assume there is a single event to update the ngModel.
  • When updateOn is passed an array, ngModel will be updated by any of the events
  • When updateOn is an object, each key of the object will be considered a list of the events, whereas the value of the keys is a number that indicates a debounce update.

debounce: {integer}:

The debounce option defines the minium delay between when the last event and when the update occurs in the ngModel. This is useful when we don’t want to batch updates to the ngModel value.

When debounce is a number, this value will be taken as the debounce delay.

<input ng-model="book.title"
       ng-model-options="{ updateOn: 'blur', debounce: myFn() }">
<input ng-model="book.title"
       ng-model-options="{ updateOn: ['blur', ‘keypress’], debounce: 200 }">
<input ng-model="book.title"
       ng-model-options="{ updateOn: {'blur': 0, ‘keypress’: 600, ‘default’: null }, debounce: 500 }">
<input ng-model="book.title"
       ng-model-options="{ updateOn: {'blur': 0, ‘keypress’: 600, ‘default’: null } }">

This functionality allows us to extend the object to future options.

@lrlopez
Contributor
lrlopez commented Mar 18, 2014

Thanks Ari. Just to be sure, a null key value means default debounce delay?

@petebacondarwin
Member

@lrlopez - yes, null or undefined would mean take the value defined in the debounce property or zero if that is not specified. Of course a zero value 0 would mean override the default value with a zero ms debounce, i.e. don't debounce. So

ng-model-options="{ updateOn: { blur: 0, keypress: null, mouseover: null }, debounce: 200 }"

would mean that the model is updated instantly when the input is blurred, and 200ms after the last key press or after the mouse has stopped moving over the input.

By the way, is debounce the best name here? Perhaps delay or debounce-delay would be more clear? @lrlopez & @auser what do you think?

@auser
Contributor
auser commented Mar 18, 2014

@lrlopez I personally like debounce, but I can be swayed to go with debounce-delay too... I don't think delay is descriptive enough

@lrlopez
Contributor
lrlopez commented Mar 18, 2014

I find delay too ambiguous. So, either debounce or debounce-delay should be ok. Maybe the later is too verbose, but it's clearer. I don't have any particular preference.

@gabrielmaldi
Contributor

My 2 cents on the subject: Knockout has a similar feature that used to be named throttle and has been renamed to rateLimit in newer versions.

@petebacondarwin
Member

Trouble with rateLimit and throttle, it doesn't map well to the units of the property, i.e. milliseconds. Both of these imply a kind of velocity.

Happy to stick with debounce if that is the concensus. Ari and I are preparing a task list for you @lrlopez :-)

@petebacondarwin
Member

One more question, if we have:

{ updateOn: { blur: 0, keyup: 500 }, debounce: 200 }

Would it make sense to interpret this as, "the blur trigger is not debounced, the keyup trigger is debounced with a 500ms delay, and all updates to ngModel are debounced by 200ms?

In other words, the input directive read the updateOn property and may do some debouncing before calling $setViewValue() but the NgModelController reads the debounce property and does its own debouncing of the call to $setViewValue independently of the input directives.

@tynman
tynman commented Mar 18, 2014

It's unfortunate that debounce is explicit for the default, and implicit
in updateOn.

At the risk of beating a dead horse, what about calling it updateDebounce and
defaultDebounce? I hate how verbose that is.

What about pulling the default debounce value into updateOn/
updateDebounce? I.e.,

{ updateOn: { blur: 0, keyup: 500, default: 200 }

This assumes there's no UI event called default. :-\

On Tue, Mar 18, 2014 at 4:31 PM, Pete Bacon Darwin <notifications@github.com

wrote:

One more question, if we have:

{ updateOn: { blur: 0, keyup: 500 }, debounce: 200 }

Would it make sense to interpret this as, "the blur trigger is not
debounced, the keyup trigger is debounced with a 500ms delay, and all
updates to ngModel are debounced by 200ms
?

Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/pull/2129#issuecomment-37997042
.

@petebacondarwin
Member

@tynman - we are indeed using "default" as a trigger name. This refers to the current built in trigger that ng-model uses right now.

@auser
Contributor
auser commented Mar 19, 2014

Alright, so it seems we've settled on an implementation. Here's the list of TODOs to get it implemented:

  • Move the debounce stuff into its own service. I’m in favor of calling the service $debounce.

It’s usage is in the familiar form with the arguments:

  1. function to call
  2. delay in milliseconds
  3. Boolean indicating immediate execution
var fn = $debounce(function() {}, 1000, false);
fn(); // Debounce after delay of 1 sec
  • Consolidate ng-update-model-on and ng-update-model-debounce into one directive, `ng-model-options’. This is an attribute directive which takes an angular expression as its attribute value. The expression’s value must be an object.

The object can provide the following properties:

  • updateOn {string|array.|object.<string, number>} - describes the triggers that will cause the model to be updated. The possible trigger names will be “default” (the current default trigger) and any jQuery event name, such as “blur”, “mouseup”, “keypress”, etc.
    1. string: a single trigger that will cause the model to be updated
    2. array.: a collection of triggers that will cause the model to be updated, such as [‘blur’, ‘click’]
    3. object.<string, number>: where the property keys match a trigger name and the property value is the number of milliseconds to debounce this particular trigger. If the value is null then the default delay is applied, see above.
  • debounce- the default number of milliseconds that must elapse between trigger events before the model is updated. If the updateOn property is an object the values of the properties of the object will override this value.
  • Strip out the new directives description from the ngInput parameter list
  • Update the examples so they use the new <example> tag
  • Add unit tests for the $debounce service
  • Add Protractor specs for the examples
  • Fix src/ng/directive/input.js:L1523
  • We can leave the “fix(ngScenario): Restrict radio/checkbox check switch in IE<9” commit in the PR, as it does no harm.

@lrlopez Interested in tackling these changes?

@lrlopez
Contributor
lrlopez commented Mar 19, 2014

@auser, sure! It'll take some time though.

Pete wrote a $debounce service for angular-ui some time ago. With very minor changes it could be adapted to the core. @petebacondarwin, could you prepare the PR? I would add test/docs if you don't have enough time to spare.

@petebacondarwin
Member

Sure. I can do that today
On 19 Mar 2014 08:23, "Luis Ramón López" notifications@github.com wrote:

@auser https://github.com/auser, sure! It'll take some time though.

Pete wrote a $debounce servicehttps://github.com/angular-ui/angular-ui-OLDREPO/blob/debounce/modules/services/debounce/debounce.jsfor angular-ui some time ago. With very minor changes it could be adapted
to the core. @petebacondarwin https://github.com/petebacondarwin, could
you prepare the PR? I would add test/docs if you don't have enough time to
spare.

Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/pull/2129#issuecomment-38026876
.

@lgersman

+1. Thats how a community work !

@petebacondarwin
Member

I have submitted a PR for the $debounce service. Please do go and review, etc.

@shahata
Contributor
shahata commented Mar 21, 2014

I have a few comments on this:

  1. Regarding @petebacondarwin question about $setViewValue: I think it is better if the input directives only know what triggers they should wait for when updating the view value and know nothing about the debounce. When calling $setViewValue, the input directives should simply pass this trigger name as an additional argument so that ngModelController will be able to decide how long the debounce should be (or which debounced function to use - see point 3).
  2. I think that we should consider the possibility that the model value might get updated while a debounce is pending. In this case I think that the correct behavior should be to cancel the pending debounce (I have done something similar in a debounce directive I wrote by hooking $render, but obviously this can be done much more elegantly in this PR). Example: let's say I have a debounced search input and a button that clears the search term. The button has an ng-click function that clears the same variable used in the debounced search input ng-model. That means that if I click the clear button a few moments after I finish typing, we will have a race condition where it is possible that the search term will be cleared and then immediately go back to the value we had when the debounce was invoked.
  3. I am a bit confused with the notion of debounce value for each trigger. How does this work exactly? Do we have a different debounced function for each trigger? Or do we have one debounced function which can somehow update the debounce interval in run-time?
    • Different function for each trigger might cause weird scenarios like {keypress: 350, blur: 100} if I press a key and then immediately blur - what exactly happens? $setViewValue after 100ms and then again after 350ms? Maybe both calls will be with the same view value and the user won't notice it, but it seems wrong to me.
    • One debounced function sounds much less confusing, but the $debounce service will need to support this. Another option is to have multiple functions and cancel all pending debounce before running the a function for a new trigger, but this also seems like something $debounce should support internally.
@petebacondarwin
Member

@shahata - thanks for the feedback - (nice debounce project by the way).

  1. I feel a little uncomfortable about passing the name of the event through to NgModelController as this puts an additional responsibility and need for knowledge on the NgModelController, but I am willing to be convinced otherwise. The idea would actually be inline with the earlier suggestions by Misko, Ari and Igor. If this were implemented, I imagine that the ng-model-options would change:
  • the updateOn property would become a simple string or array of strings { string | Array.<string> }, where each string was the name of an event that should be monitored by the input control and cause $setViewValue to be called.
  • a new parameter would be added to $setViewValue(value, trigger) where the trigger parameter would be passed by input element - null or undefined meaning the default trigger.
  • the debounce property of ng-model-options would take a number or an object where the keys match trigger names and the value is a number, { number | Object.<string,number} }. The number would be the debounce delay for the given trigger name, if the property is an object, or for all triggers, if only a number is provided.
  • the NgModelController would use this debounce property to manage debounce $setViewValue based on the trigger parameter.
  1. The way that @lrlopez has this implemented right now does not have a problem with the model being updated during a debounce and it doesn't require an explicit cancel method to be called on the debounced functions. Check out this Plunker: http://plnkr.co/edit/wmfngZhhX7r26GCtwYay?p=preview. I would expect that we have such tests in place to ensure that this is the case with the final implementation.
  2. This was a concern that @IgorMinar and I earlier in this comment thread but in practice it doesn't seem to be a problem. I think this is due to the fact that, as you say, the same value is being passed through to $setViewValue each time. While it is possible to build a algorithm that manages multiple possible debounce delays and ensures that they are aware of each other, this extra complexity does not seem to have any real benefit in practice. It would be great if you could put together a running example of a situation where this is problematic, since until then we have nothing to test against anyway.

Thanks again and keep the ideas coming.

@lrlopez
Contributor
lrlopez commented Mar 22, 2014

@shahata,
About your second and third comment: When a new event is received and there is a pending update, the old one is cancelled and a new one is created with the timeout associated to the latest event (which can be none). Whenever the update is triggered it will always call $setViewValue with the current input value. So, following your example, if you press the clear button it will have the intended effect.

@shahata
Contributor
shahata commented Mar 22, 2014

Hi @petebacondarwin & @lrlopez, thanks for your replies.

You are right - since $setViewValue is always called with the current input value, we should have no problem with the debounce being invoked after the model has changed or after a different debounce from a different trigger was invoked.

There is an interesting bug here though which we can see in the plunkr link @petebacondarwin sent (and I'm sure also in my directive implementation) - what if the model is updated while the debounce is pending, BUT it is updated with the same value it had before the debounced update? For example, you start typing the search box when it is empty and then click the clear button. In this case the model is already an empty string since $setViewValue was not called yet and the input value will not get updated by $render. This can be solved by changing the $scope.$watch in ngModelController to invoke $render while a debounce is pending, but it means that ngModelController must know about the debounce, which is another reason to always debounce in ngModelController and not in the input directives (my first point).

I believe that debouncing only in ngModelController is important also from aspect of duplicate code for each input directive (currently duplicated in text input and checkbox - but not present radio button and select box - is there a reason for it not being present in radio/select? or am I missing something?). Also this duplication will be required by anyone writing a directive which works with ng-model...

@lrlopez
Contributor
lrlopez commented Mar 22, 2014

For example, you start typing the search box when it is empty and then click the clear button. In this case the model is already an empty string since $setViewValue was not called yet and the input value will not get updated by $render.

Shahar, I don't get your point. Why should be the model anything but an empty string in your example? If the update hasn't been triggered yet and you make another change, it should be debounced and no model update should take place.

Also, the reason of debouncing not being implemented in radio buttons is given in the beginning of this PR:

One note about the missing support for timeouts in radio buttons: Radios are, in fact, different input controls that are tied to the same model. The timeout timer gets reset & launched on every input content change, but it can't know about the other radio controls' timers.

This has an unpleasant side effect: imagine you have 3 radio buttons bound to the same model with a timeout of 1 second. If you click the three options one after the other fast enough, after (approx.) one second there will be 3 timeouts that will trigger a model update three times in a row instead of just one.

I agree this limitation could be avoided if debouncing took place inside ngModelController, but we should be careful for unintended side effects. Should the team decide to go this way, the final implementation would look like @petebacondarwin proposed 3 replies above.

@shahata
Contributor
shahata commented Mar 22, 2014

@lrlopez, I'll try to describe the problem more clearly:

  1. open this plunker example: http://plnkr.co/edit/wmfngZhhX7r26GCtwYay?p=preview
  2. type 'aaa' in the search box
  3. click the 'clear' button immediately (before the $setViewValue is invoked by debounce)
  • Expected result:
    input value & model should be cleared (since this is the last operation the user did)
  • Actual result:
    input value is not cleared & ~1 second later the model changes to 'aaa' when the debounce calls $setViewValue
  • Why this happened:
    The $scope.$watch in ngModelController tested if the current value of the model is any different from ngModelController.$modelValue and since it was the same (because $setViewValue was not invoked yet), it did not call $render
  • How this can be fixed:
    If ngModelController knows there is a debounced action pending, it can workaround this issue
@petebacondarwin
Member

Good catch @shahata. I agree that we need to be able to deal with this scenario.

@petebacondarwin
Member

What @shahata is saying is that if you update the model programmatically while a debounce delay is in progress the model update is not recognised as an update and so is overridden by the delayed debounce.

Of course one way around this would be to use immediate true on the debouncing, which means that we never get a delayed update...

@shahata
Contributor
shahata commented Mar 22, 2014

@petebacondarwin, I think using immediate true when debouncing user input will cause bad user experience where the user types stuff and it does not update the model.

@petebacondarwin
Member

Good point!

@caitp
Contributor
caitp commented Mar 22, 2014

programmatic model updates don't need to go through the debounce pipeline, I think we can structure it so that they don't.... as for the model not being updated due to debounce, this is really the whole point of debouncing, that's what people have been asking for --- and it would be opt-in. So I'm not sure there's really an issue here. Also note, the user doesn't really see what's in the model, unless you're explicitly showing it to them. The browser will still show you the contents of whatever form control you're manipulating, though, so this covers the "bad user experience" point, I believe.

@shahata
Contributor
shahata commented Mar 22, 2014

@caitp - Programmatic model updates don't go through the debounce pipeline. That's already how it works in this PR, but there is a minor bug in the implementation which I mentioned a few comments ago. Also, the use case I have in mind when I think about debouncing, is having an input and next to it some view that is generated form/with this input's value like a full text search on a long list, or a translation of the text you wrote. This means that I am always showing the user the "result" of the model and while I don't want the result to recalculated with every key stroke, I do want it to be calculated once the user is done typing. I believe this is what most ppl are asking for.

@petebacondarwin
Member

I think I am convinced. NgModelController should handle the debouncing from within $setViewValue. It can then also coordinate cancelling a delayed update from the view if the model changes in the meantime.

@petebacondarwin
Member

I had a play around with NgModelController owning the debounce. I like it but it is going to be difficult to deal with model updates... #6813

@lrlopez
Contributor
lrlopez commented Mar 23, 2014

I'll wait until @auser says what he thinks about this before continuing working on the PR.

@lrlopez
Contributor
lrlopez commented Mar 25, 2014

I may have fixed the problem, but is not ideal at all; consider this as WIP.

I moved timeout into ngModelController is you can check from $watch if there is a pending debounce:

  $scope.$watch(function ngModelWatch() {
    var value = ngModelGet($scope);

    // if scope model value and ngModel value are out of sync
    if ((ctrl.$modelValue !== value) || (ctrl.$timeout)) {

      var formatters = ctrl.$formatters,
          idx = formatters.length;

      ctrl.$modelValue = value;
      while(idx--) {
        value = formatters[idx](value);
      }

      if ((ctrl.$viewValue !== value) || (ctrl.$timeout)) {
        ctrl.$viewValue = value;
        ctrl.$render();
      }
    }

    return value;
  });

See the plunker: http://plnkr.co/edit/CO4L2UY2OsEQZdJZxs7m?p=preview

The big big problem with this approach is that it updates the view each digest. I will try storing the pending view value so we can filter when there is an actual change.

@lrlopez
Contributor
lrlopez commented Mar 25, 2014

Ok, adding $pendingValue seems to work. Can someone have a look into this?

Updated plunker: http://plnkr.co/edit/4rT31TURMO8a0TQDAjNC?p=preview

@petebacondarwin
Member

Sadly this approach falls down when you have a $parser, which modifies the view value.

See http://plnkr.co/edit/g8hoJTz8xQKM4vDGCTGW?p=preview

Triggering any digest before the debounce completes overwrites the view value.

We need to compare the potentially changes model value against what the view value will be when the parsers complete. This is what @shahata was suggesting by moving everything except calling setModelValue out of the debounced function. But I am still concerned about running the $parsers (and so validators) against a view value that could be overridden by a change to the model before it arrives... I am less concerned about performance impact, now, since I realise that we are not running a digest until the debounce completes.

@shahata
Contributor
shahata commented Mar 25, 2014

@petebacondarwin, do you have an idea for a specific scenario that might have problems with running $parsers without setting the model? It's hard for me to think about anything that could go wrong...

@petebacondarwin
Member

What if the $parser sets the validity to false for the view that is then cancelled? The view would not be invalid but the $valid would be false.

@shahata
Contributor
shahata commented Mar 25, 2014

@petebacondarwin I don't think it is relevant that the ngModelSet was cancelled, it is not the one responsible for re-checking the validity - the $formatters who see the programmatic model change are... Input validations should be done both in $parsers and $formatters exactly for this reason and unrelated to this PR, so this shouldn't be a problem (take a look in existing validations in angular core, like ngMaxlength, ngMinlength, ngPattern)

I just checked and I see that the importance of testing validity also in $formatters is not documented. I think this should be fixed. It means that anyone who sets validity only using $parsers does not get back to a valid state when the value is changed through a programmatic model update. (even before this PR)

I never wrote a validation directive, but I think I got this right. Is there something I'm missing?

@petebacondarwin
Member

@shahata - there is no formal definition for what you can put in formatters and parsers, so even if what you're suggesting was the right practice - which it might well be - we cannot assume that people are doing this. So in effect we have no control over what they are doing inside these functions.

This means that we can't really run the pipeline on pending values, which we may cancel. But equally without running them we have no reliable way of knowing whether a programmatic change to the model value should cancel a pending update.

It would be nice if we could somehow sandbox the formatters and parsers so that they can be run without being able to effect the real scope and that way get the model value for comparison in the watch but I suspect that this may be infeasible without a major API change.

@shahata
Contributor
shahata commented Mar 26, 2014

My point is that ppl that are affected by this change are already affected by the same issues of programmatically updating the model without relation to this PR... If you update the model of someone who only added to $parsers and not to $formatters, the validity will not get updated, even without this PR.

@petebacondarwin
Member

It is true that validity will not get updated if someone writes a validation directive and only checks the validity in the $parser but that is not necessarily a bug. They may have a good reason for allowing "non-valid" programmatic updates.

Imagine, for example, you have a user name field. After people have been using the application for a while you add in new validity constraint that going forward user names must be at least 10 characters long. Now if you enforce this on both the parser and the formatter then when people go to edit their user name the input box will be empty with an error message.

In the future we need a much more sophisticated validation framework that is independent of transforming values between the model and the view. But for now we have to work with what we have.

@shahata
Contributor
shahata commented Mar 27, 2014

Yes, I understand. But the question remains - what problematic side effects we might get by debouncing only the ngModelSet? I think we agree that there is no inconsistency between how it behaves today and how it will behave with this change regarding setting of validity in $parsers.

What if the $parser sets the validity to false for the view that is then cancelled? The view would not be invalid but the $valid would be false.

Is there any other problem you think can arise by this change? I really can't think of a single example where it will cause a problem.

@petebacondarwin
Member

@IgorMinar and I paired on this last night and we identified a fundamental problem with the whole issue of implicitly cancelling debounced events. Even if we were to compute the $modelValue outside of the debounced function there is still the problem of knowing whether the model has really been reset or whether it is just the same as it was before.

Consider the following situation, where we have a text input and a button that will clear it:

<form name="myForm"
  <input ng-model="myVal" name="myInput" ng-model-options="{ debounce: 1000 }">
  <button ng-click="myVal = ''">Clear</button>
  <button ng-click="foo = 'bar'">Trigger Digest</button>
</form>

Situation 1: (Good)
0. Input is empty.

  1. User types xyz into the input.
  2. We process the $parsers and set the $modelValue to xyz, but we debounce the update to myVal (i.e. the call to modelValueSet().
  3. 1000ms after the user stops typing we update the real model (i.e. calling modelValueSet(scope, 'xyz')) and run a $digest

This works fine.

Situation 2: (Good)
0. Input is empty.

  1. User types xyz into the input.
  2. We process the $parsers and set the $modelValue to xyz, but we debounce the update to myVal (i.e. the call to modelValueSet().
  3. Before 1000ms elapse the user clicks the "Clear" button. This sets myVal = '', which it already was, and runs a $digest.
  4. The $watch in the NgModelController runs, compares the value of the model (i.e. myVal) against the current $modelValue, which are different.
  5. We assume therefore that the model has been updated while waiting for the pending view change to resolve and cancel the pending view change.

This is also fine. So far this is what we have been considering and by running the pending view value through the $parsers we don't fall foul of the various issues that have been raised.

Situation 3: Bad
0. Input is empty.

  1. User types xyz into the input.
  2. We process the $parsers and set the $modelValue to xyz, but we debounce the update to myVal (i.e. the call to modelValueSet().
  3. Before 1000ms elapse the user clicks the "Trigger Digest" button. This executes an arbitrary expression which has absolutely nothing to do with the myVal model value, and runs a $digest.
  4. The $watch in the NgModelController runs, compares the value of the model (i.e. myVal) against the current $modelValue, which are different.
  5. We assume therefore that the model has been updated while waiting for the pending view change to resolve and cancel the pending view change.

Oh, dear! It seems that any $digest that is triggered while waiting for a pending view change to resolve causes the change to be cancelled. This digest can be triggered by a whole load of things, from clicks and key presses, to $http requests and $timeout calls resolving.

The way that our dirty checking method works means that there is no way to tell the difference between a digest where a model change is actually resetting the value that is pending an update and a digest where the model has not been updated but happens to be different to the pending value.

Without a major revision of the way model changes are identified, in AngularJS generally, there is no way to get what we want explicitly. This may be fixed in Angular v2 but for v1.3, our suggested solution is to provide an explicit API for cancelling pending updates on NgModelController, which the user must apply if they really are updating a debounced value, as in the case of the "Clear" button. Here is how the example above would look in this case:

<form name="myForm"
  <input ng-model="myVal" name="myInput" ng-model-options="{ debounce: 1000 }">
  <button ng-click="myForm.myInput.$cancelDebounce(); myVal = ''">Clear</button>
  <button ng-click="foo = 'bar'">Trigger Digest</button>
</form>

You can see that we are providing an explicit method on NgModelController, $cancelDebounce, which you call whenever you are aware that the model change you are making should override any pending view changes. While this is somewhat verbose and requires the application developer to proactively apply the call, it is, at least very clear in its intention, having little hidden magical characteristics, and gives full control to the developer. To mitigate application developers from failing to apply this, we should provide a very prominent warning block in the documentation right next to the information about debouncing, so as to ensure that developers are aware of their responsibilities when debouncing inputs.

@lrlopez, @auser and @shahata - can you take a look at this idea? I don't believe there is a better solution, which would remove the need for this explicit API change. Let's mull it over for a day or two. If nothing better comes to light then I suggest that we (@lrlopez) put together a new PR, which incorporates all our ideas so far.

@shahata
Contributor
shahata commented Mar 28, 2014

Wow. I can't believe I didn't notice this issue :(. I agree that explicit cancel is our best option. This means that the $scope.$watch will return immediately if a debounce is pending, right? And we keep running the $parsers immediately and debounce only ngModelSet()? I'd be happy to help with this new PR.

@petebacondarwin
Member

@shahata - now that we are not going to try to implicitly cancel the debounce we don't need to run anything before the debounce resolves. This is the safest solution since we do not end up with periods of time where the $viewValue, NgModelController properties and the model are inconsistent with each other.

@shahata
Contributor
shahata commented Mar 28, 2014

@petebacondarwin Yes, that's what I thought you'd say :) I'm still not convinced it wouldn't be better to let $parsers set the validity immediately, but let's see how it goes...

@lrlopez
Contributor
lrlopez commented Mar 28, 2014

So, just to be sure, you want to keep the current PR behavior and just introduce a $cancelDebounce() method into ngModelController. Isn't it?

I'll start working on it now. By the way, now that the model will hold the timeout reference we could implement debouncing into every input type, including radiobuttons.

@petebacondarwin
Member

@lrlopez - make it so! Just to be clear, we are looking at one new directive:

ng-model-options = "{ updateOn: ..., debounce: ...}"

where updateOn is either a string (one trigger) or an array of strings (many triggers) and debounce is either a number (debounce delay) or an object. If it is an object then the key is the trigger name and the value is the debounce delay for that trigger.

The individual input directives will be responsible for dealing with updateOn and the NgModelController is responsible for dealing with debounce. To achieve this the changes to NgModelControllers API are:

  • $setViewValue will take a second optional parameter, which is the name of the trigger that updated the model.
  • A new method $cancelDebounce(), which will cancel any pending debounced view updates.

$setViewValue will store a pending view value and trigger the real update after the specified delay. Each new call to $setViewValue will cancel any pending debounce (i.e. like calling $cancelDebounce()).
We are going to ditch the $debounce service, since it doesn't provide a huge benefit at this stage and would add to the public API, which must be supported going forward.

@petebacondarwin
Member

@shahata - in the long run we should decouple the transformation pipeline from the validation.

@lrlopez
Contributor
lrlopez commented Mar 28, 2014

@petebacondarwin Ok, working on it.

@lrlopez
Contributor
lrlopez commented Mar 28, 2014

Pete, some of the features were already coded by you in PR #6813. If you agree I could start working over your changes. This way you'll be credited too.

@petebacondarwin
Member

@lrlopez - as you wish. I am happy for you to get the credit. I'll be pleased just to get this in.

@lrlopez
Contributor
lrlopez commented Mar 30, 2014

Pete, another question: Should I keep debouncing support into timeout or move the feature directly into ngModelController?

@petebacondarwin
Member

I think it is best if we minimize the changes to public APIs. Which in this case means move it all into NgModelController. This also means that @shahata's 3rd party debounce service is still a viable project!

@petebacondarwin
Member

@lrlopez - Find me on G+ if you want to Hangout and discuss anything.

@lrlopez
Contributor
lrlopez commented Mar 30, 2014

Update: I'm almost done. I'm just having some problems with a couple of Protractor tests, but it should be easy to fix.

@petebacondarwin
Member

Go @lrlopez!
Looking forward to seeing it.

On 31 March 2014 00:14, Luis Ramón López notifications@github.com wrote:

Update: I'm almost done. I'm just having some problems with a couple of
Protractor tests, but it should be easy to fix.

Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/pull/2129#issuecomment-39043580
.

@lrlopez lrlopez added a commit to lrlopez/angular.js that referenced this pull request Apr 1, 2014
@lrlopez lrlopez feat(ngModelOptions): Model update behavior can now be customized
By default, any change on the content will trigger an immediate model update
and form validation. This PR implements a new directive `ng-model-options`
that allow you to override this default behavior in several ways.

You should specify an object with the different parameters.

For example, it allows to trigger an update only when a particular event or
list of events is received by the input using the `updateOn` key. Should you
need multiple events, just assign an array to it.

I.e. `ng-model-options="{ updateOn: 'blur' }"` will update and validate only
after the control loses focus.

If you want to keep the default behavior and just add new events that may
trigger the model update and validation, add "default" as one of the specified
events.

I.e. `ng-model-options="{ updateOn: ['default','submit'] }"`

Also, with the `debounce` option, `ng-model-options` will allow defering the
actual model update until a timer expires. The timer will be reset each time
an event is triggered.

I.e. `ng-model-options="{ debounce: 500 }" for 500ms after the latest event.

Custom timeouts for each event can be set for each event if you use an object
in `debounce`. This can be useful to force immediate updates on some specific
circunstances (like blur events).

I.e. `ng-model-options="{ updateOn: ['default', 'blur'], debounce: { default: 500, blur: 0} }"`

You can use the directive in any tag so its contents became the default settings
for any child control, although they can be overriden.

Closes #1285, #2129
d91d65d
@lrlopez lrlopez added a commit to lrlopez/angular.js that referenced this pull request Apr 1, 2014
@lrlopez lrlopez feat(ngModelOptions): Model update behavior can now be customized
By default, any change on the content will trigger an immediate model update
and form validation. This PR implements a new directive `ng-model-options`
that allow you to override this default behavior in several ways.

You should specify an object with the different parameters.

For example, it allows to trigger an update only when a particular event or
list of events is received by the input using the `updateOn` key. Should you
need multiple events, just assign an array to it.

I.e. `ng-model-options="{ updateOn: 'blur' }"` will update and validate only
after the control loses focus.

If you want to keep the default behavior and just add new events that may
trigger the model update and validation, add "default" as one of the specified
events.

I.e. `ng-model-options="{ updateOn: ['default','submit'] }"`

Also, with the `debounce` option, `ng-model-options` will allow defering the
actual model update until a timer expires. The timer will be reset each time
an event is triggered.

I.e. `ng-model-options="{ debounce: 500 }" for 500ms after the latest event.

Custom timeouts for each event can be set for each event if you use an object
in `debounce`. This can be useful to force immediate updates on some specific
circunstances (like blur events).

I.e. `ng-model-options="{ updateOn: ['default', 'blur'], debounce: { default: 500, blur: 0} }"`

You can use the directive in any tag so its contents became the default settings
for any child control, although they can be overriden.

Closes #1285, #2129
fff1f1c
@lrlopez lrlopez added a commit to lrlopez/angular.js that referenced this pull request Apr 1, 2014
@lrlopez lrlopez feat(ngModelOptions): Model update behavior can now be customized
By default, any change on the content will trigger an immediate model update
and form validation. This PR implements a new directive `ng-model-options`
that allow you to override this default behavior in several ways.

You should specify an object with the different parameters.

For example, it allows to trigger an update only when a particular event or
list of events is received by the input using the `updateOn` key. Should you
need multiple events, just assign an array to it.

I.e. `ng-model-options="{ updateOn: 'blur' }"` will update and validate only
after the control loses focus.

If you want to keep the default behavior and just add new events that may
trigger the model update and validation, add "default" as one of the specified
events.

I.e. `ng-model-options="{ updateOn: ['default','submit'] }"`

Also, with the `debounce` option, `ng-model-options` will allow defering the
actual model update until a timer expires. The timer will be reset each time
an event is triggered.

I.e. `ng-model-options="{ debounce: 500 }" for 500ms after the latest event.

Custom timeouts for each event can be set for each event if you use an object
in `debounce`. This can be useful to force immediate updates on some specific
circumstances (like blur events).

I.e. `ng-model-options="{ updateOn: ['default', 'blur'], debounce: { default: 500, blur: 0} }"`

You can use the directive in any tag so its contents became the default settings
for any child control, although they can be overriden.

Closes #1285, #2129
f9eb1d8
@lrlopez lrlopez added a commit to lrlopez/angular.js that referenced this pull request Apr 1, 2014
@lrlopez lrlopez feat(ngModelOptions): Model update behavior can now be customized
By default, any change on the content will trigger an immediate model update
and form validation. This PR implements a new directive `ng-model-options`
that allow you to override this default behavior in several ways.

You should specify an object with the different parameters.

For example, it allows to trigger an update only when a particular event or
list of events is received by the input using the `updateOn` key. Should you
need multiple events, just assign an array to it.

I.e. `ng-model-options="{ updateOn: 'blur' }"` will update and validate only
after the control loses focus.

If you want to keep the default behavior and just add new events that may
trigger the model update and validation, add "default" as one of the specified
events.

I.e. `ng-model-options="{ updateOn: ['default','submit'] }"`

Also, with the `debounce` option, `ng-model-options` will allow defering the
actual model update until a timer expires. The timer will be reset each time
an event is triggered.

I.e. `ng-model-options="{ debounce: 500 }" for 500ms after the latest event.

Custom timeouts for each event can be set for each event if you use an object
in `debounce`. This can be useful to force immediate updates on some specific
circumstances (like blur events).

I.e. `ng-model-options="{ updateOn: ['default', 'blur'], debounce: { default: 500, blur: 0} }"`

You can use the directive in any tag so its contents became the default settings
for any child control, although they can be overriden.

Closes #1285, #2129
9bd4aa5
@lrlopez lrlopez added a commit to lrlopez/angular.js that referenced this pull request Apr 1, 2014
@lrlopez lrlopez feat(ngModelOptions): Model update behavior can now be customized
By default, any change on the content will trigger an immediate model update
and form validation. This PR implements a new directive `ng-model-options`
that allow you to override this default behavior in several ways.

You should specify an object with the different parameters.

For example, it allows to trigger an update only when a particular event or
list of events is received by the input using the `updateOn` key. Should you
need multiple events, just assign an array to it.

I.e. `ng-model-options="{ updateOn: 'blur' }"` will update and validate only
after the control loses focus.

If you want to keep the default behavior and just add new events that may
trigger the model update and validation, add "default" as one of the specified
events.

I.e. `ng-model-options="{ updateOn: ['default','submit'] }"`

Also, with the `debounce` option, `ng-model-options` will allow deferring the
actual model update until a timer expires. The timer will be reset each time
an event is triggered.

I.e. `ng-model-options="{ debounce: 500 }" for 500ms after the latest event.

Custom timeouts for each event can be set for each event if you use an object
in `debounce`. This can be useful to force immediate updates on some specific
circumstances (like blur events).

I.e. `ng-model-options="{ updateOn: ['default', 'blur'], debounce: { default: 500, blur: 0} }"`

You can use the directive in any tag so its contents became the default settings
for any child control, although they can be overridden.

Closes #1285, #2129
1b8f296
@lrlopez lrlopez added a commit to lrlopez/angular.js that referenced this pull request Apr 2, 2014
@lrlopez lrlopez feat(ngModelOptions): Model update behavior can now be customized
By default, any change on the content will trigger an immediate model update
and form validation. This PR implements a new directive `ng-model-options`
that allow you to override this default behavior in several ways.

You should specify an object with the different parameters.

For example, it allows to trigger an update only when a particular event or
list of events is received by the input using the `updateOn` key. Should you
need multiple events, just assign an array to it.

I.e. `ng-model-options="{ updateOn: 'blur' }"` will update and validate only
after the control loses focus.

If you want to keep the default behavior and just add new events that may
trigger the model update and validation, add "default" as one of the specified
events.

I.e. `ng-model-options="{ updateOn: ['default','submit'] }"`

Also, with the `debounce` option, `ng-model-options` will allow deferring the
actual model update until a timer expires. The timer will be reset each time
an event is triggered.

I.e. `ng-model-options="{ debounce: 500 }" for 500ms after the latest event.

Custom timeouts for each event can be set for each event if you use an object
in `debounce`. This can be useful to force immediate updates on some specific
circumstances (like blur events).

I.e. `ng-model-options="{ updateOn: ['default', 'blur'], debounce: { default: 500, blur: 0} }"`

You can use the directive in any tag so its contents became the default settings
for any child control, although they can be overridden.

Closes #1285, #2129
640ef4a
@lrlopez lrlopez added a commit to lrlopez/angular.js that referenced this pull request Apr 2, 2014
@lrlopez lrlopez feat(ngModelOptions): Model update behavior can now be customized
By default, any change on the content will trigger an immediate model update
and form validation. This PR implements a new directive `ng-model-options`
that allow you to override this default behavior in several ways.

You should specify an object with the different parameters.

For example, it allows to trigger an update only when a particular event or
list of events is received by the input using the `updateOn` key. Should you
need multiple events, just assign an array to it.

I.e. `ng-model-options="{ updateOn: 'blur' }"` will update and validate only
after the control loses focus.

If you want to keep the default behavior and just add new events that may
trigger the model update and validation, add "default" as one of the specified
events.

I.e. `ng-model-options="{ updateOn: ['default','submit'] }"`

Also, with the `debounce` option, `ng-model-options` will allow deferring the
actual model update until a timer expires. The timer will be reset each time
an event is triggered.

I.e. `ng-model-options="{ debounce: 500 }" for 500ms after the latest event.

Custom timeouts for each event can be set for each event if you use an object
in `debounce`. This can be useful to force immediate updates on some specific
circumstances (like blur events).

I.e. `ng-model-options="{ updateOn: ['default', 'blur'], debounce: { default: 500, blur: 0} }"`

You can use the directive in any tag so its contents became the default settings
for any child control, although they can be overridden.

Closes #1285, #2129
b879484
@lrlopez lrlopez added a commit to lrlopez/angular.js that referenced this pull request Apr 2, 2014
@lrlopez lrlopez feat(ngModelOptions): Model update behavior can now be customized
By default, any change on the content will trigger an immediate model update
and form validation. This PR implements a new directive `ng-model-options`
that allow you to override this default behavior in several ways.

You should specify an object with the different parameters.

For example, it allows to trigger an update only when a particular event or
list of events is received by the input using the `updateOn` key. Should you
need multiple events, just assign an array to it.

I.e. `ng-model-options="{ updateOn: 'blur' }"` will update and validate only
after the control loses focus.

If you want to keep the default behavior and just add new events that may
trigger the model update and validation, add "default" as one of the specified
events.

I.e. `ng-model-options="{ updateOn: ['default','submit'] }"`

Also, with the `debounce` option, `ng-model-options` will allow deferring the
actual model update until a timer expires. The timer will be reset each time
an event is triggered.

I.e. `ng-model-options="{ debounce: 500 }" for 500ms after the latest event.

Custom timeouts for each event can be set for each event if you use an object
in `debounce`. This can be useful to force immediate updates on some specific
circumstances (like blur events).

I.e. `ng-model-options="{ updateOn: ['default', 'blur'], debounce: { default: 500, blur: 0} }"`

You can use the directive in any tag so its contents became the default settings
for any child control, although they can be overridden.

Closes #1285, #2129
c93c454
@lrlopez lrlopez added a commit to lrlopez/angular.js that referenced this pull request Apr 3, 2014
@lrlopez @lrlopez lrlopez + lrlopez feat(ngModelOptions): Model update behavior can now be customized
By default, any change to the content will trigger an immediate model update
and form validation. This PR implements a new directive `ng-model-options`
that allow you to override this default behavior in several ways.

You should specify an object with the different parameters.

For example, it allows to trigger an update only when a particular event or
list of events is received by the input using the `updateOn` key. Should you
need multiple events, just assign an array to it.

I.e. `ng-model-options="{ updateOn: 'blur' }"` will update and validate only
after the control loses focus.

If you want to keep the default behavior and just add new events that may
trigger the model update and validation, add "default" as one of the specified
events.

I.e. `ng-model-options="{ updateOn: ['default','submit'] }"`

Also, with the `debounce` option, `ng-model-options` will allow deferring the
actual model update until a timer expires. The timer will be reset each time
an event is triggered.

I.e. `ng-model-options="{ debounce: 500 }" for 500ms after the latest event.

Custom timeouts for each event can be set for each event if you use an object
in `debounce`. This can be useful to force immediate updates on some specific
circumstances (like blur events).

I.e. `ng-model-options="{ updateOn: ['default', 'blur'], debounce: { default: 500, blur: 0} }"`

You can use the directive in any tag so its contents became the default settings
for any child control, although they can be overridden.

Closes #1285, #2129
837ce52
@lrlopez lrlopez added a commit to lrlopez/angular.js that referenced this pull request Apr 3, 2014
@lrlopez @lrlopez lrlopez + lrlopez feat(ngModelOptions): Model update behavior can now be customized
By default, any change to the content will trigger an immediate model update
and form validation. This PR implements a new directive `ng-model-options`
that allow you to override this default behavior in several ways.

You should specify an object with the different parameters.

For example, it allows to trigger an update only when a particular event or
list of events is received by the input using the `updateOn` key. Should you
need multiple events, just assign an array to it.

I.e. `ng-model-options="{ updateOn: 'blur' }"` will update and validate only
after the control loses focus.

If you want to keep the default behavior and just add new events that may
trigger the model update and validation, add "default" as one of the specified
events.

I.e. `ng-model-options="{ updateOn: ['default','submit'] }"`

Also, with the `debounce` option, `ng-model-options` will allow deferring the
actual model update until a timer expires. The timer will be reset each time
an event is triggered.

I.e. `ng-model-options="{ debounce: 500 }" for 500ms after the latest event.

Custom timeouts for each event can be set for each event if you use an object
in `debounce`. This can be useful to force immediate updates on some specific
circumstances (like blur events).

I.e. `ng-model-options="{ updateOn: ['default', 'blur'], debounce: { default: 500, blur: 0} }"`

You can use the directive in any tag so its contents became the default settings
for any child control, although they can be overridden.

Closes #1285, #2129
747055d
@lrlopez lrlopez added a commit to lrlopez/angular.js that referenced this pull request Apr 3, 2014
@lrlopez @lrlopez lrlopez + lrlopez feat(ngModelOptions): Model update behavior can now be customized
By default, any change to the content will trigger an immediate model update
and form validation. This PR implements a new directive `ng-model-options`
that allow you to override this default behavior in several ways.

You should specify an object with the different parameters.

For example, it allows to trigger an update only when a particular event or
list of events is received by the input using the `updateOn` key. Should you
need multiple events, just assign an array to it.

I.e. `ng-model-options="{ updateOn: 'blur' }"` will update and validate only
after the control loses focus.

If you want to keep the default behavior and just add new events that may
trigger the model update and validation, add "default" as one of the specified
events.

I.e. `ng-model-options="{ updateOn: ['default','submit'] }"`

Also, with the `debounce` option, `ng-model-options` will allow deferring the
actual model update until a timer expires. The timer will be reset each time
an event is triggered.

I.e. `ng-model-options="{ debounce: 500 }" for 500ms after the latest event.

Custom timeouts for each event can be set for each event if you use an object
in `debounce`. This can be useful to force immediate updates on some specific
circumstances (like blur events).

I.e. `ng-model-options="{ updateOn: ['default', 'blur'], debounce: { default: 500, blur: 0} }"`

You can use the directive in any tag so its contents became the default settings
for any child control, although they can be overridden.

Closes #1285, #2129
0efe039
@petebacondarwin petebacondarwin added a commit that referenced this pull request Apr 4, 2014
@lrlopez @petebacondarwin lrlopez + petebacondarwin feat(ngModelOptions): custom triggers and debounce of ngModel updates
By default, any change to an input will trigger an immediate model update,
form validation and run a $digest. This is not always desirable, especially
when you have a large number of bindings to update.

This PR implements a new directive `ngModelOptions`, which allow you to
override this default behavior in several ways. It is implemented as an
attribute, to which you pass an Angular expression, which evaluates to an
**options** object.

All inputs, using ngModel, will search for this directive in their ancestors
and use it if found.  This makes it easy to provide options for a whole
form or even the whole page, as well as specifying exceptions for
individual inputs.

* You can specify what events trigger an update to the model by providing
  an `updateOn` property on the **options** object. This property takes a
  string containing a space separated list of events.

  For example, `ng-model-options="{ updateOn: 'blur' }"` will update the
  model only after the input loses focus.

  There is a special pseudo-event, called "default", which maps to the
  default event used by the input box normally. This is useful if you
  want to keep the default behavior and just add new events.

* You can specify a debounce delay, how long to wait after the last triggering
  event before updating the model, by providing a `debounce` property on
  the **options** object.

  This property can be a simple number, the
  debounce delay for all events. For example,
  `ng-model-options="{ debounce: 500 }" will ensure the model is updated
  only when there has been a period 500ms since the last triggering event.

  The property can also be an object, where the keys map to events and
  the values are a corresponding debounce delay for that event.
  This can be useful to force immediate updates on some specific
  circumstances (like blur events). For example,
  `ng-model-options="{ updateOn: 'default blur', debounce: { default: 500, blur: 0} }"`

This commit also brings to an end one of the longest running Pull Requests
in the history of AngularJS (#2129)!  A testament to the patience of @lrlopez.

Closes #1285, #2129, #6945
dbe381f
@petebacondarwin
Member

Landed!

dbe381f

Thanks to everyone who has worked toward getting this into Angular.

@tynman
tynman commented Apr 4, 2014

Thanks, everyone, for all your hard work on this! I'm excited to tell my
friends it's a default feature now!

On Fri, Apr 4, 2014 at 7:55 AM, Pete Bacon Darwin
notifications@github.comwrote:

Closed #2129 #2129.

Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/pull/2129
.

@mtrainham

OMG is this really happening!?

@caitp
Contributor
caitp commented Apr 4, 2014

It totally is!

@gabrielmaldi
Contributor

Thank you @lrlopez and everyone who collaborated to make this happen 👍

@mtrainham

This is the dawn of a new era.

@shidhincr

Great work everyone .. Enjoyed reading the complete comments of an year .. Awesome AngularJS community

@tedyyu
tedyyu commented Apr 9, 2014

Couldn't wait to use this feature. well done!

@jaulz
jaulz commented Apr 14, 2014

Great! This is what I was waiting for... any chance to get this by bower right now?

@just-boris
Contributor

@jaulz, just specify version as snapshot build:

bower install angular#1.3.0-build.2601+sha.ede9984
@jaulz
jaulz commented Apr 14, 2014

@just-boris thanks a lot!

@paulyoder

And just to clarify, will this feature be available in version 1.3.0 when it comes out?—
Sent from my iPhone

On Mon, Apr 14, 2014 at 12:20 AM, jaulz notifications@github.com wrote:

@just-boris thanks a lot!

Reply to this email directly or view it on GitHub:
#2129 (comment)

@lrlopez
Contributor
lrlopez commented Apr 14, 2014

Yep. It will be in 1.3.0

@kyrios
kyrios commented Apr 15, 2014

I just created an issue with that feature. Unsure if it's a bug. #7117

@revolunet revolunet added a commit to revolunet/angular.js that referenced this pull request Jun 12, 2014
@clkao @revolunet clkao + revolunet fix(input): do not hold input for composition on android
Workaround for chrome for android until #2129 is ready.

Closes #5308, #5323
627f197
@revolunet revolunet added a commit to revolunet/angular.js that referenced this pull request Jun 16, 2014
@lrlopez @revolunet lrlopez + revolunet feat(ngModelOptions): custom triggers and debounce of ngModel updates
By default, any change to an input will trigger an immediate model update,
form validation and run a $digest. This is not always desirable, especially
when you have a large number of bindings to update.

This PR implements a new directive `ngModelOptions`, which allow you to
override this default behavior in several ways. It is implemented as an
attribute, to which you pass an Angular expression, which evaluates to an
**options** object.

All inputs, using ngModel, will search for this directive in their ancestors
and use it if found.  This makes it easy to provide options for a whole
form or even the whole page, as well as specifying exceptions for
individual inputs.

* You can specify what events trigger an update to the model by providing
  an `updateOn` property on the **options** object. This property takes a
  string containing a space separated list of events.

  For example, `ng-model-options="{ updateOn: 'blur' }"` will update the
  model only after the input loses focus.

  There is a special pseudo-event, called "default", which maps to the
  default event used by the input box normally. This is useful if you
  want to keep the default behavior and just add new events.

* You can specify a debounce delay, how long to wait after the last triggering
  event before updating the model, by providing a `debounce` property on
  the **options** object.

  This property can be a simple number, the
  debounce delay for all events. For example,
  `ng-model-options="{ debounce: 500 }" will ensure the model is updated
  only when there has been a period 500ms since the last triggering event.

  The property can also be an object, where the keys map to events and
  the values are a corresponding debounce delay for that event.
  This can be useful to force immediate updates on some specific
  circumstances (like blur events). For example,
  `ng-model-options="{ updateOn: 'default blur', debounce: { default: 500, blur: 0} }"`

This commit also brings to an end one of the longest running Pull Requests
in the history of AngularJS (#2129)!  A testament to the patience of @lrlopez.

Closes #1285, #2129, #6945
a756220
@RyanElfman

This is awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment