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

Ribbon animation code too repetitive

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
animationribbontoorepetitivecode

Problem

I spent the last two days getting a ribbon animation to work with divs so that I can later add li link elements to them to act as a menu. I used the progess bar animation code as inspiration from w3schools. The animation works great, only issue is that the code is very un-DRY. I tried to simplify the code by not repeating myself in the CSS and especially not in the javascript but I kept getting closure issues even after using self invoked functions. I would be greatful if anyone can give me some advice or even a solution that will greatly shorten my code to have the same animation effect. I know that I will have several ribbons on the page, each with different number of ribbon layers, so the code as it is can get very complicated very fast. N.B the code below is the working version.



`function move1() {
var width = 0;
var elem = document.getElementById("ribbon-part-1");
var id = setInterval(frame, 1);

function frame() {
if (width >= 100) {
clearInterval(id);
} else {
width++;
elem.style.width = width + '%';
}
}
}

function move2() {
var width = 0;
var elem = document.getElementById("ribbon-part-2");
var id = setInterval(frame, 1)

function frame() {
if (width >= 100) {
clearInterval(id);
} else {
width += 2;
elem.style.width = width + '%';
}
}
}

function move3() {
var width = 0;
var elem = document.getElementById("ribbon-part-3");
var id = setInterval(frame, 1)

function frame() {
if (width >= 100) {
clearInterval(id);
} else {
width += 2;
elem.style.width = width + '%';
}
}
}

function move4() {
var width = 0;
var elem = document.getElementById("ribbon-part-4");
var id = setInterval(frame, 1)

function frame() {
if (width >= 100) {
clearInterval(id);
} else {
width += 2;
elem.style.width = width + '%';
}
}
}

function move5() {
var width = 0;
var elem = document.getElementById("end-ribbon");
var

Solution

A DRYer version of your code could be:

function prepare(elem, rate, width) {
    var width = 0;
    return function frame() {
        if (width >= 100) return;
        width += rate;
        elem.style.width = width + '%';
        setTimeout(frame, 5);
    }
}

function move() {
    ['ribbon-part-1', 'ribbon-part-2', 'ribbon-part-3', 'ribbon-part-4', 'end-ribbon'].forEach(function(id, i) {
            var el = document.getElementById(id);
            setTimeout(prepare(el, i == 0 ? 1 : 2), i * 300);
    });
}


jsFiddle: https://jsfiddle.net/wLwo0vs6/

I use the ids of the elements to determine how many parts/things that has to move and like that I don't have to hard code stuff. You could also get them from the DOM knowing the starting button that was pressed. This way you could use the exact same code for all the places you have.

But I don't like the animation not being synced properly, the correct for me would be that when one section reached 100% the next section would start the animation. So I suggest a modification that could be done like this:

function prepare(elems, width) {
    var width = 0;
    var elem = elems.shift();
    return function frame() {
        if (width >= 100) {
            elem = elems.shift();
            if (!elem) return;
            width = 0;
        } else {
            width++;
            elem.style.width = width + '%';
        };
        setTimeout(frame, 5);
    }
}

function move() {
    var elements = ['ribbon-part-1', 'ribbon-part-2', 'ribbon-part-3', 'ribbon-part-4', 'end-ribbon'].map(function(id) {
        return document.getElementById(id);
    });
    setTimeout(prepare(elements), 300);
}


jsFiddle: https://jsfiddle.net/db8hbpry/

The missing part is the logic for accelerating. If you know what you want there I can adjust.

More than this, small things could be more pressed, but I think it would reduce readability.

Code Snippets

function prepare(elem, rate, width) {
    var width = 0;
    return function frame() {
        if (width >= 100) return;
        width += rate;
        elem.style.width = width + '%';
        setTimeout(frame, 5);
    }
}

function move() {
    ['ribbon-part-1', 'ribbon-part-2', 'ribbon-part-3', 'ribbon-part-4', 'end-ribbon'].forEach(function(id, i) {
            var el = document.getElementById(id);
            setTimeout(prepare(el, i == 0 ? 1 : 2), i * 300);
    });
}
function prepare(elems, width) {
    var width = 0;
    var elem = elems.shift();
    return function frame() {
        if (width >= 100) {
            elem = elems.shift();
            if (!elem) return;
            width = 0;
        } else {
            width++;
            elem.style.width = width + '%';
        };
        setTimeout(frame, 5);
    }
}

function move() {
    var elements = ['ribbon-part-1', 'ribbon-part-2', 'ribbon-part-3', 'ribbon-part-4', 'end-ribbon'].map(function(id) {
        return document.getElementById(id);
    });
    setTimeout(prepare(elements), 300);
}

Context

StackExchange Code Review Q#136086, answer score: 2

Revisions (0)

No revisions yet.