Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds a timeout to the stop function as to avoid a stop and an instant request after that #43

Closed
wants to merge 1 commit into from

Conversation

ianmurrays
Copy link

I had an "issue" where my app would instantly request something after the bar had completed, making it look like it just returned to zero instantly. Since that looked kind of weird I added a timeout to the setComplete method with a customizable threshold.

By default, the threshold is 0 meaning it behaves just like normal.

@chieffancypants
Copy link
Owner

This should already be accounted for:

// Attempt to aggregate any start/complete calls within 500ms:
completeTimeout = $timeout(function() {
$animate.leave(loadingBarContainer, function() {
status = 0;
started = false;
});
$animate.leave(spinner);
}, 500);

What value did you use for your stopThreshold? Could you do me a favor and try using that same value in place of the current 500ms and see if that solves it? If so -- we can simply make that configurable to accomplish what you want without the extra timeout

@ianmurrays
Copy link
Author

I actually used 100ms and it seemed to work better. Didn't notice that there was already a stop timer.

@chieffancypants
Copy link
Owner

So in other words, the timer already included isn't working as well as yours?

Looking through the current code, I did notice that the completeTimer isn't canceled as part of the _complete() method. Maybe that is the reason.

@chrisg2
Copy link
Contributor

chrisg2 commented Jul 11, 2014

I have just created a pull request that cancels completeTimeout as part of the _complete() method - see #79.

@chieffancypants
Copy link
Owner

completed with #79

@chieffancypants chieffancypants added this to the next version milestone Jul 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants