0

What I want is to click on #bt-1 and change the color of #target-1, click on #bt-2 and change the color of #target-2... I started writing a particular click event handler for each #bt-n / #target-n but as the site got bigger I thought about using a loop. My approach was using a for loop with variables in jQuery selectors. Here is my code:

$(document).ready(function() {
  
  var total = $('.target').length;
  for(n=1; n<=total; n++) {
    var num = String(n);
    $('#bt-'+num).on('click', function() {
      $('#target-'+num).toggleClass('yellow');
    });
  }
  
});
.wrapper {
  display: flex;
  text-align: center;
}
.button, .target {
  padding: 20px;
  margin: 10px;
}
.button {
  background: gray;
}
#target-1 {
  background: red;
}
#target-2 {
  background: green;
}
#target-3 {
  background: blue;
}
.yellow {
  background: yellow !important;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="wrapper">
  <div id="bt-1" class="button">
    <h1>Button 1</h1>
  </div>
  <div id="target-1" class="target">
    <h1>Target 1</h1>
  </div>
</div>
<div class="wrapper">
  <div id="bt-2" class="button">
    <h1>Button 2</h1>
  </div>
  <div id="target-2" class="target">
    <h1>Target 2</h1>
  </div>
</div>
<div class="wrapper">
  <div id="bt-3" class="button">
    <h1>Button 3</h1>
  </div>
  <div id="target-3" class="target">
    <h1>Target 3</h1>
  </div>
</div>

I don't understand why it only targets the last #target-n as the loop seems to be working on #bt-n. I also thought about using an array but can't figure out how to implement it.

I managed to make it work using $(this).siblings('.target')... which do not require the for loop and ids but a parent element for each .button / .target, in this case .wrapper Code Here. Although this was a good solution, I would like to understand what I did wrong and how to properly implement a loop to achieve this without using the parent .wrapper. Thank you.

guizo
  • 2,594
  • 19
  • 26
  • instead of `'bt-'+num` i think you can use `'bt-'+n` – AzizurRahamanCA Dec 23 '16 at 13:12
  • Possible duplicate of [Assign click handlers in for loop](http://stackoverflow.com/questions/4091765/assign-click-handlers-in-for-loop) – Mike Scotty Dec 23 '16 at 13:14
  • 1
    use `$(this).next().toggleClass('yellow')` – Kevin Kloet Dec 23 '16 at 13:15
  • @RandomDeveloper At first I was using bt+n. If I use `'#bt-'+n` / `'#target'+n` on this snippet nothing happens while using `'#bt-'+num` / `'#target'+num` at least the the last object changes color. Here is where it gets strange: This works: `$('#bt-'+n).on('click', function() { $(this).toggleClass('yellow'); });` – guizo Dec 23 '16 at 16:41

2 Answers2

2

The reason that only the last item gets affected is because the loop has completed before any event fires. Therefore n holds the last value in the loop. To fix this you need to use a closure:

for (n = 1; n <= total; n++) {
  (function(n) {
    $('#bt-' + n).on('click', function() {
      $('#target-' + n).toggleClass('yellow');
    });
  })(n);
}

That said, a much better approach would be avoid the loop and to use DOM traversal to find the .target related to the clicked .button, like this:

$('.button').click(function() {
  $(this).next('.target').toggleClass('yellow');
});
.wrapper {
  display: flex;
  text-align: center;
}
.button,
.target {
  padding: 20px;
  margin: 10px;
}
.button {
  background: gray;
}
#target-1 {
  background: red;
}
#target-2 {
  background: green;
}
#target-3 {
  background: blue;
}
.yellow {
  background: yellow !important;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="wrapper">
  <div id="bt-1" class="button">
    <h1>Button 1</h1>
  </div>
  <div id="target-1" class="target">
    <h1>Target 1</h1>
  </div>
</div>
<div class="wrapper">
  <div id="bt-2" class="button">
    <h1>Button 2</h1>
  </div>
  <div id="target-2" class="target">
    <h1>Target 2</h1>
  </div>
</div>
<div class="wrapper">
  <div id="bt-3" class="button">
    <h1>Button 3</h1>
  </div>
  <div id="target-3" class="target">
    <h1>Target 3</h1>
  </div>
</div>
Rory McCrossan
  • 331,213
  • 40
  • 305
  • 339
  • I thought that `num` was holding only the last value but it is strange because the click event fires if I click `#bt-1`, `#bt-2` or `#bt-3`. So somehow not only the last valye is being passed at this line which does not happen in the next line for the `'#target'+num`. – guizo Dec 23 '16 at 16:55
  • Using `next()` sure is a better solution that using `siblings()` as I did. Thank you. – guizo Dec 23 '16 at 16:56
  • 1
    @guizo the event is attached to the correct elements, because the `num` is holds the correct value when finding those elements. The event handler itself attached to the elements isn't used until later, after the loop runs, hence `num` always has the final value. – Rory McCrossan Dec 23 '16 at 17:30
0

It is unwise to register a lot of event handlers. You can bind one event handler and perform action for given specific idx read from element id, eg:

$('body').on('click', function (event) {
    if (!event.target.id.match(/^bt-\d+/)) {
        return; //id of clicked element does not match bt-{number}
    }

    var idx = event.target.id.replace('bt-', ''); //remove prefix "bt-" and leave only numeric postfix

    $('#target-' + idx).toggleClass('yellow');
});

Explanation:

When you bind click on body element You are getting access to all click events from child elements that not cancelled passing that event up. Element that has been clicked in saved inside event.target and it has property id in event.target.id.

On this id property I call match function with regular expression - it will match string which starts ^ from bt- and have any number \d at least one one + .

if (!event.target.id.match(/^bt-\d+/)) {
    return; //id of clicked element does not match bt-{number}
}

There is negation of this statement, so If this id is not in format bt-someNumber it will not go further.

var idx = event.target.id.replace('bt-', '');

Than takes id and replaces bt- part in it with empty string ''

$('#target-' + idx).toggleClass('yellow');

Finally You are toggling class on element with same number as button but with different prefix target- instead of bt-.

Happy hacking!

Hakier
  • 505
  • 2
  • 13