0

I have a probleme with my code. Now if I click the first element, all elements will become red, if I click second time they will become green. I would like have two independent events for each element with class fa-heart. I will explain better: If I click the first time the first element DOM, only this element will become red, and if I click it second time, it will become green, and so for all the others. I apologize if my request is not clear. Thank you so much for your help.

<script src="https://kit.fontawesome.com/a1d70a0cda.js"></script>
<a onclick="change()"><i class="fas fa-heart"></i></a>
<a onclick="change()"><i class="fas fa-heart"></i></a>
<a onclick="change()"><i class="fas fa-heart"></i></a>

<script>
 function change(){
      var a = document.querySelectorAll('.fa-heart');
      var qty = a.length;
      var i = 0;

      for(i; i<qty; i++){
         if(a[i].style.color !== 'red'){
             a[i].style.color = 'red';
         }else{
             a[i].style.color='green';
           }
     }
}
</script>

2 Answers2

1

Add an individual listener to each <i> instead, and in the listener, check the current .style of the clicked element to figure out what to assign next:

document.querySelectorAll('.fa-heart').forEach((i) => {
  i.addEventListener('click', () => {
    i.style.color = i.style.color !== 'red'
      ? 'red'
      : 'green';
  });
});
<script src="https://kit.fontawesome.com/a1d70a0cda.js"></script>
<a><i class="fas fa-heart"></i></a>
<a><i class="fas fa-heart"></i></a>
<a><i class="fas fa-heart"></i></a>

Or, with event delegation:

document.addEventListener('click', (e) => {
  if (!e.target.matches('.fa-heart')) {
    return;
  }
  e.target.style.color = e.target.style.color !== 'red'
    ? 'red'
    : 'green';
});

console.log('start');
setTimeout(() => {
  console.log('adding dynamic elements');
  document.body.innerHTML += `<a><i class="fas fa-heart"></i></a>
<a><i class="fas fa-heart"></i></a>
<a><i class="fas fa-heart"></i></a>`;
}, 1000);
<script src="https://kit.fontawesome.com/a1d70a0cda.js"></script>

If you must use inline handlers (which you shouldn't), pass the this (the clicked element) to the listener:

function change(i) {
  i.style.color = i.style.color !== 'red'
    ? 'red'
    : 'green';
}
<script src="https://kit.fontawesome.com/a1d70a0cda.js"></script>
<a onclick="change(this)"><i class="fas fa-heart"></i></a>
<a onclick="change(this)"><i class="fas fa-heart"></i></a>
<a onclick="change(this)"><i class="fas fa-heart"></i></a>
CertainPerformance
  • 260,466
  • 31
  • 181
  • 209
  • Thank for your answer but I have to use the onclick inline like in my example! – ulisse0744 Jun 02 '19 at 01:14
  • You really shouldn't - it's widely considered to be quite bad practice. Why can't you attach the event properly using Javascript? – CertainPerformance Jun 02 '19 at 01:15
  • I suppose you can pass `this` in the inline handler instead, but it's not a good idea, there's certainly a better way – CertainPerformance Jun 02 '19 at 01:25
  • I am trying to edit a wishlist plugin on Joomla, this code has got onclick inline because it is Ajax request. Is not bad idea attach two click event in the same code (onclick and click)? Sorry if my question is silly – ulisse0744 Jun 02 '19 at 01:27
  • If the elements are being added dynamically, it would be better to use event delegation, see https://stackoverflow.com/questions/203198/event-binding-on-dynamically-created-elements - example given in second snippet in my answer – CertainPerformance Jun 02 '19 at 01:28
0

Here you go. This will change the clicked one to green and others to red.

function change(clicked) {
  document.querySelectorAll('a').forEach(el => el.setAttribute("style", "color:red"));
  clicked.style.cssText ="color:green;";
}
<script src="https://kit.fontawesome.com/a1d70a0cda.js"></script>
<a onclick="change(this)"><i class="fas fa-heart"></i></a>
<a onclick="change(this)"><i class="fas fa-heart"></i></a>
<a onclick="change(this)"><i class="fas fa-heart"></i></a>
Randy Casburn
  • 11,404
  • 1
  • 12
  • 26