CODEWITHSUNDEEP
javascript
Petja Makkonen
Petja Makkonen

Reputation: 11

Adding multiple eventlisteners at once

So, I'm building a game which consists of openable pieces. I need an event listener for each and every one of them. There are 49 pieces. This is how I plan to accomplish this: the ID's are box and inventory, + 1 to 49. The for loops are inside a function which fires on page load.

    for(var i = 1; i < 50; i++){
    document.getElementById("box" + "" + i).addEventListener('click', function(){
        addImgBox(i);
    });
}

for(var j = 1; j < 4; j++){
    document.getElementById("inventory" + "" + j).addEventListener('click', function(){
        addImgInv(j);
    });
}

function addImgInv(j){
    console.log(j);
    $('#inventory' + '' + j).prepend('<img class="img" src=' +     hero.inventory[j - 1].picture  + ' />');
    $( "#theImg" + "" + j ).addClass( "img" );
}

function addImgBox(i){
   console.log(i);
   $('#box' + '' + i).prepend('<img class="img" src=' + levelToPlay[i - 1].picture + ' />');
   $( "#theImg" + "" + i ).addClass( "img" );

}

The problem is that whichever box I click, it always logs 50 for i or 4 for j. Clearly the attaching of eventlisteners ain't working. The second thing I'm wondering is: even though I tell for loop to stop before 50, it still gets up to that. What am I missing or what should I do differently?

p.s. The syntax should be right. Had some trouble pasting the code.

Upvotes: 1

Views: 82

Answers (3)

Azamantes
Azamantes

Reputation: 1461

If something doesn't work as intended, let me know.

window.addEventListener('click', function(event) {
    const target = event.target;
    const valid = /^(box|inventory)([1-9]|[1-4][0-9])$/; // 1 - 49
    const id = target.id;

    if(!valid.test(id)) { // invalid ID
        return;
    }

    const pattern = "<img class='img' src='#'>";
    let number, string;

    if(/box/.test(id)) {
        number = +id.match(/[0-9]+/)[0];
        string = pattern.replace(/#/, levelToPlay[number - 1].picture);
        $('#box' + number).prepend(string);
    } else {
        number = +id.match(/[0-9]/)[0];
        string = pattern.replace(/#/, hero.inventory[number - 1].picture;
        $('#inventory' + number).prepend(string);
    }

    $('#theImg' + number).addClass('img');
});

Upvotes: 0

Nafiul Islam
Nafiul Islam

Reputation: 1220

Your event callbacks are getting fired after all these initializations have been done meaning i=50 & j=4. To get rid of these you may try to send the element that is clicked and from that dom find id/index and perform your action. Something like this: (I haven't tested)

document.getElementById("box" + "" + i).addEventListener('click', function() {
    addImgBox($(this));
});

function addImgBox(el) {
    var index = el.attr('id');
    el.prepend('<img class="img" src=' + levelToPlay[index - 1].picture + ' />');
    $("#theImg" + "" + index).addClass("img");
}

Upvotes: 0

Mattan Bitner
Mattan Bitner

Reputation: 1533

This is a classic closure issue, the scopes of i or j are the same for all events so the counter reaches the end of the loop and that value is the value all get. A solution for example is with a self execution func that creates a scope for each event handler:

    document.getElementById("inventory" + "" + j).addEventListener('click', 
    function(index){
       return   function(){ addImgInv(index);
    }(j)
     }

);

Upvotes: 1

Related Questions