istvan-ujjmeszaros / bootstrap-autohidingnavbar

An extension for Bootstrap's fixed navbar which hides the navbar while the page is scrolling downwards and shows it the other way.
Other
321 stars 106 forks source link

cancel any pending animations when a new one starts #9

Closed mwcz closed 9 years ago

mwcz commented 9 years ago

I think it's good practice to use jQuery's stop to cancel any current/pending animations before starting a new animation. It keeps them from stacking.

I honestly didn't do any testing before adding this, so I don't know how much benefit it provides, but here it is anyway! :)

istvan-ujjmeszaros commented 9 years ago

I think it is not needed when queue is set to false, but I am not hundred percent sure about it. Need to test it.

mwcz commented 9 years ago

Ah, cool, learned something new. I don't know a lot about how jQuery handles animations. After looking into it and doing some testing, I don't think this PR is helpful at all.

From the docs it sounds like...

It sounds like queue: false won't stop any prior animations from running, but it should cause the new anim to run right away. I would think that stop(true) might be helpful, but after testing it it looks like autohidingNavbar already behaves perfectly.

I tested it by setting animationDuration to 1000, then scrolling up and down rapidly to see if the animations stack up and cause jittering. It looked great though! No need for change.

istvan-ujjmeszaros commented 9 years ago

Thanks for testing it!