0

var toggleButton = document.getElementsByClassName('toggle-button')
for (var i = 0; i < toggleButton.length; i++) {
  toggleButton[i].addEventListener('click', function () {
      toggleButton[i].parentElement.parentElement.children[2].style.background = "red";
  });
}
<div>
  <div>
    <button class="toggle-button">view</button>
  </div>

  <div>content</div>

  <div>the div which I want to apply changes on</div>
</div>

What's wrong with this code? When I run it it gives me this error "Uncaught TypeError: Cannot read property 'parentElement' of undefined at HTMLButtonElement."

*I don't want to apply the CSS changes on a class I need it to be applied on this specific div using DOM.

Mike 'Pomax' Kamermans
  • 49,297
  • 16
  • 112
  • 153
  • Never touch the `style` attribute, and more broadly: if you want to do something on the JS side, use the JS APIs for that thing, rather than trying to change the "markup" for DOM elements. In this case, add/toggle/remove CSS classes instead, using the [element.classList](https://developer.mozilla.org/en-US/docs/Web/API/Element/classList) namespace, so: `toggleButtons.forEach(e => e.addEventListener("click", _event => e.classList.toggle("active")))` with a CSS class `.active { background-color: red; }`. – Mike 'Pomax' Kamermans Jan 21 '21 at 18:39
  • It gave this error "toggleButton.forEach is not a function" –  Jan 21 '21 at 18:49
  • I did it like that -- var toggleButton = document.getElementsByClassName('toggle-button') toggleButton.forEach(e => e.addEventListener("click", _event => e.parentElement.parentElement.children[2].classList.toggle("active"))) -- –  Jan 21 '21 at 18:50
  • Actually the mistake here is the `i` in click handler: after loop it will be `1` (or equal to togleButton.length), and there is no toggleButton[1]. Using `this` would make more sense here. – myf Jan 21 '21 at 19:02
  • Sorry, [getElementsByClassName](https://developer.mozilla.org/en-US/docs/Web/API/Document/getElementsByClassName) returns an HTMLCollection, not a NodeList, so you need to wrap it in `Array.from(...)` to get `.forEach`, which solves the problem of an `i` being the wrong value. – Mike 'Pomax' Kamermans Jan 21 '21 at 19:11
  • Does this answer your question? [JavaScript closure inside loops – simple practical example](https://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) – Heretic Monkey Jan 21 '21 at 19:33

3 Answers3

1

If it's just one button can't you just do this:

const toggleButtons = document.getElementsByClassName('toggle-button')

toggleButtons[0].addEventListener('click', () => {
    toggleButtons[0].parentElement.parentElement.children[2].style.background = "red";
});

Or if you really want to use a for loop for some reason this works just fine for me:

var toggleButtons = document.getElementsByClassName('toggle-button')

for(let i=0; i<toggleButtons.length; i++) {
  toggleButtons[i].addEventListener('click', function () {
    toggleButtons[i].parentElement.parentElement.children[2].style.background = "red";
  });
}

Better yet, make a CSS rule for a class called red and use this code to toggle that class on and off on the selected div:

var toggleButtons = document.getElementsByClassName('toggle-button')

for(let i=0; i<toggleButtons.length; i++) {
  toggleButtons[i].addEventListener('click', function () {
    toggleButtons[i].parentElement.parentElement.children[2].classList.toggle('red')
  });
}

CSS:

.red {
  background: red
}
Daisho Arch
  • 520
  • 2
  • 16
  • `const toggleButtons = Array.from(document.getElementsByClassName('toggle-button'))` and then `toggleButtons.forEach(e => ...)` gives you a much more robust pattern. You should not need to explicitly work with the iteration counter. – Mike 'Pomax' Kamermans Jan 21 '21 at 19:11
0

Try using this that way each event listener is referring to itself and backgroundColor

var toggleButton = document.getElementsByClassName('toggle-button')
for (var i = 0; i < toggleButton.length; i++) {
    toggleButton[i].addEventListener('click', function () {  this.parentElement.parentElement.children[2].style.backgroundColor = "red";
    });
}
<div>

    <div>
        <button class="toggle-button">view</button>
    </div>

    <div>content</div>

    <div>the div which I want to apply changes on</div>
    
</div>

<div>

    <div>
        <button class="toggle-button">view</button>
    </div>

    <div>content</div>

    <div>the div which I want to apply changes on</div>
    
</div>
0

Change getElementsByClassName to querySelectorAll('.toggle-button') as below

const toggleButton = document.querySelectorAll('.toggle-button')
for (let i = 0; i < toggleButton.length; i++) {
  toggleButton[i].addEventListener('click', function () {
      toggleButton[i].parentElement.parentElement.children[2].style.background = "red";
  });
}
<div>
  <div>
    <button class="toggle-button">view</button>
  </div>

  <div>content</div>

  <div>the div which I want to apply changes on</div>
</div>
mah111
  • 146
  • 8
  • N.B. actual fix here is not the DOM nodes retrieval, but subtle change of `var i` to `let i` in the loop, so the scope and actual value in click handler. – myf Jan 21 '21 at 19:33