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

jQuery/Velocity SlideUp/Down bug #260

Closed
Omniferum opened this issue Aug 22, 2014 · 27 comments
Closed

jQuery/Velocity SlideUp/Down bug #260

Omniferum opened this issue Aug 22, 2014 · 27 comments
Labels

Comments

@Omniferum
Copy link

SlideUp and SlideDown both apply the following styles as expected

display: none;
overflow: visible;
height: auto;
margin-top: 0px;
margin-bottom: 0px;
padding-top: 0px;
padding-bottom: 0px;

However Velocity does not remove these style tags from the elements upon completion. Tried it on multiple elements.

I am trying to set up a JSFiddle but I keep getting a console error of:
Uncaught Error: Velocity: Either jQuery or Velocity's jQuery shim must first be loaded.

Will happily throw up an example if somebody wouldn't mind telling me how exactly I do that with JSFiddle (never used it before).

@Omniferum
Copy link
Author

I kind of feel bad for not being able to make a JSFiddle so I did the only other way I could demonstrate it. I made a simple html file that accurately recreates the situation.

http://padfly.com/umtjnergb

@Rycochet
Copy link
Collaborator

Just a quick comment - are you sure you're running against the latest version of velocity? Until a week ago(?) there was an issue with values being left behind, but a recent update removed them (and made it smaller and faster) - not to worry if you are running the latest, just something to check :-)

@Omniferum
Copy link
Author

Just downloaded the zip fresh from github and tested again, same thing.

Also tested with jQuery 2.1.1 and 2.0.0 (just in case?) and bug is present.

Man people sure are fast and nice on github.

@julianshapiro
Copy link
Owner

This isn't so much a bug as it is a general side-effect, but I came up with a way to solve this (I believe). I'll plow through this today and report back. Thanks so much!

@Omniferum
Copy link
Author

I wasn't sure if I should have included this point (in retrospect: probably) but I only noticed this because with velocity when I slide up a div that changed window dimensions, that 'sliding' div had a scroll bar appear for the animation duration. There were 'no' style tags or rules set about overflow on anything in the document.

@julianshapiro
Copy link
Owner

Please test, confirm it works, and get back to me. Thank you so much!

@Omniferum
Copy link
Author

Just using the padfly and dropping in the new velocity there is a new problem.

The element that is being slide over is more overlapped than 'hidden'. The jQuery animation hides while it slides, velocity did before but doesn't now.

@julianshapiro
Copy link
Owner

not sure what you're saying

@Omniferum
Copy link
Author

The elements that slide over each other overlap visually, whereas before the element being 'slid' over should normally be hidden as the other element is revealed.

The 'style persistence' however is gone.

A closer check reveals that jQuery uses a overflow:hidden style tag during transition, while velocity does not.

@Omniferum
Copy link
Author

A few manual tests for me confirms that the lac of the overflow:hidden is what causes the overlap effect.

@Omniferum
Copy link
Author

If you don't mind me using this ticket for asking, but what are the differences between jQuery and velocity in terms of their fade/slide logic? I am aware those two things you, I think the term is, baked in. Does this mean you just replicated the functionality or rely on jQuery to complete it?

I'm just curious if using the jQuery slide/fade is actually, performance/compatibility-wise, any different to velocity (I just default assume that velocity treats the same functions far more gracefully/less intensively than jQuery).

@Rycochet
Copy link
Collaborator

The biggest difference with jQuery slide is that it's not actually a simple animation - it actually wraps the elements in a div and then animates that so things aren't that simple ;-)

@Omniferum
Copy link
Author

So basically Mr. Shapiro's velocity approach to the fade/slide would be no different from the jQuery, hence no real point? (Aside from not having to rely on jQuery (dependency being/is removed?))

@julianshapiro
Copy link
Owner

I will fix this in the next version so that overflow:hidden gets re-applied like it was before. Thanks!

@julianshapiro
Copy link
Owner

Done. Please test, confirm it works, and get back to me. Thank you so much!

@Omniferum
Copy link
Author

Confirmed fixed.

Don't suppose I could get your 2 cents on my question re: jQuery/Velocity fade/slide methods? Simplified I am just trying to identify if using velocity for these functions instead of jQuery is worthwhile (I still NEED jQuery for my site regardless, still velocity helped IMMENSELY.)

@julianshapiro
Copy link
Owner

Velocity's slide/fade functions are completely custom and do not rely on jquery in any way. The performance with velocity will be much better.

@Omniferum
Copy link
Author

Last question (feel free to punch me here): Is the compatibility of the function between both comparable, or do you believe jQuery would win out? I don't know much about the function itself so apologies about the thousand questions.

Just trying to identify if a drop-replacement would be worthwhile (simple regex thing to do anyway)

@julianshapiro
Copy link
Owner

Should work everywhere, back to ie8 and android 2.3 according to my tests.

@Omniferum
Copy link
Author

Drop-replacement it is then! Thanks dude.

@Omniferum
Copy link
Author

Simply reporting on the differences I experience between velocity/jquery in regards to the fade/slide, whether or not it is actually a bug or just how velocity has to do things I don't know. (e.g. I can sort of understand why the 'fade back to previous value' was not implemented.)

In jQuery if you slide up/down an element that is 'wider' than its container no scroll bar is shown.
In Velocity a scroll bar is shown.

@julianshapiro
Copy link
Owner

I'll have it set horizontal scrollbars to hide in the next release (prob coming tomorrow) :)

@julianshapiro
Copy link
Owner

done

@julianshapiro
Copy link
Owner

lemme know that all works as expected

@Omniferum
Copy link
Author

100% A-okay

You're a champion.

@Omniferum
Copy link
Author

Oh, for reference I suppose you might consider my testing for the slide/fade fairly extensive as I have now completely replaced all jQuery fade/slide with velocity. Seeing as my site started as paying some consultancy to create, which they abandoned, then another person came in to try and clean it up, then abandoned, and then I had to because I couldn't go through that all again (and I really had no actual knowledge of jQuery/PHP/CSS/HTML beyond understanding basic code logic flow) I am sure a large number of 'broken' situations have been encountered.

I have even replaced the one animation plugin I have to be powered by velocity. That plugin runs a lot smoother with velocity.js now, probably more so because this one used to animate 'n' divs (~40 is the default), and like the engine comparison on the velocity homepage I can actually crank that number up quite high (120, looks too busy/cluttered at that point though) before an old, low-performance computer will complain.

So, yeah, velocity.js gets mega-points from me for being both simple and powerful.

@julianshapiro
Copy link
Owner

thanks so much :)

On Thu, Aug 28, 2014 at 10:32 PM, Omniferum [email protected]
wrote:

Oh, for reference I suppose you might consider my testing for the
slide/fade fairly extensive as I have now completely replaced all jQuery
fade/slide with velocity. Seeing as my site started as paying some
consultancy to create, which they abandoned, then another person came in to
try and clean it up, then abandoned, and then I had to because I couldn't
go through that all again (and I really had no actual knowledge of
jQuery/PHP/CSS/HTML beyond understanding basic code logic flow) I am sure a
large number of 'broken' situations have been encountered.

I have even replaced the one animation plugin I have to be powered by
velocity. That plugin runs a lot smoother with velocity.js now, probably
more so because this one used to animate 'n' divs (~40 is the default), and
like the engine comparison on the velocity homepage I can actually crank
that number up quite high (120, looks too busy/cluttered at that point
though) before an old, low-performance computer will complain.

So, yeah, velocity.js gets mega-points from me for being both simple and
powerful.


Reply to this email directly or view it on GitHub
#260 (comment)
.

Rycochet pushed a commit that referenced this issue Aug 3, 2020
Dropped support for the easeBack and easeElastic easings. They are
rarely used, look stupid, and take up a bunch of lines.

Sped up bezier curve easing performance. Thanks, @gre.

The display option now gets browser-prefixed if necessary. This
inherently adds cross-browser support for the ‘flex’ value. Closes #257.

Fixed bug where slideDown/up left excess inline styles on elements.
Closes #260. cc @kpeatt, @scalvert.

Allow passing an empty string to `display` to remove the inline style
from the element. Closes #184.

Shimmed version of jQuery’s $.dequeue() now accepts a raw DOM element
*set* instead of just a single element.

You can now stop custom queues individually. Closes #262.
Rycochet pushed a commit that referenced this issue Aug 3, 2020
Apply `overflow: hidden` when using the slide commands. Closes #260.

Sequence (UI pack effect) options object no longer gets externally
modified by Velocity. Closes #265.

Fixed bug where tweens would jump when dequeuing custom queues. Closes
#262. (The reverse command now only applies to the default effects
queue; reverse cannot be used with custom queues or parallel queueing
(queue: false).)

Fixed unit conversion bug on the `x` property of SVG elements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants