0

I've got multiple elements on .list1 and .list2 uls, .list2 is not visible and elements from .list1 triggers elements from .list2. I do not know how many elements there will be so I would like to use loop for some jQuery code with click event. But the loops does not work. What am I doing wrong?

JS Fiddile here

<ul class="list1">
<li class="opt1">option1</li>
<li class="opt2">option2</li>
<li class="opt3">option3</li>
<li class="opt4">option4</li>
<li class="opt5">option5</li>


<ul class="list2">
<li class="opt1">option1</li>
<li class="opt2">option2</li>
<li class="opt3">option3</li>
<li class="opt4">option4</li>
<li class="opt5">option5</li>


for (var x = 1; x <= 5; x++){
  $(".list1 li.opt" + x).click(function() {
      $(".list2 li").removeClass("selected");   
      $(".list2 li.opt" + x).addClass("selected");
  });
}
Gowri
  • 1,832
  • 3
  • 29
  • 53
Ecnelis
  • 55
  • 6
  • class name can be of same to make everything easy! – Dhaval Marthak Sep 26 '14 at 12:47
  • 1
    Reason why orginal code failed. http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example – epascarello Sep 26 '14 at 12:51
  • If you know that you can have multiple chained lists, you maybe transform them in a select and use this http://www.appelsiini.net/projects/chained ( only if you know some stuff will go far away and you might lose control of them ) – stefanz Sep 26 '14 at 13:16

1 Answers1

2

Try to write a single event for this purpose do not go for a for loop, and make use of the .index() to grab the related element from list-2

$('.list1 li').click(function () {
    $('.list2 li').removeClass("selected").eq($(this).index()).addClass("selected");
});

DEMO

Rajaprabhu Aravindasamy
  • 66,513
  • 17
  • 101
  • 130
  • So, that simple, no need for loop? Thank you. But why loop was not working? – Ecnelis Sep 26 '14 at 12:51
  • @Ecnelis, because inside `click` event handler `x` equals to 6, not to current value. – Regent Sep 26 '14 at 12:53
  • @Ecnelis That's because of false scoping. We need to create a scope per iteration to over that issue. But that's a kind of advanced concept, probably you need to go through some online topics related to scoping and closure to get a good grip over it. – Rajaprabhu Aravindasamy Sep 26 '14 at 12:53
  • Just to mention: even though `index()` solves problems like this, it still will be better to use some `data` attribute for mapping elements from one list to another. – Regent Sep 26 '14 at 12:55
  • @Regent But personally, i hate hard coding stuffs.. :\ – Rajaprabhu Aravindasamy Sep 26 '14 at 13:01
  • looks complicated ;) Thanks. – Ecnelis Sep 26 '14 at 13:02
  • @Ecnelis it's actually the shortest and simple solution. You can split code into several lines to make it looks simplier. I think this answer is worth to be accepted, isn't it? – Regent Sep 26 '14 at 13:08
  • 1
    @RajaprabhuAravindasamy well, if these `li` are generated dymanically, and somewhy `.list2` will be formed in another order (or one element from list will be lost), this will crash the whole solution. Yes, it will probably never happen, but nevertheless basing on element's attribute, not on element's position, sounds more reliable to me. But, of course, I vote up for this answer as correct one without HTML restructuring. – Regent Sep 26 '14 at 13:16