I decided to build my own fade-in fade-out function, since that is all I need on my page.

Please comment on things I can make better.

<!DOCTYPE html>
<html>
<title></title>
<meta charset="utf-8" />

<body>
<div>
</div>
</div>

<script type="text/javascript">
// global varibles
var done = true,

fading_div.style.filter = 'alpha(opacity=' + opacity_value + ')';
}
done = true;
}
done = true;
}
}

if (done && fading_div.style.opacity !== '1') {
done = false;
for (var i = 1; i <= 100; i++) {
setTimeout("function_opacity(" + i + ",'in')", i * 5);
}
}
};

if (done && fading_div.style.opacity !== '0') {
done = false;
for (var i = 100; i >= 1; i--) {
setTimeout("function_opacity(" + i + ",'out')", (i - 100) * -1 * 5);
}
}
};
</script>
</body>
</html>

-
You could skip all of this and let css transitions do the work for you. css3.bradshawenterprises.com/transitions – Dagg Dec 31 '11 at 0:34
GGG, maybe it's a good idea. People will be updating there browsers more often in 2012. – Hakan Dec 31 '11 at 10:33
Yeah, I think it's fine for something like a fade (progressive enhancement). People who's browsers don't support it won't see the fade, but the thing will still appear/disappear, so they're just missing out on a visual effect, and you get to save 50 or 60 lines of javascript. – Dagg Dec 31 '11 at 17:54

Just some generic notes about the JavaScript code: I'd extract out a setOpacity function and create a fadeOut and a fadeIn function too.

function setOpacity(opacity) {
fading_div.style.filter = 'alpha(opacity=' + opacity + ')';
}

setOpacity(opacity);
if (opacity == 1) {
done = true;
}
}

setOpacity(opacity);
if (opacity == 1) {
}
if (opacity == 100) {
done = true;
}
}

...
setTimeout("fadeIn(" + i + ")", i * 5);
...


It eliminates the in and out magic constants and lots of conditions which checks their values.

Using variable name suffixes like opacity_value looks a little bit redundant (since variables stores values), so I've renamed them. The same is true for the function_opacity function.

I've changed the second for loop too to use fewer arithmetic operations, I think the following is easier to read:

for (var i = 1; i <= 100; i++) {
setTimeout("fadeOut(" + (100 - i) + ")", i * 5);
}

-
Please don't pass strings to setTimeout! It evals them. Pass a function. setTimeout((function(x){ return function(){ fadeOut(100-x); }; })(i), i * 5); It's in a closure because i changes in the for loop. – Rocket Hazmat Dec 30 '11 at 21:58
Thanks, @Rocket! Could you give me an example or link with some explanation why is it better? I'm not a JavaScript guru. (Maybe you want to write it as an answer since in the question there is the same string passing, and we will be able to upvote it.) – palacsint Dec 30 '11 at 22:08

This is just an extension to @palacsint's answer. You shouldn't pass strings to setTimeout, it uses eval, which is inefficient and insecure. You should pass a function.

Problem is, in the for loop i changes, so you'll have to use a closure.

Don't do this:

for (var i = 100; i >= 1; i--) {
setTimeout("function_opacity(" + i + ",'out')", (i - 100) * -1 * 5);
}


for (var i = 100; i >= 1; i--) {
setTimeout((function(x){
return function(){
function_opacity(x, 'out')
};
})(i), (i - 100) * -1 * 5);
}


This may look a little messy. I suggest declaring a function that returns a function separately.

function call_opacity(i, d){
return function(){
function_opacity(i, d);
};
}


Then do:

for (var i = 100; i >= 1; i--) {
setTimeout(call_opacity(i, 'out'), (i - 100) * -1 * 5);
}

-
Thanks Rocket! Here is a link on the subject to back up your answer. Thanks for shring! stackoverflow.com/questions/6232574/… – Hakan Dec 31 '11 at 9:14

## protected by Mat's Mug♦Jan 22 '14 at 15:46

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).