Loop through querySelectorAll

Question

I've got a function to close a modal when clicking anywhere outside of it. This is the code JS:

var modal = document.querySelectorAll('.quickview-modal')
// When the user clicks anywhere outside of the modal, close it
modal.forEach(function() {
window.onclick = function(event) {
  if (event.target == modal) {
    $('.item').removeClass('modal-carousel-fix');
    $('.modal-backdrop').css('display','none')
    $('.quickview-modal').css('display','none');
    $(this).parent().closest('.carousel-inner').css('overflow', 'hidden');
  }
 }
});

HTML:

<div class="modal quickview-modal" id="quickViewModal" tabindex="-1" role="dialog" aria-labelledby="exampleModalLabel" aria-hidden="true">
  <div class="modal-dialog" role="document">
    <div class="modal-content">
      <div class="modal-body">
        <p>Modal Content</p>
      </div>
    </div>
  </div>
</div>

But this doesn't seem to work. If I change it to

var modal = document.querySelector('.quickview-modal')
// When the user clicks anywhere outside of the modal, close it
window.onclick = function(event) {
  if (event.target == modal) {
    $('.item').removeClass('modal-carousel-fix');
    $('.modal-backdrop').css('display','none')
    $('.quickview-modal').css('display','none');
    $(this).parent().closest('.carousel-inner').css('overflow', 'hidden');
  }
 }

It works on the first .quickview-modal object, so I'm assuming something is wrong with the loop. Any ideas how to fix this?

Answer 1

As @SebastianSpeitel says in a comment above

document.querySelectorAll doesn't return a real array

That's true. It returns NodeList or HTMLCollection, but you can still map it with .forEach, so that's not the real issue.

The @Luca's comment provides a solution.

you are re-assigning window.onclick over and over, and you are comparing an HTMLElement (event.target) to a HTMLCollection

So to make easier for the author of this question I wrote the following code:

// modal is a list of elements
var modals = document.querySelectorAll('.quickview-modal')
modals.forEach(function(modal) {
  // modal is the current element
  modal.onclick = function(e) {
    $('.item').removeClass('modal-carousel-fix');
    $('.modal-backdrop').css('display','none')
    $('.quickview-modal').css('display','none');
    $(this).parent().closest('.carousel-inner').css('overflow', 'hidden');
  }
});

But using addEventListener is definitely better practise. So consider using it like this: modal.addEventListener("click", function callback(e) {}), where click can be replaced with other events (hover, keypress, ect..)

Also even better JQuery solution will be to use $(document).on('click', '.YOURCLASS', function)

$(document).on('click', '.quickview-modal', function (e) {
    // The same code as the onclick above, OR
    $(this).css('display','none');
    $(this).parent().closest('.carousel-inner').css('overflow','hidden');
});