Front End Web exercise, no HTML

Question

I was given the following instructions

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

<div>This is a cube</div>
<div>This is a triangle</div>
<div>This is a big, green circle</div>

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+

Example Result gif

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;
 border-width: 0 100px 160px 100px;
 border-color: transparent transparent red transparent;
 white-space: nowrap;
 box-sizing: border-box;
 text-indent: -50px;
 line-height: 250px;
 color: white;
}
#circle {
 position: relative;
 top: 250px;
 left: 500px;
 height: 200px;
 width: 200px;
 background-color: green;
 border-radius: 100px;
 -moz-border-radius: 100px;
 -webkit-border-radius: 100px;
 text-align: center;
 color: white;
 display: table-cell;
 vertical-align: middle;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div id='square' class='downUp'>This is a square</div>
<div id='triangle' class='upDown'>This is a triangle</div>
<div id='circle' class='spin'>
 <p>This is a big,
 <br/>green circle</p>
</div>

JSFiddle

I borrowed some code from a StackOverflow answer to make the circle spin, it can be found here

I am sure that the up down code can be found places on the internet as it is super simple.

---

I am sure that there is something that I could be doing better on the JavaScript side of things within the restrictions I have been given.

Answer 1

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.

• Malachi: I thought I had added the plain transform into my code, I must have undone it in my troubleshooting. thank you