CODEWITHSUNDEEP
javascriptoopeventsecmascript-6eventhandler
McPhelpsius
McPhelpsius

Reputation: 253

RemoveEventListener not removing event

The code below is supposed to add an event listener for each playing card in a players hand while it's their turn, and then remove the events while it's a different player's turn.

It isn't working. That player's cards remain clickable once the event is initially set on their first turn.

takeTurn ( playerIndex ) {
    console.log(this.name);
    let handCards = document.getElementById(this.name).querySelector('.handCards');
    let theCards = handCards.querySelectorAll('.handCards .card');
    let that = this;
    for ( let h = 0; h < theCards.length; h++ ) {
        theCards[h].addEventListener("click", function onPlayCard (){
            let theseCards = handCards.querySelectorAll('.handCards .card');
            let discarded = that.playCard(theCards[h], theseCards, handCards);

            that.gameInstance.discardPile.push(discarded);
            console.log(that.gameInstance.discardPile);
            for ( let e = 0; e < theseCards.length; e++) {
                theseCards[e].removeEventListener("click", onPlayCard, false);
            }
            console.log(that.name + 'is done');
            that.gameInstance.nextPlayer( playerIndex );
        }, false);
    }
}

I tried some of the ideas from here and here, but none of them quite solved the problem.

Any helps are appreciated. I might pull my hair out soon. I thought I knew this stuff.

Upvotes: 0

Views: 207

Answers (1)

user1106925
user1106925

Reputation:

The problem:

The problem is that you're adding a unique function instance to each card, but then trying to remove them all using only the function instance of the clicked card. This will only remove the one card that actually matches the instance added to the clicked card.

So element 0 gets handler 0, element 1 gets handler 1, element 2 gets handler 2, and so on...

When an element is clicked, say element 2, you then iterate all the elements to remove their handler, but you're providing the handler for the one that was clicked, element 2. The handler 2 is only useful for removing the element 2, and not the others.

The solution:

I won't actually give a solution given your current code, except to say that it would involve maintaining an array of the handlers parallel with the elements to which they were bound. It would work just fine, but I think a redesign of the app may be better.

A different approach:

IMO, repeated binding and unbinding is an indication that a different approach needs to be taken in designing your app.

You could take an object oriented approach, and represent each type of item in your app as a "class" (constructor function). So you'd have Game that manages the game as a whole, Card that represents each instance of a playing card, and Player to represent each player.

Create a Card instance for each card. Perhaps other special ones like Dealer or Deck and Discard for a discard pile. The Card could have properties representing its value and suit, and an .owner property that references the current holder of that card. The .owner could be a Player, the Discard pile or the Deck for example.

The Game has reference to all these objects, so that it can coordinate them. It deals the cards by changing the .owner from Deck to a certain Player. When a Player discards a Card, the card's .owner changes to Discard.

So basically each object maintains its own "state".

Each item could also reference its DOM element, to which you bind handlers. The handler would have a reference to the DOM element via this, but it also would need to reference the data for its associated entity. Most people would create a separate handler for each object, and use a closure to reference the data. I personally would use a little known, yet extremely useful feature called the EventListener Interface. I'll leave it to you to research this topic.

How exactly you set it up is up to you. Should the Game assign an .owner to a Card? Or the Dealer? Or should the Card be passed to an owner (like a Player) and have it assign the .owner property? Or should the Card have a method that receives its new owner and assigns it to its own .owner property? Up to you.

Hope this gives you some ideas anyway.

Upvotes: 1

Related Questions