CODEWITHSUNDEEP
javascriptfor-loopcallback
Kelly Cecil
Kelly Cecil

Reputation: 5

Callback Function does not work with for Loop

Why doesn't the callButtonTwo() function work on all buttons? Only button1 reacts to it?

document.querySelector("h1").addEventListener("click", function() {
    alert("Working!");
})

var totalButton = document.querySelectorAll(".testSecond").length;
for (var i = 0; i < totalButton; i++) {
    document.querySelectorAll(".testSecond")[i].addEventListener("click", function() {
        var buttonNow = this.innerHTML;
        callButtonOne(buttonNow);
        callButtonTwo(buttonNow);
    });
}

function callButtonOne() {
    alert("I got clicked!");
};

function callButtonTwo() {
    var changeRed = document.querySelector(".testSecond");
    changeRed.classList.add("red");
    setTimeout(function() {
        changeRed.classList.remove("red");
    }, 300);
};
.red {
  background-color: red;
}
<h1 class="testFirst">Hello World!</h1>
<button class="testSecond button1" type="button" name="button">button1</button>
<button class="testSecond button2" type="button" name="button">button2</button>
<button class="testSecond button3" type="button" name="button">button3</button>
<button class="testSecond button4" type="button" name="button">button4</button>
<button class="testSecond button5" type="button" name="button">button5</button>

Upvotes: 0

Views: 107

Answers (3)

Sascha Gschwind
Sascha Gschwind

Reputation: 61

The problem is, that your querySelector in callButtonTwo just finds the first button because all of them have the same class .testSecond.

One possible solution is to add the reference of the button which should be colored red to the callButtonTwo function and use the reference to add the class name.

It would look like this:

function callButtonTwo(button) {
  button.classList.add("red");
  setTimeout(function() {
    button.classList.remove("red");
  }, 300);
};

And you would call it like this: callButtonTwo(this);

Here's a working example based on the code you provided.

Upvotes: 0

nickfla1
nickfla1

Reputation: 185

Inside callButtonTwo you're querying .testSecond again which will always return the first element in the page with that class. If you want to handle each button differently you should pass the button element as a parameter to callButtonTwo, like so:

var totalButton = document.querySelectorAll(".testSecond").length;
for (var i = 0; i < totalButton; i++) {
document.querySelectorAll(".testSecond")[i].addEventListener("click", function() {
var buttonNow = this.innerHTML;
callButtonOne(buttonNow);
callButtonTwo(this); // 'this' in this case is the clicked button element
});
}

[...]

function callButtonTwo(button) {
button.classList.add("red");
setTimeout(function() {
button.classList.remove("red");
}, 300);
};

I would also consider to change the way you're iterating your elements in order to query the DOM less frequently:

// 'querySelectorAll' returns an array which can be directly iterated using its method 'forEach'
document.querySelectorAll(".testSecond").forEach(function(button) {
  button.addEventListener("click", function() {
    var buttonNow = this.innerHTML;
    callButtonOne(buttonNow);
    callButtonTwo(this);
  });
});

Upvotes: 0

Quentin
Quentin

Reputation: 943567

While, inside your click handler you have var buttonNow = this.innerHTML; which operates on that button, when you call callButtonTwo you say var changeRed = document.querySelector(".testSecond"); which operates on the first button (no matter which button you click).

You need to tell it which button you are dealing with (e.g. by passing this as an argument)

Upvotes: 1

Related Questions