HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Fade-in and fade-out in pure JavaScript

Submitted by: @import:stackexchange-codereview··
0
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.


        
        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 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.