How can I make this click counter work properly?

Question

I'm making a Marvel API project. I added a Like button so it could count the likes, but everytime I click any like button the only counter adding numbers is the first one. Can anyone tell me how I do this properly?

The problem is because the ID is being made by a for, so how could I solve this?

This is my JS code (on the js file of my project) :

success: function(data)
{
    footer.innerHTML = data.attributionHTML;
    var string = "";
    string += "<div class='row'>";

    for (var i = 0; i < data.data.results.length; i++)
    {
        var element = data.data.results[i];                   
        string += '<div class="col-md-3" align="center">';                 
        string += "<img src='" + element.thumbnail.path + "/portrait_fantastic." + element.thumbnail.extension + "'/>";                       
        string += '<button class="btn btn-success" onClick="testo()"><i class="fas fa-thumbs-up"></i> | <a id="likes">0</a></button>';
        string += "<h3>" + element.title + "</h3>";                                                  
        string += "</div>";

        if ((i + 1) % 4 == 0) 
        {
            string += "</div>";
            string += "<div class='row'>";
        }
    }

    marvelContainer.innerHTML = string;
}

And this is my onclick function (It is on my html file because it wont work on my js file)

<script>
var likes=0;
function testo()
{
    likes += 1;
    document.getElementById("likes").innerHTML = likes;
}
</script>

IDs are supposed to be unique, looks like you’ve set all the elements with the same ID, am I wrong? — Davide Vitali, Mar 29 '19 at 19:59
As @DavideVitali has mentioned, you need to have unique ids. I also see another problem with your code. Looks like your "likes" variable always going to be 1, since you start it from 0 every time user hits like. What you might want to do is that, get the "likes" counter value for clicked element, then parse it to Int, then increment it by 1, finally write it back to the element. — Caner Akdeniz, Mar 29 '19 at 20:00
Possible duplicate of [Does ID have to be unique in the whole page?](https://stackoverflow.com/questions/9454645/does-id-have-to-be-unique-in-the-whole-page) — Taplar, Mar 29 '19 at 20:01
The duplicate ids is one issue. The single global variable containing the like count is a second one, not looking an any of the rest of your logic. — Taplar, Mar 29 '19 at 20:02
@CanerAkdeniz a data attribute would be more appropriate to avoid the requirement to parse it every time. — Taplar, Mar 29 '19 at 20:03
As you people can see on the code, the buttons are generated by a FOR so the IDs are going to be the same. How can I solve this? — Luis Fernando Badel Méndez, Mar 29 '19 at 20:04
The button you are clicking belongs to a `class="row"`. This a prime canidate to change the ids on the elements to classes, and perform contextual lookups. — Taplar, Mar 29 '19 at 20:05
@LuisFernandoBadelMéndez as suggested in an answer, use data-* attribute — Davide Vitali, Mar 29 '19 at 20:05

Adriano Marra · Answer 1 · 2019-03-29T21:15:59.383

That is because all your buttons are being generated with the same id="likes" and then you are changing the HTML with document.getElementById("likes").innerHTML = likes; for your code to work properly you will need to use a different approach, maybe adding a data-* attribute to your buttons and then change the likes by the data-* atribute using .getAttribute('data-id-something').innerHTML instead of document.getElementById("likes").innerHTML. Or even better in this case you can give the buttons a class name and handle it with: document.getElementsByClassName("like-btn")

You can check the last option in this example:

  var init = function(data){
                var string ="";
                string += "<div class='row'>";
                for(var i = 0; i<4; i++)
                {
                    // var element = data.data.results[i];                   
                    string += '<div class="col-md-3" align="center">';                 
                    string += "<img src='/portrait_fantastic.jgeg'/>";                       
                    string += '<button class="btn btn-success" onClick="testo('+i+')"><i class="fas fa-thumbs-up"></i> | <span class="like-btn">0</span></button>';
                    string += "<h3>Element title</h3>";                                                  
                    string += "</div>";                   
                    if((i+1) % 4 ==0) 
                    {
                        string += "</div>";
                        string += "<div class='row'>";
                    }
                }
                document.getElementById("marvelContainer").innerHTML = string;
            }
            init();

<script>
var likes=0;
function testo(id) {      
        var btn = document.getElementsByClassName("like-btn");
        likes = parseFloat(btn[id].innerHTML);
        likes += 1;
        btn[id].innerHTML = likes;
    }
</script>

<div id="marvelContainer"></div>

I hope it will help you...

@Adriano hey Im just learning Javascript, can you show me how the code should look? thanks for you time — Luis Fernando Badel Méndez, Mar 29 '19 at 20:07

score 1 · Accepted Answer · edited Jun 20 '20 at 09:12

Gave the buttons a class, as that is what is going to be clicked.
Changed the link element to a span. Having a link in a button doesn't make much sense, as you can't "not" click the button and click the link.
Removed the inline onclick for the link and added an event listener logically on all the buttons.
The click logic finds the nested span in the button
- It takes the data attribute on the span, turns it into an integer, and increments it
- It then updates the data attribute value for the next click
- And finally it updates the visible text that the user can see

EDIT

Changed it to bind the click event listener on the span as well and stop propagation on the click event. Actually clicking the span was causing the click event to register for the span, and not the button.

// fake out some data for the element generation
var data = { data: {
  results: [
    { thumbnail: { path: '/path1', extension: 'jpg', title: 'Element1' } }
    ,{ thumbnail: { path: '/path2', extension: 'png', title: 'Element2' } }
    ,{ thumbnail: { path: '/path3', extension: 'gif', title: 'Element3' } }
  ]
} };
// fake out the container the elements are built to
var marvelContainer = document.querySelector('#container');

var string = "<div class='row'>";

for (var i = 0; i < data.data.results.length; i++) {
  var element = data.data.results[i];
  string += '<div class="col-md-3" align="center">';
  string += "<img src='" + element.thumbnail.path + "/portrait_fantastic." + element.thumbnail.extension + "'/>";
  // put a class on the button
  // also removed the id and inline onclick
  // change the id on the link to a class
  // also initialized the data-likes on the link to zero
  string += '<button class="btn btn-success likes-button"><i class="fas fa-thumbs-up"></i> | <span class="likes" data-likes="0">0</span></button>';
  string += "<h3>" + element.title + "</h3>";
  string += "</div>";

  if ((i + 1) % 4 == 0) {
    string += "</div>";
    string += "<div class='row'>";
  }
}

marvelContainer.innerHTML = string;

document.querySelectorAll('.likes-button, .likes').forEach(function(likeButton){
  likeButton.addEventListener('click', incrementLikes);
});

function incrementLikes (e) {
  e.stopPropagation();
  
  // find the inner likes element of the button
  var likes = e.target.querySelector('.likes') || e.target;

  // increment the likes
  var incrementedLikes = parseInt(likes.dataset.likes) + 1;

  // update the data attribute for next click, and update the text
  likes.dataset.likes = incrementedLikes.toString();
  likes.innerText = incrementedLikes;
}

<div id="container">
</div>

How can I make this click counter work properly?

2 Answers2

EDIT