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

Fix #7641: .animate() should accept "auto" and "" (empty string) values #408

Closed
wants to merge 5 commits into from

Conversation

@louisremi
Copy link
Contributor

louisremi commented Jun 7, 2011

This PR fixes a "patchwelcome" bug in 10 short lines of code.
Unit-tests included.

http://bugs.jquery.com/ticket/7641 (was "patchwelcome", I just reopened it)
http://bugs.jquery.com/ticket/9482

@timmywil

This comment has been minimized.

Copy link
Member

timmywil commented Jul 12, 2011

Correct me if I'm wrong, but won't this only work if there are uncomputed styles set that we can fall back to? For instance, the default value of width is "auto" and unless a width is set somewhere in the user css that isn't auto, we still can't do the animation. Doesn't sound like full support of animating "auto".

@louisremi

This comment has been minimized.

Copy link
Contributor Author

louisremi commented Jul 19, 2011

That shouldn't be a problem since we use the computed style of the element

@timmywil

This comment has been minimized.

Copy link
Member

timmywil commented Jul 19, 2011

Which will return 'auto' in some cases.

@timmywil

This comment has been minimized.

Copy link
Member

timmywil commented Jul 19, 2011

@dmethvin

This comment has been minimized.

Copy link
Member

dmethvin commented Aug 5, 2011

OK, so this pull is no good? Or @louisremi do you think you can solve the problem?

@louisremi

This comment has been minimized.

Copy link
Contributor Author

louisremi commented Aug 5, 2011

Sorry, the discussion continued in the bug report and on IRC:

"The problem of the "auto" value returned for computed style is also present in other part of the effects code, see how jQuery.fx.cur() deals with it. This didn't prevent us from writing the effect component ;-)"

I think Timmywil agreed with that (we can discuss it again if necessary).

@dmethvin

This comment has been minimized.

Copy link
Member

dmethvin commented Sep 2, 2011

If the premise of this PR is still okay, it needs to be updated before it can be merged. Not sure where it stands.

@dmethvin dmethvin closed this Sep 2, 2011
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants
You can’t perform that action at this time.