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

width: auto incorrectly reverted to "auto" in extreme edge case #223

Closed
paul-geonomics opened this issue Aug 7, 2014 · 5 comments
Closed
Labels

Comments

@paul-geonomics
Copy link

Hi,

the following bug can be reproduced in this fiddle: http://jsfiddle.net/pingless/48dub1LL/

When an element has a negative margin, and is flown in from beyond the side of the screen, velocity incorrectly sets the size to 'auto' after the animation.
This is visible when toggling the sidebar on, then off; when it moves out, the sidebar content gets "squished" when it should stay fixed-size (100%).

The problem is caused by lines 2601-2613: https://github.com/julianshapiro/velocity/blob/master/jquery.velocity.js#L2601
The element's size is set to auto, then the width is compared, to determine if the width was auto beforehand. In the case where there is a negative margin, it appears this calculation returns a false positive.

We're working around this problem by setting width: 100% !important on the element being animated (which overrides any values velocity might set), but this is not an ideal solution. I hope this can be fixed...

Hi Julian,
I've managed to reproduce it with positive margin: http://jsfiddle.net/pingless/48dub1LL/1/
It also reproduces with position: fixed: http://jsfiddle.net/pingless/48dub1LL/2/

I tried it with box-sizing: content-box but it seems that when calc() is used, you don't get the same width as auto (probably due to fractional pixel units, i.e. e.g. 350.2px looks the same as 350px but is not considered the same width).

Thanks,

Paul L

@julianshapiro julianshapiro changed the title Width: auto incorrectly set with negative margins width: auto incorrectly reverted to "auto" in extreme edge case Aug 7, 2014
@julianshapiro
Copy link
Owner

Thank you for the detailed bug report, Paul. Looking into this now...

Okay, so updating my list from above. It's caused by the element having:

  • a negative margin/left/top resulting in the element being partially hidden from view
  • position: absolute/fixed
  • animating to a px unit
  • a width: 100% that resolves to the same value as width: auto during the beginning of the animation.

@julianshapiro
Copy link
Owner

I'm doing a drastic overhaul of Velocity's unit conversion logic. Performance will be degraded a tiny bit, but it'll result in being able to avoid placing all these arbitrary internal CSS values on element animated with Velocity. Expect an update early next week.

@julianshapiro
Copy link
Owner

Actually, Velocity just got significantly faster because I fixed a latent bug in the process. Yay!

@paul-geonomics
Copy link
Author

Brilliant, thanks Julian!
Looking forward to the release.

@julianshapiro
Copy link
Owner

Please test and let me know if all is good :)

Rycochet pushed a commit that referenced this issue Aug 3, 2020
Drastic speed boost at the start of and between chained animations.
Especially on mobile.

Overhaul of unit conversion logic. Velocity no longer applies internal
CSS properties inline to elements that are being animated. This should
fix all post-animating bugs that have been encountered. Closes #178.
Closes #223.
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

2 participants