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

Help Wanted: Do a better job than me on the slideDown/slideUp sequences #96

Closed
julianshapiro opened this issue Jun 4, 2014 · 17 comments

Comments

@julianshapiro
Copy link
Owner

Velocity's packaged slideUp/slideDown sequences are ugly. They're the only part of Velocity's codebase that I don't like: https://github.com/julianshapiro/velocity/blob/master/jquery.velocity.js#L2681

To tackle recoding them, you'll first need to read the docs on Sequences (http://VelocityJS.org/#sequences) and display toggling (http://VelocityJS.org/#display).

Next, here's a snippet you can use to quickly test your fork. Make sure this runs the same in your fork as it does in the current version of Velocity:

$element
    .css({ overflow: "scroll", height: 400 })
    .delay(1500)
    .hide()
    .velocity("slideDown", 1000)
    .velocity("slideUp", 1000)
    .velocity("slideDown", "easeInOutQuad", 1000, function() { console.log("All done!") });

If you have any questions about why the current slide sequences are coded they way they are, ask here. It would be my pleasure to expand on my existing decisions (which I don't think are necessarily very sound).

Two things I'll preemptively point out:

  • Overflow toggling is done so that scrollbars don't show during the animation (to parallel jQuery's implementation, but it's also a good idea from an aesthetic standpoint).
  • Reverting to a height of "auto" (if appropriate) is an ugly necessity. Refer to the code for more info.

Finally, I'm looking for an enhancement: Don't re-trigger the slideDown animation if the element is already visible. Likewise, don't re-trigger slideUp if the element is already hidden.

@legomushroom
Copy link

@julianshapiro I will take this one

@julianshapiro
Copy link
Owner Author

I love you.

Sent from my phone.

On Jun 12, 2014, at 1:08 PM, Oleg Solomka [email protected] wrote:

@julianshapiro I will take this one


Reply to this email directly or view it on GitHub.

@legomushroom
Copy link

@julianshapiro :D 😊

@legomushroom
Copy link

Tried the snippet. Doesn't work for me either with my fork or latest Velocityjs release. Element[s] doesn't show up at all. Am I doing right?

@julianshapiro
Copy link
Owner Author

Could you please show me a JS fiddle of it not working?

@legomushroom
Copy link

Got it, if element uses border-box box model, it fails. That's why it didn't work for me.

@julianshapiro
Copy link
Owner Author

Awesome :) One more thing to fix.

@julianshapiro julianshapiro changed the title HELP WANTED: Do a better job than me on the slideDown/slideUp sequences Help Wanted: Do a better job than me on the slideDown/slideUp sequences Jun 15, 2014
@julianshapiro
Copy link
Owner Author

Heyo! How's this coming along? Would you like any help?

@legomushroom
Copy link

@julianshapiro still tweaking. The last time worked just fine, but then I tried it with a delay :/ Unit tests needed :)

@legomushroom
Copy link

made a PR #121

@julianshapiro
Copy link
Owner Author

We'll continue this discussion in the PR.

@benoitgrelard
Copy link

Am I right in thinking that Velocity's support for slideUp/slideDown is only "vertical" as in "height" or can I also do it "horizontally" as in "width"?

I haven't found a way to do that, could someone let me know if I missed anything?

Cheers!

@julianshapiro
Copy link
Owner Author

Heyo! there's no way to do it horizontally :) Although feel free to code your own solution and share it with others if you're interested! You can tweak the current vertical solution.

@ghost
Copy link

ghost commented Sep 23, 2015

How can we prevent animation "stacking" when using "slideDown" and "slideUp"? In JQuery I used "stop"?

Example (on click function):

var elem = document.getElementById('#test');
Velocity(elem, elem.clientHeight > 0 ? 'slideUp' : 'slideDown');

@MattyBalaam
Copy link

The simple answer is by using Velocity(elem, 'stop').

The long answer is that I just tried it and it causes some issues with caching heights if used before an animation has been completed.

@ghost
Copy link

ghost commented Sep 24, 2015

Yes, I have tried that. Animation is messed up then.

@eddneal
Copy link

eddneal commented Mar 23, 2016

@MattyBalaam Did you find a solution to prevent the animation "stacking" which didn't cause the height caching issue? Thanks in advance.

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 a pull request may close this issue.

5 participants