CODEWITHSUNDEEP
javascripthtmldom
Thuan Do
Thuan Do

Reputation: 51

Toggle class which be clicked in multiple class - DOM

I try to toggle class when td clicked in multiple class but it does not work. I prefer working on DOM.

window.onload = function() {
 var seats = document.getElementsByClassName('seat');

 for(var i = 0; i < seats.length; i++) {
 seats[i].onclick = function() {
   seats[i].classList.toggle = 'selected';
  }
 }
}
td {
  width: 30px;
  height: 30px;
  background-color: red;
  color: white;}

td.selected {
  background-color: black;
}
<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>JS Bin</title>
</head>
<body>
<table>
  <tr>
    <td class="seat">1</td>
    <td class="seat">2</td>
    <td class="seat">3</td>
    <td class="seat">4</td>
    <td class="seat">5</td>
  </tr>
</table>
</body>
</html>

http://jsbin.com/kufemuhagi/edit?html,css,js,output

Upvotes: 3

Views: 101

Answers (3)

Zakaria Acharki
Zakaria Acharki

Reputation: 67505

Problem :

You've a scope problem in the posted code where your variable i counldn't found inside the event handler, this variable is accessible just outside of it, to fix this you need to wrap the assignment of the event listener in a closure, like :

for (var i = 0; i < seats.length; i++) {
  (function(i) {
    seats[i].onclick = function() {
      seats[i].classList.toggle('selected');
    }
  })(i);
}


window.onload = function() {
  var seats = document.getElementsByClassName('seat');

  for (var i = 0; i < seats.length; i++) {
    (function(i) {
      seats[i].onclick = function() {
        console.log(i);
        seats[i].classList.toggle('selected');
      }
    })(i);
  }
}
td {
  width: 30px;
  height: 30px;
  background-color: red;
  color: white;
}

td.selected {
  background-color: black;
}
<!DOCTYPE html>
<html>

<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>JS Bin</title>
</head>

<body>
  <table>
    <tr>
      <td class="seat">1</td>
      <td class="seat">2</td>
      <td class="seat">3</td>
      <td class="seat">4</td>
      <td class="seat">5</td>
    </tr>
  </table>
</body>

</html>

My suggestion :

Separate you logic and use addEventListener() to attach the events, then use this to toggle your class, like the sample below shows.

window.onload = function() {
  var seats = document.getElementsByClassName('seat');

  var clickHandler = function() {
    this.classList.toggle('selected');
  };

  for (var i = 0; i < seats.length; i++) {
    seats[i].addEventListener('click', clickHandler, false);
  }
}
div.selected {
  background-color: green;
}
<div class="seat">Div 1</div>
<div class="seat">Div 2</div>
<div class="seat">Div 3</div>
<div class="seat">Div 4</div>

Upvotes: 3

Siggy
Siggy

Reputation: 334

The Problem with your code is that the variable i isn't defined per iteration it is defined in the function scope. That means that the for loop you have goes trough every element and assigns the onclick handler to each "seat" but after the last iteration the i variable will be set to an out-of-bounds index since the for loop does the i++ expression after each iteration. The problem is your onclick handler refers to the i variable, which is set to an out-of-bounds index, so that the seats[i] expression returns an undefined value. You can fix this by using the this statement inside the onclick handler or by using the first argument which is the click event. The click event contains a target which refers to the clicked EventTarget (this should be your "seat" element).

function handleSeatClick(ev) {
  var seat = ev.target;

  if(seat.classList.contains('selected')){
    seat.classList.remove('selected');
  } else {
    seat.classList.add('selected');
  }
}

window.onload = function() {
  var seats = document.getElementsByClassName('seat');

  for(var i = 0; i < seats.length; i++) {
    seats[i].addEventListener('click', handleSeatClick);
  }
}
td {
  width: 30px;
  height: 30px;
  background-color: red;
  color: white;}

td.selected {
  background-color: black;}
<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>JS Bin</title>
</head>
<body>
<table>
  <tr>
    <td class="seat">1</td>
    <td class="seat">2</td>
    <td class="seat">3</td>
    <td class="seat">4</td>
    <td class="seat">5</td>
  </tr>
</table>
</body>
</html>

Upvotes: 0

Harsh Patel
Harsh Patel

Reputation: 6830

Hope this will help.

window.onload = function() {
  var seats = document.getElementsByClassName('seat');
  for(var i = 0; i < seats.length; i++) {
    seats[i].onclick = function() {
      this.classList.toggle('selected'); //this will refer current object here and toggle is function 

    }
  }
}
td {
  width: 30px;
  height: 30px;
  background-color: red;
  color: white;}

td.selected {
  background-color: black;}
<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>JS Bin</title>
</head>
<body>
<table>
  <tr>
    <td class="seat">1</td>
    <td class="seat">2</td>
    <td class="seat">3</td>
    <td class="seat">4</td>
    <td class="seat">5</td>
  </tr>
</table>
</body>
</html>

Upvotes: -1

Related Questions