With JS, open and close buttons work, but escape always tries to close the last modal

Question

I've got this working with my buttons, but I can't figure out how to make the escape key work with the correct modal. It always 'closes' the last modal, no matter how many I have.

It's all within my for loop, so I don't understand why thisModal tracks to the correct one for the buttons, but not for the document.onkeydown function.

Also, no comments about using jQuery please :).

const modalToggle = document.querySelectorAll(".button"),
  modal = document.querySelectorAll(".modal"),
  closeButton = document.querySelectorAll(".close");

if (modalToggle) {

  for (i = 0; i < modalToggle.length; i++) {
    let thisToggle = modalToggle[i];
    let thisModal = modal[i];
    let thisClose = closeButton[i];

    thisToggle.addEventListener("click", () => openModal(thisModal));
    thisClose.addEventListener("click", () => closeModal(thisModal));
    document.onkeydown = (e) => {
      if (e.keyCode == 27) {
        console.log(thisModal); // Is always the LAST modal... why?
        closeModal(thisModal);

      }
    }
  };
}

function openModal(el) {
  el.style.display = "block";
}

function closeModal(el) {
  el.style.display = "none";
}

.modal {
  display: none;
  background: lightgray;
  border: 1px solid black;
  width: 50%;
  height: 100px;
}

<button class="button">One</button>
<button class="button">Two</button>
<button class="button">Three</button>

<div class="modal">ONE
  <button class="close">Close</button>
</div>

<div class="modal">TWO
  <button class="close">Close</button>
</div>

<div class="modal">Three
  <button class="close">Close</button>
</div>

You've fallen into a Javascript closure pitfall... you could try adding the keydown event listener and passing the reference to `thisModal` the same way as your first two event listeners: `document.addEventListener('keydown', (e) => closeModal(closeModal, e));` and then in your closeModal function, check for the escape key event. Or make a different function to check the event and then pass the element along to your close function - the point here is to not have this anonymous function inside your for loop which is causing the issue. — skyline3000, Sep 07 '17 at 15:02
Thanks @skyline3000 I read about it in the link to a possible duplicate, and didn't really get what was going on still, for whatever reason what you said really clicked! Cheers. — Kevin Powell, Sep 08 '17 at 14:11
This is one of the more difficult concepts to understand in JS, so don't fret about not getting it at first. What's happening is that the variable `i` is bound to its entire containing function as variables have function-level scope in JS. If you were to continue using the variable `i` after assigning your event listeners, it would still be `3`. The same goes for your other vars like `closeModal`. So the for loop sets up all of your event listeners, but when you actually call the function to be run (by doing a keypress) it looks for the variable `closeModal`, which is now the third one. — skyline3000, Sep 08 '17 at 14:45
And note that this is different than your first two event listeners which pass the reference to the function and variable in the listener, rather than using the variable through an anonymous function -- which is actually using the variable from the outside scope (a.k.a. a closure). — skyline3000, Sep 08 '17 at 14:48

score 1 · Answer 1 · answered Sep 07 '17 at 15:01

It was because each run of the for loop was overriding the previous bind. Try this, pressing esc closes all modals

const modalToggle = document.querySelectorAll(".button"),
  modal = document.querySelectorAll(".modal"),
  closeButton = document.querySelectorAll(".close");

if (modalToggle) {

  for (i = 0; i < modalToggle.length; i++) {
    let thisToggle = modalToggle[i];
    let thisModal = modal[i];
    let thisClose = closeButton[i];

    thisToggle.addEventListener("click", () => openModal(thisModal));
    thisClose.addEventListener("click", () => closeModal(thisModal));
  };
  document.onkeydown = (e) => {
      if (e.keyCode == 27) {
          for (i = 0; i < modalToggle.length; i++) {
               let thisToggle = modalToggle[i];
              let thisModal = modal[i];
              let thisClose = closeButton[i];
              console.log(thisModal); // Is always the LAST modal... why?
              closeModal(thisModal);
          };


      }
    }
}

function openModal(el) {
  el.style.display = "block";
}

function closeModal(el) {
  el.style.display = "none";
}

.modal {
  display: none;
  background: lightgray;
  border: 1px solid black;
  width: 50%;
  height: 100px;
}

<button class="button">One</button>
<button class="button">Two</button>
<button class="button">Three</button>

<div class="modal">ONE
  <button class="close">Close</button>
</div>

<div class="modal">TWO
  <button class="close">Close</button>
</div>

<div class="modal">Three
  <button class="close">Close</button>
</div>

That works, so thanks! Through @skyline3000 comment on my original post, I've come up with a solution that let's me keep it all within the same for loop though, which I prefer. — Kevin Powell, Sep 08 '17 at 14:10

With JS, open and close buttons work, but escape always tries to close the last modal

1 Answers1