1
\$\begingroup\$

How can I simplify this jQuery code? I feel like I should be able to do this with one click function.

jQuery

$('.btn').eq(0).click(function () { $('.carousel').css({ 'left': '0%' }); });
$('.btn').eq(1).click(function () { $('.carousel').css({ 'left': '-100%' }); });
$('.btn').eq(2).click(function () { $('.carousel').css({ 'left': '-200%' }); });
$('.btn').eq(3).click(function () { $('.carousel').css({ 'left': '-300%' }); });

HTML

<ul>
<li><a href="" class="btn">Button 1</a></li>
<li><a href="" class="btn">Button 2</a></li>
<li><a href="" class="btn">Button 3</a></li>
<li><a href="" class="btn">Button 4</a></li>
</ul>
\$\endgroup\$
3
\$\begingroup\$

You are correct, you can have one function binding click events to all of your .btn elements.

Working Fiddle Example:

$('.btn').bind("click", function(event) {
  event.preventDefault();
  var index = $(this).parent().index();
  $('.carousel').css({ 'left': '-' + (index*100) + '%' });
});

jQuery documentation about index().

Describing what's being suggested:

  1. Bind the click to the .btn class and within the click event you ascertain the index position:

    I've opted to use bind() since it's performance efficient when compared with click().
    Another reason is the fact that all your elements share the same class, so, you only need to bind a click event to the class instead of attaching a click handler to each .btn.

  2. Use the index and multiply it by 100 as to get the desired left value:

    This way you can easily ascertain the correct value since you have a direct relation between the anchor's parent index and the desired left value.

  3. Using event.preventDefault(); to prevent the browser from following the href:

    With this, you tell the browser to leave the href attribute alone, thus preventing it to bubble up.

  4. Safe to use the minus signal all the time, since 0 and -0 is the same thing.


In order to further improve the function, and if using the lastest jQuery, I would give an id to the ul element, thus losing the .btn class, having the click event binded to a single DOM element thru delegation:

$('#myUL').on("click", "a", function(event) {
  event.preventDefault();
  var index = $(this).parent().index();
  $('.carousel').css({ 'left': '-' + (index*100) + '%' });
});

jQuery documentation about on().

Useful reading: Why use jQuery on() instead of click().

\$\endgroup\$
  • \$\begingroup\$ This is perfect. Thanks for the extra information as well. \$\endgroup\$ – Kris Hollenbeck Oct 2 '12 at 19:38
  • \$\begingroup\$ Why not use .on() directly in your first example? Also, I would bind to the anchor and not to the li. \$\endgroup\$ – ANeves Oct 2 '12 at 19:52
  • \$\begingroup\$ @ANeves Thank you, I've updated the answer as I should have placed the bind over the anchor in the first place... (distracted). About both suggestions, the first one is to cover an old jQuery version as I don't know what version is OP using. \$\endgroup\$ – Zuul Oct 2 '12 at 20:02
1
\$\begingroup\$

Some problems you have to be aware of:

var index = $(this).index();

...constructs a singleton jQuery set, so index() will be a constant 0.

var index = $(this).parent().index();

...coincidentally does the trick here. But it's not the index from the selected jQuery set, but rather the index of the selected DOM node within the parent node's children. It will not yield the desired result anymore if you f.ex. insert an additional <li>Buttons</li> as the first item in the <ul>.

var set = $('.btn');
set.click(function() {
    console.log('via set: ' + set.index(this));
});

​...will always yield the desired result, no matter what other nodes are around.

I created a fiddle http://jsfiddle.net/4jxvZ/1/ that shows the three different behaviors (open your browser's js console, it logs the different index values).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.