-
Notifications
You must be signed in to change notification settings - Fork 481
Effects: Merge 2 articles on queues #633
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
Conversation
58ee88d
to
eb8623b
Compare
}, "slow" ) | ||
.queue(function() { | ||
}, "slow") | ||
.queue( function() { |
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.
We should either call dequeue
in this function, or take a next
parameter and call it so that something "after" the queue would run and it doesn't get locked up.
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.
That was what I was trying to explain people with the text further in the article. But you're right, maybe it makes sense to directly show people a good example.
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.
fixed
Looking good |
e8d88f3
to
4648c9b
Compare
Did a commit with some fixes. What do you think, @gnarf? |
height: 20 | ||
}, "slow" ) | ||
.queue(function() { | ||
}, "slow", function() { | ||
$( "#title" ).html( "We're in the animation, baby!" ); |
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.
This is a callback, "in the animation" seems....
LGTM |
@gnarf Thanks for being that quick reviewing it. I addressed that last little thing. Gonna give people some more time to throw in feedback, and merging tomorrow. |
Currently we had 2 articles on queues overlapping each other, those need to be merged together (#77). After all I ended up rewriting/clarifying the whole article.
Would fix #77.