0

I wanted to add event listener to all checkboxes on my page.

<input type="checkbox" name="sel[]" id="1" value="1">
<input type="checkbox" name="sel[]" id="2" value="2">
<input type="checkbox" name="sel[]" id="3" value="3">
<input type="checkbox" name="sel[]" id="4" value="4">
<input type="checkbox" name="sel[]" id="5" value="5">
<input type="checkbox" name="sel[]" id="6" value="6">
<!--  .....  -->

<script>
var arr =  document.getElementsByName("sel[]");

var copy = [];

for (i = 0; i < arr.length; i++)
    copy[i] = arr[i].id;


var checkboxes = [];
var data;


for (j = 0; j < arr.length; j++)
{
    checkboxes[j] = document.getElementById(copy[j]);
    checkboxes[j].addEventListener('change', (event) => {
    if (event.target.checked) 
    {
        data = j;
        $.post("/php/ses.php", {data:data});
    }
    else 
    {
        data = (-1)*j;
        $.post("/php/ses.php", {data:data});
    }
 });
</script>

It works for every checkbox, but always sends 6 if any checkbox is checked (or -6 when unchecked). I wanted to: 1 and -1 for first chceckbox, 2 and -2 for second etc, not 6/-6 for every.

  • why not attach a single event handler to a parent element? The [change event will bubble.](https://developer.mozilla.org/en-US/docs/Web/Events/change). Also, might be better to use `event.target.id` or `.value` to get your index. It's the scope of the `j` variable: by the time the events fire, `j` will always have iterated to its max value. – David784 Jan 06 '19 at 18:52

3 Answers3

0

As far as you use JQuery this code may be more preferable

var checkboxes =  $('[name="sel[]"]');

//listen all checkboxes change event
checkboxes.on('change', function(){
    var id = this.value; // id equals j in your code
    $.post("/php/ses.php", { data: this.checked ? id: -id }); // send data id if checked and -id if not checked
});
0

You could use checkboxes data value for your data instead of keeping a separate instance of it.

var arr =  document.getElementsByName("sel[]");
for (j = 0; j < arr.length; j++){
    arr[j].addEventListener('change', (event) => {  
    if (event.target.checked) {
        //$.post("/php/ses.php", {data:data});
        console.log({data: parseInt(event.target.value)});
    }else {
        //$.post("/php/ses.php", {data:data});
        console.log({data: -parseInt(event.target.value)});
    }
 });
 }
<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
</head>
<body>
<input type="checkbox" name="sel[]" id="1" value="1">
<input type="checkbox" name="sel[]" id="2" value="2">
<input type="checkbox" name="sel[]" id="3" value="3">
<input type="checkbox" name="sel[]" id="4" value="4">
<input type="checkbox" name="sel[]" id="5" value="5">
<input type="checkbox" name="sel[]" id="6" value="6">
</body>
</html>

The reason your code ends up with 6 is that your data is a global variable and gets updated after each loop, and it ends up with 6 at the end, there is no local data for event listener for each item on arr. Please check this to learn more about it JavaScript closure inside loops – simple practical example

azs06
  • 3,467
  • 2
  • 21
  • 26
0

While the answer by Prokhor Orlov is correct I want to explain what is wrong in OP. At the moment when event handler fires the for loop iterator is already ran out of the loop and has its max value. This error is very common in beginners and the question was asked 100+ times. The solution is also well known, use closure in the loop. Something like this.

for (i = 0; i < arr.length; i++) {
(function(j){
  checkboxes[j] = document.getElementById(copy[j]);
  checkboxes[j].addEventListener('change', (event) => {
    if (event.target.checked) {
      data = j;
      $.post("/php/ses.php", {
        data: data
      });
    } else {
      data = (-1) * j;
      $.post("/php/ses.php", {
        data: data
      });
    }
  });
})(i);
}

Disclaimer: This is not recommended code in this case

Alex Kudryashev
  • 9,120
  • 3
  • 27
  • 36