My Javascript forEach loop combined with my event listener only works once

Question

I'm trying to make a simple game where the player clicks on the active button (the only one that's currently visible.) and once it's clicked, it will disappear and a new button will appear. From what I could build-out, it works but only once. When I click on the second button it is stationary. I currently have it so that when the button is active it has the .active class which contains various CSS styles applied to it. The inactive class has transparent styles applied to it. I figured that in my forEach loop, every time I click on the active button, it would produce another random number, add that number to my button Id to activate it, and render the old button inactive. What am I missing?

lodashRandNumber = () => {
  return _.random(1, 10);
}

let activeButton = document.querySelector(`#btn${lodashRandNumber()}`);
console.log(activeButton);
activeButton.className = "active"

let visibleButtons = document.querySelectorAll('.active');


visibleButtons.forEach(button => button.addEventListener('click', (event) => {
  if (button.classList.contains('active')) {
    button.classList.remove('active');
    button.classList.add('inactive');
    let newLodashRandNumber = () => {
      return _.random(1, 10);
    }
    let newActiveButton = document.querySelector(`#btn${newLodashRandNumber()}`);
    console.log(newActiveButton);
    newActiveButton.className = "active"
  }
}))

body {
  display: grid;
  grid-template-rows: 1fr;
  grid-template-columns: 1fr 4fr 1fr 2fr;
  background-color: #d9e4dd;
}

.inactive {
  background-color: transparent;
  opacity: 0.5;
  margin: 10px;
  padding: 50px;
  border-radius: 28px;
  color: transparent;
  border: transparent;
  font-size: 1em;
}

.active {
  background-color: #9aa39d;
  opacity: 0.5;
  margin: 83px;
  padding: 50px;
  border-radius: 28px;
  color: #433d3c;
  border: transparent;
  font-size: 2em;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.21/lodash.min.js"></script>
<button id="btn1" class="inactive">button 1</button>
<button id="btn2" class="inactive">button 2</button>
<button id="btn3" class="inactive">button 3</button>
<button id="btn4" class="inactive">button 4</button>
<button id="btn5" class="inactive">button 5</button>
<button id="btn6" class="inactive">button 6</button>
<button id="btn7" class="inactive">button 7</button>
<button id="btn8" class="inactive">button 8</button>
<button id="btn9" class="inactive">button 9</button>
<button id="btn10" class="inactive">button 10</button>

Use `event.target` instead of `button` in your click event...You are already passing the `(event)` — ikiK, Feb 26 '21 at 16:58

score 1 · Answer 1 · answered Feb 26 '21 at 16:56

1

Read about Event-Delegation here and here

Wrap all the buttons in some div, add event listener (click) to the parent element. Get a current button where event was triggered using event target.

answered Feb 26 '21 at 16:56

awHamer

81
3

This is really helpful. I'll go and study those links you added too. Thank you! – loganjanderson Feb 26 '21 at 17:06

score 0 · Accepted Answer · answered Feb 26 '21 at 17:00

The problem with your code is that the part which adds the event listener to the .active element is only run once, on the first active element. To make this work delegate the event to a higher element (I wrapped it all in a div below) and use that.

There's also no need to redefine the method which picks a random number from lodash.

You might need to full screen the snippet below to see it working easily as your buttons are all quite spread out.

lodashRandNumber = () => {
  return _.random(1, 10);
}

let activeButton = document.querySelector(`#btn${lodashRandNumber()}`);
console.log(activeButton);
activeButton.className = "active"

document.querySelector("#buttons").addEventListener("click", function(e) {
  const button = e.target;
  if (button.classList.contains('active')) {
    button.classList.remove('active');
    button.classList.add('inactive');
    let newActiveButton = document.querySelector(`#btn${lodashRandNumber()}`);
    console.log(newActiveButton);
    newActiveButton.className = "active"
  }
})

body {
  display: grid;
  grid-template-rows: 1fr;
  grid-template-columns: 1fr 4fr 1fr 2fr;
  background-color: #d9e4dd;
}

.inactive {
  background-color: transparent;
  opacity: 0.5;
  margin: 10px;
  padding: 50px;
  border-radius: 28px;
  color: transparent;
  border: transparent;
  font-size: 1em;
}

.active {
  background-color: #9aa39d;
  opacity: 0.5;
  margin: 83px;
  padding: 50px;
  border-radius: 28px;
  color: #433d3c;
  border: transparent;
  font-size: 2em;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.21/lodash.min.js"></script>
<div id="buttons">
  <button id="btn1" class="inactive">button 1</button>
  <button id="btn2" class="inactive">button 2</button>
  <button id="btn3" class="inactive">button 3</button>
  <button id="btn4" class="inactive">button 4</button>
  <button id="btn5" class="inactive">button 5</button>
  <button id="btn6" class="inactive">button 6</button>
  <button id="btn7" class="inactive">button 7</button>
  <button id="btn8" class="inactive">button 8</button>
  <button id="btn9" class="inactive">button 9</button>
  <button id="btn10" class="inactive">button 10</button>
</div>

This makes a lot of sense and was super helpful. It makes sense that I need to delegate it to a parent element. Thank you! — loganjanderson, Feb 26 '21 at 17:07

My Javascript forEach loop combined with my event listener only works once

2 Answers2