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({ //You should apply a class instead of doing all of this in here like this.
"transform": "rotate(0deg)",
"-webkit-transform": "rotate(0deg)",
"-ms-transform": "rotate(0deg)",
"-moz-transform": "rotate(0deg)",
"-o-transform": "rotate(0deg)"
});
main.animate({"right":"1%"}, "slow");
}
if ( window.width() < 960) {
main.animate({"top":"22%"});
//This use of .toggle() was deprecated in jQuery 1.8 and removed in 1.9
//Consider doing this differently
//If you provide me with more details on this I can help you come up with a different solution
$('#tblcontents').toggle(function() {
main.fadeOut(1000);
}, function() {
main.fadeIn(1000);
main.animate({"top":"22%"});
});
} else {
$('.menu-button').on('click', function() {
animate(); //Here we call our previously set animate function
});
}
//Refer to the previous toggle comment.
$('.toggle_div').toggle(function() {
main.animate({"right":"50%"}, "slow");
}, function(){
main.animate({"right":"1%"}, "slow");
});
$(".pulse").effect("pulsate", { times:100 }, 2000).on('click', function() {
$(this).effect().stop(true, true); //In a callback function, 'this' refers to the element in question.
$(this).animate({"opacity":"1"}, "fast");
});
$('.thumb').on('click', function() {
animate();
});
window.on('scroll', function() {
if ($(this).scrollTop() > 20) {
animate();
} else{
sbContainer.draggable();
}
});
//You only need to make this draggable once, either here or in your else statement.
//Remove one depending on what you need.
sbContainer.draggable()
})(jQuery, window); //Here we set our awesome stuff in the function
UPDATE
I've looked at your .js file and there's quite a bit I would change. But focus on the resize event for now. The way you have it now, the code runs hundreds of times since it is called each time the window size changes, even if the user isn't done resizing.
Now we don't want to do that since it can really slow down the browser, if not crash it all together. So what you want to do is only run when the user stops or is done resizing, then run the code. Here's an example of how I'd probably do it:
var $window = $(window); //Remember, if you use a selection more than once, you should cache it.
$window.resize(function() {
//Check if there has been a call already
//If the check is true that means the code was already called
//So we cancel any previous calls, this way the code is only called once
if(this.resizeTO) clearTimeout(this.resizeTO);
this.resizeTO = setTimeout(function() {
$(this).trigger('resizeEnd'); //Call the actual code with a 500ms delay
}, 500);
});
$window.on('resizeEnd', function() {
//This is a custom event that is triggered in the timeout
//This will only run on the last time the resize method was called
var winWidth = $window.width();
if( winWidth < 960 ) {
toggletwo();
animate();
} else {
toggleone();
}
});