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

feat($timeout): Add debouncing support #4500

Closed
wants to merge 1 commit into from

Conversation

lrlopez
Copy link
Contributor

@lrlopez lrlopez commented Oct 17, 2013

This feature allows resetting the timer on an ongoing $timeout promise. The implementation is really simple (see the diff) and fully backwards compatible. Documentation and tests are included. It adds just 98 bytes to the current minified build.

The PR adds a fourth optional argument to $timeout which is the old promise to be reset.

With this feature you can debounce events. In the following example, doRealSave() will only be called 2 seconds after the last model change:

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

And in the form:

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

This PR has been extracted from a bigger one: #2129

@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

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!

@lrlopez
Copy link
Contributor Author

lrlopez commented Oct 17, 2013

I've already signed the CLA. The commit message seems to be ok too, just tell me if I need to change anything.

* }
* $scope.save = function() {
* // debounce call for 2 seconds
* debounce = $timeout(doRealSave, 2000, debounce);
Copy link
Member

Choose a reason for hiding this comment

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

it seems the signature is incorrect as the function timeout definition has the debounce argument in the 4th position.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Fixed. Thanks!

@lrlopez
Copy link
Contributor Author

lrlopez commented Oct 21, 2013

By the way, I forgot to tell that the PR is rebased to a recent master.

@lrlopez
Copy link
Contributor Author

lrlopez commented Oct 27, 2013

@petebacondarwin,
Pete, I'm pinging you because you were involved in the related commit. Just a reminder: this PR still hasn't been assigned to anyone nor given a milestone.

@petebacondarwin
Copy link
Contributor

@lrlopez - I'm afraid only regressions and documentation fixes are going to go into 1.2.0 now. We should push to get this in; along with, what I think is a great PR, #2129.

@lrlopez
Copy link
Contributor Author

lrlopez commented Oct 27, 2013

Ok. Thanks again!

@lrlopez
Copy link
Contributor Author

lrlopez commented Nov 9, 2013

Rebased to latest master (which is the official 1.2.0) and solved a rebase conflict. By the way, the Travis CI build failed due to #4822

@petebacondarwin
Copy link
Contributor

@lrlopez - thanks for hanging in there on these PRs. No that 1.2.0 is out we can start going through these.

@lrlopez
Copy link
Contributor Author

lrlopez commented Nov 11, 2013

@petebacondarwin,
Yeah, we can work on them once you are ready...

This feature allows resetting the timer on an ongoing `$timeout` promise.

A fourth optional argument is added to `$timeout` which is the
old promise to be reset.

I.e.: `promise = $timeout(fn, 2000, true, promise);`

This will call `fn()` 2 seconds after the last call to the `$timeout()`
function.
@lrlopez
Copy link
Contributor Author

lrlopez commented Nov 27, 2013

It may be a good time to assign this PR a more concrete milestone...

@lrlopez
Copy link
Contributor Author

lrlopez commented Apr 6, 2014

Closed as #2945 has landed

@lrlopez lrlopez closed this Apr 6, 2014
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.

4 participants