CODEWITHSUNDEEP
javascriptjqueryclick
Marko Stojanovic
Marko Stojanovic

Reputation: 23

Putting and removing background color on click from button

I have problem with this code because it works. Well, partially. My idea is to change background color of button on which user has clicked but data-line attributes increments only once and by that changing background color only once. (it enters in else and changes data-line value from 0 to 1 but my idea is to increment its value further and then by concluding whether value is odd or even to remove or add bg-danger class). All buttons (all with trashCan class) initially have data-line attribute with value zero and neutral background color. What is wrong with this code?

$(".trashCan").click(function(){
        let trashCan = $(this);
        let trash = parseInt(trashCan.data('line'))+1;
        trashCan.attr('data-line',trash);
        if(trash%2==0){
          trashCan.removeClass("bg-danger");
        }
        else {
          trashCan.addClass("bg-danger");
        }
      });

Upvotes: 2

Views: 1138

Answers (1)

Rory McCrossan
Rory McCrossan

Reputation: 337570

The issue is because you're accessing the data-line attribute with a mix of attr() and data(). The former reads from the DOM, while the latter reads from jQuery's internal cache. If you get the value with one of them you need to also set the value using the same method.

In the example below I've amended the logic to use data() only, which is the preferred choice where possible. If you absolutely have to update the DOM, then you would use attr() instead.

$(".trashCan").click(function() {
  let trashCan = $(this);
  let trash = parseInt(trashCan.data('line')) + 1;
  trashCan.data('line', trash);
  
  if (trash % 2 == 0) {
    trashCan.removeClass("bg-danger");
  } else {
    trashCan.addClass("bg-danger");
  }
});
.trashCan {
  background-color: #CCC;
  color: #000;
  border-radius: 5px;
  padding: 10px 30px;
  border: 0;
}   

.trashCan.bg-danger {
  background-color: #C00;
  color: #FFF
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<button class="trashCan" data-line="0">Trash</button>

However, it's worth noting that this logic is not necessary. It seems that your goal is simply to toggle the class on successive clicks, in which case just use toggleClass() and remove the data attribute entirely:

$(".trashCan").click(function() {
  $(this).toggleClass('bg-danger');
});
.trashCan {
  background-color: #CCC;
  color: #000;
  border-radius: 5px;
  padding: 10px 30px;
  border: 0;
}   

.trashCan.bg-danger {
  background-color: #C00;
  color: #FFF
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<button class="trashCan">Trash</button>

Upvotes: 2

Related Questions