jQuery animation/blinking code?

Question

I have one container div which contains some children div members. From time to time, I need to run a custom blinking effects on some of the child div members. The effect steps are as follows:

1. Change font color to Yellow
2. Pause for 0.5 secs
3. Change background color to yellow and text color to black
4. Pause for 0.25 secs
5. Change font color to Yellow & background to black
6. Pause for 0.25 secs
7. Change background color to yellow and text color to black
8. Pause for 0.5 secs
9. Repeat step 1 to 8

Below is the JavaScript code that I came up using jQuery to achieve the above effect. The code is working perfectly fine, but somehow I feel that this is not an elegant solution and there may be possible issue with how I pass $elements var from one function to another.

Is there anything wrong with this approach? How can this code be improved?

function startAnimation($container) {
 var $elements = $container.find(".animation");
 if ($elements.length) { 
 $elements.switchClass("mc-ylw-blk", "mc-blk-ylw", 0).show(0).delay(500).promise().done(function () {
 $elements.switchClass("mc-blk-ylw", "mc-ylw-blk", 0).delay(250).promise().done(function () {
 $elements.switchClass("mc-ylw-blk", "mc-blk-ylw", 0).delay(250).promise().done(function () {
 $elements.switchClass("mc-blk-ylw", "mc-ylw-blk", 0).delay(500).promise().done(function () { 
 startAnimation($container); //Repeat 
 }); 
 });
 });
 });
 }
}

• rolfl: Hi, and welcome to Code Review. Your question would benefit from having a JSFiddle to illustrate your task. Have you considered putting one together?
• indusBull: I already got the answer now. Next time I will definitely consider the jsfiddle.

Answer 1

you can chain the delay and switchClass to $elements and take the $("#container").find(".animation"); outside the function like this

function startAnimation($elements) {
$elements.switchClass("mc-ylw-blk", "mc-blk-ylw", 0)
 .delay(500).switchClass("mc-blk-ylw", "mc-ylw-blk", 0)
 .delay(250).switchClass("mc-ylw-blk", "mc-blk-ylw", 0)
 .delay(250).switchClass("mc-blk-ylw", "mc-ylw-blk", 0)
 .delay(500).promise().done(function () { 
 startAnimation($elements);
 });
}
var $elements=$("#container").find(".animation");
if ($elements.length) {
 startAnimation($elements);
} 

http://jsfiddle.net/Y95rU/1/

• indusBull: Oh man.. thank you!! Now I'm wondering why I didn't think about this at first place.

Answer 2

Here is a version using setInterval() set at 250 at the second iteration and the sixth(in a set of 6) it skips thus taking twice the time, and then uses the modulus operator(dividing by 2 this time) to detirmine the proper class to display.

var $elements = $("#container").find(".animation");
var i = 0, x = 0;
if ($elements.length) {
 setInterval(function(){
 x++;
 if((x % 6) == 1 || (x % 6) == 5) return;
 class1 = ((i % 2) == 0) ? "mc-ylw-blk" : "mc-blk-ylw";
 class2 = ((i % 2) == 1) ? "mc-ylw-blk" : "mc-blk-ylw";
 $elements.switchClass(class1, class2, 0);
 i++;
 }, 250);
}

And here is the jsFiddle: http://jsfiddle.net/rWBa7/

• indusBull: I fee that this code is more complicated and unreadable. Makes it harder to change it in future if requirement changes.

Answer 3

It would probably be more efficient to simply put all the logic into a loop rather than calling it recursively. You end up with a million copies of that function running until the page closes otherwise.