Front End Web exercise, no HTML

I was given the following instructions


Using the below HTML, using ONLY CSS, JavaScript or jQuery create the following (see attached gif animation):

This is a cube
This is a triangle
This is a big, green circle

Rules:



-
The HTML cannot be modified in any way

-
For the animation portion, please use JavaScript; however, no 3-rd party jQuery or JavaScript plugins allowed

-
Compatibility: IE 9+, FF 4+, Chrome 10+, Safari 5.1+


I fudged the HTML some, especially when it came to making the Circle's text wrap to a second line and stay in the middle of the Circle.



$(document).ready(function() {
downUp();
upDown();
spin();
});

function downUp() {
$('.downUp').animate({'top': 500}, {
duration: 1500,
complete: function() {
$('.downUp').animate({'top': 0}, {
duration: 1500,
complete: downUp
});
}
});
}

function upDown() {
$('.upDown').animate({'top': 0}, {
duration: 1500,
complete: function() {
$('.upDown').animate({'top': 500}, {
duration: 1500,
complete: upDown
});
}
});
}

function spin() {

var degree = 0, timer;
rotate();
function rotate() {
timer = setTimeout(function() {
$('.spin').css({ WebkitTransform: 'rotate(' + degree + 'deg)'});
$('.spin').css({ '-moz-transform': 'rotate(' + degree + 'deg)'});
$('.spin').css({ '-ms-transform' : 'rotate(' + degree + 'deg)'});
++degree; rotate();
},5);
}
}
`#square {
position: absolute;
height: 200px;
width: 200px;
background: black;
/ For browsers that do not support gradients /
background: -webkit-linear-gradient(black, gray, black);
/ For Safari 5.1 to 6.0 /
background: -moz-linear-gradient(black, gray, black);
/ For Firefox 3.6 to 15 /
background: linear-gradient(black, gray, black);
text-align: center;
line-height: 200px;
color: white;
}
#triangle {
position: absolute;
left: 250px;
top: 500px;
width: 0px;
height: 50px;
border-style: solid;

There isn't much (that I see) to say about this code; it is mostly straightforward and easy to understand. Therefore, my review will be fairly short.

Refactoring

Take a look at your upDown and downUp functions. These are very similar. The only differences are:

function downUp() {
    $('.downUp').animate({'top': 500}, {        <-- x2
        duration: 1500, 
        complete: function() {
            $('.downUp').animate({'top': 0}, {  <--
                duration: 1500, 
                complete: downUp                <--
            });
        }
    });
}

To simplify your code, you can combine these two functions and simply add an extra parameter: reversed.

function downUp(reversed) {
    var elements = $(reversed ? ".upDown" : ".downUp");
    var to = reversed ? 0 : 500; // the 'top' value to go *to*
    var back = reversed ? 500 : 0; // the 'top' value to go back to 

    elements.animate({'top': to}, {
        duration: 1500, 
        complete: function() {
            elements.animate({'top': back}, {
                duration: 1500, 
                complete: function() { downUp(reversed) }
            });
        }
    });
}

A few other changes I made:

• Stored the $('...') elements in a variable (faster)
• Also store the 'top' values in variables (cleaner)

Now, your code is simpler since you don't have two functions that look almost the same, yet are doing opposite tasks. Now, in the $(document).ready(), you have:

downUp();
downUp(true);
spin();

Taking spin for a spin

Your spin function looks good for the most part. As I am not that familiar with jQuery, I don't know any other way to spin an element other than to use CSS animations (like you did).

I've made some minor changes to this function:

function spin() {
    var degree = 0, elements = $('.spin');
    rotate();
    function rotate() {
        setTimeout(function() {
            elements.css({ WebkitTransform: 'rotate(' + degree + 'deg)'});  
            elements.css({ '-moz-transform': 'rotate(' + degree + 'deg)'});                      
            elements.css({ '-ms-transform' : 'rotate(' + degree + 'deg)'});
            elements.css({ 'transform'     : 'rotate(' + degree + 'deg)'});

            ++degree;
            rotate();
        },5);
    }
}

Here are some things that I changed:

• Fixed indentation (cleaner).

• Stored $('.spin') in a variable (faster). Note that DOM interaction functions like these where you are grabbing elements can be costly (especially the $() function because not only does it try to parse potentially complex parameters, it also attempts to wrap all the return elements).

• Removed the timer variable since it is not needed (you don't use it at all).

Also, about the CSS animation properties that you are adding/changing, I believe that there are quite a few more than the ones you have listed. For one, I know that there is just a plain 'transform' property (which I added above).

I don't know how necessary these other transform types are (type as in webkit, moz, etc), but I think it would be best to find a list of them and to include them to make sure that your code is as compatible as possible with other browsers.

StackExchange Code Review Q#119888, answer score: 5