-
Notifications
You must be signed in to change notification settings - Fork 20.5k
Effects: easing._default #2218
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
Effects: easing._default #2218
Conversation
43c50f9 to
d2a077f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd prefer a default easing to stay at the Tween level, rather than here in Animate. And since @gnarf is back in this headspace for a bit, we can get a second opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote a resume of the situation in the other message.
But I like the way we put the default values and after the user rewrite them with the .extend.
I think (IMHO) we should do the same thing for the duration value.
And also for the complete callback with jQuery.noop maybe (it's only a suggestion).
|
I like the direction here, but please create an issue so we can track it. Also, the CLA failure will need to be fixed by amending this commit—and all future ones—to use your full name (which we require and are now verifying). |
|
hi @gibson042 :) I signed the CLA again yesterday with my nickname and my email. I don't understand what I have to do exactly. Do I have to write my real full name? |
Yes. |
|
Thanks @mzgol :) I didn't respect exactly the principle of the atomic commit, so I redid my commit. Actually, If we don't precise the The duration don't respect the same idea. If I don't specify a |
d2a077f to
8732b95
Compare
8732b95 to
df07ff7
Compare
|
Hi, I read in your meeting nobody share my code design 😢 so I moved the But should I move the code of the |
|
I add two new tests for that but I have to change the variable's name of another test because it was using (and delete) the new After you say it's better to see that in the Tween file I changed, but I didn't see the test jQuery(data).animate(
{
a: [ 100, "_test1" ],
b: [ 100, "_test2" ],
c: 100
},
400,
"default_easing"
);The property |
|
I'm having a hard time following this, so let me back up a bit and present a call stack as it appears in master:
Having gone through all of that, I think I understand what you're after: a guarantee of animation-level So the bottom line here is that I would be fine with use of |
test/unit/effects.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's name this to better match its purpose, something like jQuery.easing.animationScope (and the other one to e.g. jQuery.easing.propertyScope).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah right, it's done.
|
@gibson042 nice summary!
I didn't understand exactly where do you think I have to put the |
Ahhh okay, I didn't know this (I push another commit). In Qunit I read (on my test): Where is this test? And what I have to do to passe it? |
|
That looks like one of our beforeEach/afterEach assertions, and indicative of an async issue (e.g., |
|
Okay, I add |
More info at the issue: #2219.