Best way to use removeEventListener

Hi there,

I’ve developed a simple word-guessing game but have run into issues when creating the “restart” element. On first play-through it works fine - adding/removing score, removing lives with incorrect guesses etc. but then when the user hits “Try again” at the end of the game, the lives, score and word population go haywire which I can only pin down to being an issue with the Event Listeners I have in place - I add them but have yet to remove them on game end.

Any thoughts on how to remove the event listeners on the buttons in the best way? There are multiple buttons - 2 for the difficulty setting, 1 for the try again scenario and then 26 for the letters.

Many thanks in advance!

Hey,

So I’ve been looking into a number of articles about removing the eventListener from my buttons (to then add them back in again once the game restarts, or as-needed) but found that you have to attach a handler as a second argument for it to work. However, in my case I simply want to remove the eventListener until it needs to be added again - is that possible?

What alternatives do I have to using this method and could it be done in a different way? Or is there just something fundamentally wrong with how I’ve set this up? I’m not used to coding events that have to reoccur, such as in the case of the game restart and whenever I have, I’ve simply fired functions inline (not best practice) instead of using eventListeners.

Many thanks!

I always think of event listeners as something you do when you either initialize the first time or tear down at the end. Not something you should be adding and removing during the course of the game. For instance, if you are starting the game for the first time, add the event listeners. However, you don’t need to remove them again to start the game over. You just need to reset the variables those event handlers work on.

Your problem is that when you restart the game, you are adding another set of event listeners onto the existing ones and never removing the original ones. You could use https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener to remove the listeners and readd them, but again I think it would be more efficient if you attach the listeners once and then work on data you change for a new game. Reset your counters etc. and have the event listeners work with those counters.

I hope you get what I am saying. :slight_smile:

1 Like

Thanks Martyr - that makes a lot of sense!
With a fresh mind on it I’ve now moved the difficulty-based and “try again” eventListeners outside of the main function (as you say, during the initialisation). What I’m struggling with now is how to do the same for the character buttons, since they are individual, and the logic to do the comparison between the button clicked and the “blanks” within the game is based on the individual button that is clicked.

What I’ve found through doing some console logs is that the lives decrease doubly-so on the second playthrough when the character buttons are clicked, presumably because it’s calling the function twice but also the lives readout doesn’t make sense, since the amount of lives left are conflicted as a result.

I’ve created a new pen with the updated location of the 3 eventListeners I talked about but I’m stumped on the character button ones… I’ll keep looking into it though but any pointers would be appreciated!

Many thanks!

Something else I’ve noticed is that on the second play through, whenever a character is clicked, the following error is thrown which is related to filling in the blank with the relevant letter from the selected button, yet it still correctly fills in the blank? This error gets thrown multiple times equal to how many playthroughs there have been (e.g. 3 playthroughs, 3 errors per letter clicked).

script.js:138 Uncaught TypeError: Cannot set properties of null (setting ‘innerText’)
gameBoardWord.querySelector(span.blank:nth-child(${characterNum})).innerText = char;

I’ve tried removing the eventListener by looping through each of the letterButtons when gameOver is called but the same errors still occur on the second and subsequent playthroughs.

    letterButtons.forEach((letterButton) => {
        letterButton.removeEventListener('click', turnOffLetters());
    });

Any ideas please?

Ok, I’ve found in the example which inspired this game idea (I was intending to write the game from scratch which is what I have done for the most-part), the buttons are dynamically created on each new game. This greatly differs from my code which uses a querySelectorAll on some HTML buttons, which loops through and adds an eventListener. In the other version of the game, the eventListeners are added to the dynamically-created buttons. The buttons are cleared from the DOM at the start of the game (to enable restarts), thus avoiding the issue of duplication on the click of each button.

See a few excerpts from this code below:

letterContainer.innerHTML = "";
.....
for (let i = 65; i < 91; i++) {
    let button = document.createElement("button");
    button.classList.add("letters");
    //Number to ASCII[A-Z]
    button.innerText = String.fromCharCode(i);
    //character button click
    button.addEventListener("click", () => {
        /// Logic for button click
    });
}

.......

letterContainer.append(button);


It seems a little heavy-going to take that approach but perhaps that’s the only way? Is there a way of using the setup that I have currently or will I just keep hitting the same issues?

Thanks in advance.

If you take these lines:

    gameDifficultyEasyButton.addEventListener('click', () => {
        gameLives = 5;
        startGame(gameLives);
    });

    gameDifficultyHardButton.addEventListener('click', () => {
        gameLives = 3;
        startGame(gameLives);
    });

OUT of the newGame function, and instead put them just before you call newGame at the bottom of the script… everything works.

Dont mess around with event listeners any more than you need to.

EDIT: Oh wait, no it doesnt, because your startGame function doesnt clear the state artifacts properly. Well, do that, and it’ll all work.

How do I make your code work?

#1: Pull this variable out to the global space. (The button listener needs it)
let selectedGameName = "";
EDIT: Also, remove the ‘let’ from line 93, because that line needs to play with the global variable, not its own local one.

#2: Pull all these bits of code out of their functions and call them once on load:

 letterButtons.forEach((currentBtn) => {
        currentBtn.addEventListener('click', () => {

            currentBtn.classList.add("disabled");

            let guess = currentBtn.innerText;
            let characterNum = 0;

            let matches = 0;
            let matchMade = false;

            for (let char of selectedGameName) {
                characterNum++;

                // console.log(char);
                /* If correct, the function feeds this back to the user
                by replacing any blanks with the letter. If no matches
                are made then the message "Try again" is fed back to
                the user */

                if (char.toUpperCase() === guess.toUpperCase()) {
                    // console.log(gameBoardWord.querySelector(`span.blank:nth-child(${characterNum})`));
                    gameBoardWord.querySelector(`span.blank:nth-child(${characterNum})`).innerText = char;
                    gameBoardWord.querySelector(`span.blank:nth-child(${characterNum})`).classList.add('letter');
                    gameBoardWord.querySelector(`span.blank:nth-child(${characterNum})`).classList.remove('blank');


                    matches++;
                    //console.log(matches);
                    matchMade = true;
                }
            }

            if (matchMade) {
                gameScore += 100 * matches;
            } else {
                gameScore -= 50;

                updateGameLives(gameLives);
                gameLives -= 1;

                if (gameLives == 0) {
                    console.log("GAME OVER");
                    gameStatus = "lose";
                    gameOver();
                } else {
                    console.log("gameLives left" + gameLives);
                }

            }

            matchMade = false;

            updateScore();
            checkBlanks(selectedGameName);
        });
  });
      gameEndMessageTryAgainButton.addEventListener('click', () => {
        newGame();
        console.log("Clicked Try Again");
    });
    gameDifficultyEasyButton.addEventListener('click', () => {
        gameLives = 5;
        startGame(gameLives);
    });

    gameDifficultyHardButton.addEventListener('click', () => {
        gameLives = 3;
        startGame(gameLives);
    });

(They’re all eventListener binds, you’ll notice.)

#3: Your game works.

1 Like

I know I’m late to the game, but could something like the following be useful?

gameButton.style.pointerEvents = "none"; // Disable Click on used button

Not to detract from m_hutleys comments. You can use event delegation and just add one listener to the letterboard wrapper, rather than for each key.

const doSomething = (key) => {
  console.log(key) // 'A', 'D', 'F' etc
}

const keyboard = document.querySelector('.letterboard')

keyboard.addEventListener('click', (event) => {
  const target = event.target
  
  if (target.matches('button')) {
    doSomething(target.textContent)
  }
})

Also just to point out there appears to be a closing </span> tag on your ‘E’ button rather than </button>

1 Like

Thanks so much everyone for your help!

I followed your steps in your next post and the game (as you say) was working again. Does your solution get around the clearing of the state artifacts point you made? Presumably what I was doing before was setting the eventListener and then attempting to “overwrite” the eventListener, thereby duplicating the action and causing the code to break… or am I way off-track?

I take it that I should only specify eventListeners once and then unbind them if I wish to change how they work? Again if I’m completely missing the point I apologise - I’m just curious to know what the best practice is.

Thanks for the suggestion - I did start using this and realised that I wanted a bit more control over how the disabled buttons looked so I opted for an add/remove of a class name instead.

Ah excellent suggestion - much cleaner! The fewer eventListeners being declared at the outset the quicker and more efficient my game will run - so by only applying the eventListener to the click of a button, it cleans things up. How is this cleared again, or does it not need to be? Are there any drawbacks with this approach?

Ah yes whoops - I was playing around with some layout options and obviously forgot to switch that around - thanks for the spot!

The artifacts in question were the listeners on the buttons playing with the life totals. Specifically, because the listeners were stacking up, it would subtract the wrong number of lives every time. When the listeners dont stack, the game works as intended.

No that’s roughly the point. Set an eventlistener if you need one; but rather than repeatedly removing them just for the purpose of readding them, set them once, or use event delegation.

If your code undoes something just to do that exact same thing again, your code has a logical flow error in it.

1 Like

Ok excellent - thank you so much for the explanations and help! :slight_smile: