Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix(ngModelOptions): introduce $cancelUpdate to cancel pending updates #7014

Closed
wants to merge 3 commits into from
Closed

fix(ngModelOptions): introduce $cancelUpdate to cancel pending updates #7014

wants to merge 3 commits into from

Conversation

shahata
Copy link
Contributor

@shahata shahata commented Apr 6, 2014

$cancelUpdate cancels any debounce action and resets the view value by invoking $render. this method should be invoked before programmatic update to the model of input that might have pending updates due to updateOn or debounced actions. Fixes #6994

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#7014)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@shahata shahata added cla: yes and removed cla: no labels Apr 6, 2014
@@ -1693,19 +1693,19 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$

/**
* @ngdoc method
* @name ngModel.NgModelController#$cancelDebounce
* @name ngModel.NgModelController#$cancelUpdate
*
* @description
* Cancel a pending debounced update.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably be something like "Cancel an update and reset the input element's value to prevent an update to the $viewValue, which may be caused by a pending debounced event or because the input is waiting for a some future event."

@petebacondarwin
Copy link
Contributor

@shahata - thanks for putting this together. It will be great to get this in before the next release so we have a solid feature. Given the potential for confusion when doing programmatic changes, we could do with a new section in the guide that explains how to deal with the various situations that you highlighted.

@petebacondarwin
Copy link
Contributor

Also we could do with some more information in the ngModelOptions docs that reference this new $cancelUpdate method and how to use it.

$cancelUpdate cancels any debounce action and resets the view value by invoking $render. this method should be invoked before programmatic update to the model of input that might have pending updates due to updateOn or debounced actions.
@shahata
Copy link
Contributor Author

shahata commented Apr 6, 2014

@petebacondarwin - thanks for the feedback. I went ahead and pushed an amended commit with the items you mentioned. You are right about the needed documentation enhancements, I will do it tomorrow night.

@lrlopez
Copy link
Contributor

lrlopez commented Apr 6, 2014

Nice work!

}, debounceDelay);
} else {
that.$$realSetViewValue(value);
this.$$realSetViewValue(value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be ctrl

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? It is in the context of this.$setViewValue...

BTW, I was wondering - what do you think about using ctrl everywhere? It will be a bit less clear, but it would minify better. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone calls $setViewValue as a free function then this would fail, or am I wrong?

I think using ctrl everywhere is a good idea for general safety and consistency, although I suspect that a good minifier might swap out this anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uglify doesn't swap this and I believe minifiers try not to do that since it is risky (by this you could in theory actually be referring to the context in which the function was invoked and not the context in which it was defined, so swapping this automatically could result in breaking your code in a very hard to debug fashion :) )

@petebacondarwin petebacondarwin self-assigned this Apr 6, 2014
@shahata
Copy link
Contributor Author

shahata commented Apr 7, 2014

@petebacondarwin I've added the docs & refactoring commits to this PR, would be great if you can review

@petebacondarwin
Copy link
Contributor

Will do today
On 7 Apr 2014 22:53, "Shahar Talmi" notifications@github.com wrote:

@petebacondarwin https://github.com/petebacondarwin I've added the docs
& refactoring commits to this PR, would be great if you can review

Reply to this email directly or view it on GitHubhttps://github.com//pull/7014#issuecomment-39788673
.

that.$cancelDebounce();
if ( debounceDelay ) {
$timeout.cancel(pendingDebounce);
if (debounceDelay) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot on the parenthesis styling

@shahata
Copy link
Contributor Author

shahata commented Apr 8, 2014

👍

@shahata shahata deleted the fixNgModelOptions branch April 8, 2014 14:52
@petebacondarwin
Copy link
Contributor

Thanks for all the help with this @shahata.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Programmatic updates of the model in ngModelOptions
4 participants