Props to you on your first plugin! Well here are a few pointers in no particular order:
- Double Wrapping: When you cache yDiv=$(this);, you're wrapping the element with the jQuery object. And then you do: $(yDiv).not(... What this does is wrap that object twice. You've already wrapped it so you don't need to do so again. Just do this yDiv.not(... after you cache.
So this is how you should do it:
//Don't
yDiv = $(this);
$(yDiv).not('.opened').addClass('closed');
//Instead do:
yDiv = $(this);
yDiv.not('.opened').addClass('closed');
- Variable Names: If you want anyone else to provide you input on your code, they have to be able to understand it. You should use variable names that will make sense to anyone reading your code. Yes, in this simple example one can deduce the what and why you named the variables the way you did and we can sort of follow along. But when you start working on large scale projects, things can get out of hand real quick. Take for example your yDiv variable. The keyword this can refer to any element that goes through the .each() which might not be a div in the first place. What if it was a ul or any other element. In that case, the name yDiv would no longer represent what is actually stored in there. Anyways, try to use meaningful and descriptive variable names and stay away from names that might not exactly represent what is stored in said variable.
- Cache your selectors: 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 has to jump into the DOM and look through all elements that would match that selection. So you should really do a search only once, and save your results for future use. This way you can look and play with them whenever you want without having to try and find them again.
Ex:
var closestElem = $(this).closest(yDiv); //Cache it
if (closestElem.hasClass('closed')){ //Now use it
//...
}
- Save a 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 parameters. 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.
});
//You can also use that to set up your own custom events
$("foo").on("myEvent", function() {
//bar
});
That just makes the syntax so easy to remember as well as provide you with enormous control over your events. If you're going to use jQuery on a regular basis, I recommend 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 methodName:. That should jump you straight to what you want to know. This way you can understand what and how they are doing things, and even find better ways to do them on your own. Also if you see something you think should be done better or differently, you understand how it works and you can contribute to jQuery.
