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

jQuery animation and rotation effects for a WordPress site

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

Problem

What is the best way to tidy up JavaScript code I have written? It seems very long winded, any suggestions to shorten it?

Note: I'm running it on Wordpress which runs jQuery in nonconflict mode, hence the insane amounts of jQuery.().

jQuery("document").ready(function (jQuery) {

    if (jQuery(window).width()  20) {
            jQuery("#sb-container div").css("transform", "rotate(0deg)");
            jQuery("#sb-container div").css("-webkit-transform", "rotate(0deg)");
            jQuery("#sb-container div").css("-ms-transform", "rotate(0deg)");
            jQuery("#sb-container div").css("-moz-transform", "rotate(0deg)");
            jQuery("#sb-container div").css("-o-transform", "rotate(0deg)");
            jQuery(".main").animate({
                "right": "1%"
            }, "slow");

        } else {
            jQuery("#sb-container").draggable()
        }
    });

});

jQuery(function () {
    jQuery("#sb-container").draggable()
});


How do I go about reducing the amount of code shown here? What are the best practices and what am I missing? Also, do I need the else statements querying the window width since they don't have any functions in them for the most part?

Solution

I'm going to go through a few main points here about your code and at the bottom I'll have a "what I would change" section with inline comments. Do not be overwhelmed by the size of this book, I prefer to be on the side of too much information than too little.

  • Cache your selectors: Probably the most important thing you can do for your code now. As a rule of thumb, if you use a selection more than once, you should cache it. What happens when you use $("someElem") is jQuery queries the DOM to try and find elements that match. So imagine that every time you do that it runs a search. Would it be better if you could save the search results? This way you can look and play with them whenever you want. Ex.:



$("#sb-container div").css("transform", "rotate(0deg)"  );
$("#sb-container div").css("-webkit-transform", "rotate(0deg)"  );

//Should be like this:
var sbContainer = $("#sb-container div"); //Saved my search to a variable, which I use later.
sbContainer.css("transform", "rotate(0deg)"  );
sbContainer.css("-webkit-transform", "rotate(0deg)"  );


  • You can use $ in Wordpress: If you have your code in the footer (which you should be doing) what you'll want to do is wrap it in a IIFE. "What the hell is that?" you may ask. A basic syntax looks like this:



(function () {
    //Code goes in here.
})();


You'll see several variations of this all over the internet as you read to find out more. The one I recommend you use in this case is like this:

;(function ($, window, document) {
    //Code goes here and the $ belongs to jQuery
})(jQuery, window, document);


So lets break this down. The ; in the front is a safety net against unclosed scripts, which can be common if you use plugins in Wordpress, or if you concat and minify your files it will protect you from your function becoming an argument.

Then we pass in $, window, and document. We pass in $ and at the bottom assign it to jQuery. So now in this function, no matter what value the $ carried outside, in here it's jQuery. Then we pass in window and document. These are optional in your case, but a good idea since you do use references to window in your code. It saves window as a local variable, and also will be good when you minify your code as the window reference can be changed to something like a or foo automatically.

Now keep in mind there might be times where you have to put your script in the header. Modernizr is an example of this. Then you'll probably end up using the document.ready. Don't sweat because all you have to do is this:

jQuery(document).ready(function( $ ) {
    //Code goes here and $ will belong to jQuery.
});


I wouldn't rely on $.noConflict

  • Chaining: As pointed out in your question, you should be using something like that. jQuery is particularly good at this and you should make the most of it. Ex.:



$("#someElem").css("background-color", "#fff").animate({"right":"1%"}, "slow").draggable();


I apply all this stuff to this selector, one after the other. If I want to continue chaining but change my selectors, I can use stuff like .find() and continue with my methods.

  • Save function calls: If you read the jQuery source code you'll see that shortcut methods like .click(), .scroll(), and etc. all reference the .on() method. All they do is basically call the .on() method with some variances. Well why not just go straight to the meat and potatoes? Check this out:



$("#foo").click(function() {
    //Do your awesomeness
});

//That does the same thing as this:
$("foo").on("click", function() {
    //Do your awesomeness even awesomener.
});


I highly recommend, if you're going to use jQuery on a regular basis, that you take some time and read through the source code of the methods you're using. To find them quickly just use CTRL+F and type method:. That should jump you straight to what you want to know. This way you can understand what and how you are doing things, and even find better ways to do them on your own.

-Playing with visible elements: You're if statements from what I can tell are trying to detect scroll position and etc. I highly recommend you check out this tiny plugin. It basically detects if an element is visible or not at any given time. This may or may not help you out but I just thought I'd throw that out there.

-Things I'd write differently: Here's an example of all of this summed up into your actual code. I've included comments so you can relate to the reading part.

```
;(function ($, window) { //Here's our IIFE giving us full $ power
//Right here at the top we declare our selection variables to be used throught the code.
//Don't forget to replace your selectors with the appropriate variable name.
var main = $(".main"),
sbContainer = $("#sb-container"),
sbContainerDiv = sbContainer.find("div");

function animate() { //Write your code only once, and call it when you need it.
sbContainerDiv.css

Code Snippets

$("#sb-container div").css("transform", "rotate(0deg)"  );
$("#sb-container div").css("-webkit-transform", "rotate(0deg)"  );

//Should be like this:
var sbContainer = $("#sb-container div"); //Saved my search to a variable, which I use later.
sbContainer.css("transform", "rotate(0deg)"  );
sbContainer.css("-webkit-transform", "rotate(0deg)"  );
(function () {
    //Code goes in here.
})();
;(function ($, window, document) {
    //Code goes here and the $ belongs to jQuery
})(jQuery, window, document);
jQuery(document).ready(function( $ ) {
    //Code goes here and $ will belong to jQuery.
});
$("#someElem").css("background-color", "#fff").animate({"right":"1%"}, "slow").draggable();

Context

StackExchange Code Review Q#24499, answer score: 8

Revisions (0)

No revisions yet.