patternjavascriptMinor
Fade-in and fade-out in pure JavaScript
Viewed 0 times
purejavascriptfadeandout
Problem
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.
Please comment on things I can make better.
Fade In |
Fade Out
Fading Box
// global varibles
var done = true,
fading_div = document.getElementById('fading_div'),
fade_in_button = document.getElementById('fade_in'),
fade_out_button = document.getElementById('fade_out');
function function_opacity(opacity_value, fade_in_or_fade_out) { // fade_in_or_out - 0 = fade in, 1 = fade out
fading_div.style.opacity = opacity_value / 100;
fading_div.style.filter = 'alpha(opacity=' + opacity_value + ')';
if (fade_in_or_fade_out == 'in' && opacity_value == 1) {
fading_div.style.display = 'block';
}
if (fade_in_or_fade_out == 'in' && opacity_value == 100) {
done = true;
}
if (fade_in_or_fade_out == 'out' && opacity_value == 1) {
fading_div.style.display = 'none';
done = true;
}
}
// fade in button
fade_in_button.onclick = function () {
if (done && fading_div.style.opacity !== '1') {
done = false;
for (var i = 1; i = 1; i--) {
setTimeout("function_opacity(" + i + ",'out')", (i - 100) * -1 * 5);
}
}
};
alert (test);
Solution
Just some generic notes about the JavaScript code: I'd extract out a
It eliminates the
Using variable name suffixes like
I've changed the second for loop too to use fewer arithmetic operations, I think the following is easier to read:
setOpacity function and create a fadeOut and a fadeIn function too.function setOpacity(opacity) {
fading_div.style.opacity = opacity / 100;
fading_div.style.filter = 'alpha(opacity=' + opacity + ')';
}
function fadeOut(opacity) {
setOpacity(opacity);
if (opacity == 1) {
fading_div.style.display = 'none';
done = true;
}
}
function fadeIn(opacity) {
setOpacity(opacity);
if (opacity == 1) {
fading_div.style.display = 'block';
}
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);
}Code Snippets
function setOpacity(opacity) {
fading_div.style.opacity = opacity / 100;
fading_div.style.filter = 'alpha(opacity=' + opacity + ')';
}
function fadeOut(opacity) {
setOpacity(opacity);
if (opacity == 1) {
fading_div.style.display = 'none';
done = true;
}
}
function fadeIn(opacity) {
setOpacity(opacity);
if (opacity == 1) {
fading_div.style.display = 'block';
}
if (opacity == 100) {
done = true;
}
}
...
setTimeout("fadeIn(" + i + ")", i * 5);
...for (var i = 1; i <= 100; i++) {
setTimeout("fadeOut(" + (100 - i) + ")", i * 5);
}Context
StackExchange Code Review Q#7315, answer score: 6
Revisions (0)
No revisions yet.