Reputation: 2995
Here's the link to my code: http://jsbin.com/edago3/edit
I would love to find out what improvements could be made to make it smaller and more efficient.
Any help is appreciated.
Upvotes: 0
Views: 152
Reputation: 50185
Looks well written to me, nice job! The only small thing I found that could be improved is line 17, change:
.html(" toggler ")
to:
.text("toggler")
and then pad .gm-toggler
on the left and right with CSS.
Using .html
relies on .innerHTML
and will be slower than a standard Javascript text insertion (but it would write out the
as text, hence the padding, which is better for separating presentation anyway).
Also, in the line just before that, you can just say $('<span>')
and jQuery will construct it exactly as you've written. Just more readable.
Upvotes: 0
Reputation: 630429
There are a number of improvements you can make if you're looking for more concise code, you can see a full updated sample here. I'll list the main areas you can save code on below (for equivalent functionality of course).
The main bits:
$($li).children('a').after(
$(document.createElement('span'))
.html(" toggler ")
.addClass("gm-toggler")
.hide()
);
Can be shortened to...
$li.children('a').after(
$('<span>', { html: " toggler ", 'class': "gm-toggler"}).hide()
);
This...
$li.hover(function() {
$('.gm-toggler', this).show();
}, function(){
$('.gm-toggler', this).hide();
});
Can be shortened to...
$li.hover(function(){
$('.gm-toggler', this).toggle();
});
This...
if($(this).parent('li').hasClass('active')){
// ... remove its active class ...
$(this).parent('li').removeClass('active');
} else {
// ... otherwise give it an active class.
$(this).parent('li').addClass('active');
}
Can be shortened to....
$(this).parent('li').toggleClass('active');
It's probably better to ask which parts you have questions on, the relevant documentation for the methods I used can be found here: .toggle()
, .toggleClass()
, jQuery(html, props)
.
Upvotes: 2