benbarnett / jquery-animate-enhanced

Extend $.animate() to detect CSS transitions for Webkit, Mozilla, IE>=10 and Opera and convert animations automatically.
http://playground.benbarnett.net/jquery-animate-enhanced/
MIT License
1.39k stars 196 forks source link

fadeOut() for an element with 'display:none' doesn't fire complete callback #128

Open Koloto opened 11 years ago

Koloto commented 11 years ago

Try the example here: http://jsfiddle.net/pD2Ag/3/ The behavior is the same for detached element. You can remove the link to your plugin from external resources - callbacks will be fired for all elements.

tD3rk commented 11 years ago

I can also confirm this issue.

Koloto commented 10 years ago

Hello, Ben. Are there any chances to fix this bug? It's enough to check is(":hidden"). And if this is true, a complete callback should be called immediately.

zunou commented 10 years ago

+1 for this bug. same for fadeIn()

Blackskyliner commented 10 years ago

+1 from me too, see referenced issue.

michaelBenin commented 10 years ago

Yep, seeing this bug in this code:

this.$el
  .empty()
  .hide()
  .append(template(config))
  .fadeIn('fast');
Blackskyliner commented 10 years ago

Problem for fadeIn is that it's not trivial to fix with a simple conditional. I tried it for like 1 or 2 days to fix it. But you will always find an edge-case where it won't work. Also the conditional will either blow your mind and have a very high complexity or it will be somehow dumb and not do CSS animations at all... At least the ways I tried it.

fadeOut() is easy as there is some real definition of "not visible". But the visible thing is that like 0.000001 opacity is already something visible. Or in the definition of jQuery: "If it takes space in the layout, its visible"

So simply checking if the object is visible will not solve the fadeIn issue. Also the precision of floats is the next big thing... I tried checking for same opacity and so on and so forth but I then had debug valus like "0.6 === 0.059999999957631" which then broke the whole thing all together again...

Like I said it only applies if you try to fix it in the naive way of adding one or two if's before doing the css animation thing... For those edge-cases where you get the error with fadeIn you may want to add the 'avoidCSSTransitions' as a temporary fix.