efficiency of jquery toggler

Question

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.

Answer 1

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.

Answer 2

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: "&nbsp; toggler &nbsp;", '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).