CodeSeven / toastr

Simple javascript toast notifications
http://www.toastrjs.com
MIT License
11.97k stars 2.04k forks source link

Timing issue with clear() function #118

Closed knunery closed 10 years ago

knunery commented 11 years ago

There is a timing issue with toastr.clear().

Got to http://codeseven.github.io/toastr/demo.html. Open a console in Chrome.

run the following command twice

toastr.clear(); toastr.success("blah");

You should see "blah" displayed twice but you don't in my testing. The second run is clearing the second blah which looks like a timing bug with the clear function.

johnpapa commented 11 years ago

Yes, this is because the clear method tells the toasts to fade out, which can be 300ms (by default). this means if you call the clear and immediately call a new toast, the new toast will flash (at best).

The workaround is time wait with a timeout.

However, I may fix this when I move to a promise based scenario for toastr in the future.

makenova commented 10 years ago

Hello @johnpapa , I took your advice and used a time wait but subsequent toasts still flash and disappear when I waited for 300ms. I had to wait at least 1005ms and that didn't always work.

var test = function (a){
  toastr.info("thinking...");
  toastr.clear();
  setTimeout(function(){
    toastr.error("Boo Hoo","Error");
  }, a );
}
test(1005) // worked sometimes.
test(1100) // always worked.
knunery commented 10 years ago

Would using the jQuery animation queue be better or worse than using promises? I would like to help fix this issue.

http://api.jquery.com/queue/

johnpapa commented 10 years ago

The issues is that you need to wait for the toast to fade in, be visible, then fade away. So there are 3 sets of time periods to wait for. And you could extend that period if you hover or use extended timeouts. So promises would be ideal to handle this.

For now the best approach I have used is to make sure you wait for all of those periods.

I am considering making a CSS3 version which would also remove most of jquery's needs and possibly implement q.js for promises. That versions would only work in modern browsers. But that's not something I am far enough along in yet to commit to.

makenova commented 10 years ago

@knunery, I'm curious how would you use the jQuery animation queue. Do you have an example?

knunery commented 10 years ago

@makenova The animations in jQuery are put into an "fx" queue. So if you call $("div").hide("slow").slideUp().fadeOut(); then the hide, slideUp, and fadeOut functions are put onto the "fx" queue and only the next method is called once the previous method has completed. If you were to use the animation queue then, wherever you see setTimeout in the code, would need to be refactored. The queue() function is basically jquery implementation of promises.

As johnpapa mentions a promises library is the right solution. With the animation queue you have to be sure to call dequeue() or next() and I'm not sure if you have to do that with a promises library.

Here is a very naive example of using the jquery queue http://jsfiddle.net/uh6n5/. I think the jquery queue page is likely the best resource for understanding this.

johnpapa commented 10 years ago

Closed with #118 and #149

jamespsterling commented 7 years ago

I have a scenario when I'm switching UI Routes and would like to be able to dismiss toasts immediately. What about .clear({force: true}) to do something like that, rather than waiting for the timeout?