Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

Typeahead: Fixed waitTime functionality #607

Closed
wants to merge 1 commit into from
Closed

Typeahead: Fixed waitTime functionality #607

wants to merge 1 commit into from

Conversation

trash
Copy link
Contributor

@trash trash commented Jul 1, 2013

A fix for bug #573.

This pull request fixes the bug where $timeout calls were not cancelled even if they occurred in the time period defined by waitTime.

The reason for the bug was that timeoutId was declared inside the scope of the function being pushed to $parsers, and therefore could not be referenced outside of the anonymous function. By declaring the timeoutId, now renamed timeoutPromise (because the return of $timeout is not an id but actually a promise) above this function, the offending old $timeout calls can be killed.

@pkozlowski-opensource
Copy link
Member

@khalilravanna Cool! We've got another PR for the same, see: #594
What do you think?

Also, would be awesome to have a test exposing the issue. It seems like the current test we've got is completely useless as it passes regardless of the fix presence....

@trash
Copy link
Contributor Author

trash commented Jul 1, 2013

@pkozlowski-opensource I'm partial to my own solution just because it changes less code and it works for me locally. I'm going to work on creating a test now that can make sure those timeouts are being killed.

@pkozlowski-opensource
Copy link
Member

@khalilravanna yeh, I like the fact that it touches less code and is shorter. A test would totally rock!

@trash
Copy link
Contributor Author

trash commented Jul 1, 2013

Added a couple of tests to make sure that timeouts are squashed when within the waitTime, and are free to execute if waitTime has passed.

@pkozlowski-opensource
Copy link
Member

Landed as 90a8aa7. Thank you!

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

Successfully merging this pull request may close these issues.

2 participants