0

I have a website that looks like below. When a text is clicked, the clicked text will get a class called large and the text size will change to 25pt. To see how I am assigning the event and the class large, please look at the js code. The problem I have is when the text "I like swimming" is clicked the large class gets added to the text "I like basketball". Why is this happening? Can anyone help me to fix this issue?

const arraySportsText = Array.from(document.getElementsByClassName('sportsText'));
for (index in arraySportsText) {
  arraySportsText[index].addEventListener("click", () => arraySportsText[index].classList.add('large'))
}
body {
  font-size: 15pt;
}

.large {
  font-size: 25pt;
}
<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Document</title>
  <link rel="stylesheet" href="styles/style.css">
</head>

<body>
  <h1>Sports</h1>


  <p class="sportsText">I like swimming</p>

  <p class="sportsText">I like basketball</p>

  <script src="scripts/hobby.js"></script>
</body>

</html>
Coder
  • 131
  • 1
  • 3
  • 10

1 Answers1

2

Your loop variable is not explicitly declared, so it gets an implicit var declaration as a Global variable.

Your use of the loop variable in the event handler creates a closure around index and so when those handlers run (later), they access the last value of that loop variable and so only the last element is affected.

Use let to declare your loop variable with block scope to solve the problem.

  • Addtionally, don't use .getElementsByClassName() and instead use .querySelectorAll().
  • And, both .getElementsByClassName() and .querySelectorAll() return collections which are array-like, so neither one of those needs Array.from() if you are going to use a counting loop to iterate it.
  • And, for/in loops should not be used on Arrays as they enumerate all the properties of the Array, not just the indexes. for/in should only be used on objects.
  • Lastly, the collection that .querySelectorAll() returns can be looped over with the Array.prototype.forEach() method, removing the need for the counting loop.

So, the reworked code really should be:

// Looping over the collection returned from .querySelectorAll()
// eliminates the need for a counting loop, which also removes
// the need to use that index in the event handler, which in turn,
// eliminates the closure problem you had in the first place.
document.querySelectorAll('.sportsText').forEach(function(element){
  element.addEventListener("click", () => element.classList.add('large'))
});
body   { font-size: 15pt; }
.large { font-size: 25pt; }
<h1>Sports</h1>
<p class="sportsText">I like swimming</p>
<p class="sportsText">I like basketball</p>
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71