Modal not closing anymore

Question

I do not code regularly and it has been a while. I'm currently building the website for my new company where im borrowing an idea of a competition website with modals. The modal was working perfectly, open and closing, I made more, and now it doesn't close anymore, opening goes well closing no? I'm sure its a stupid mistake but please bare with me,how to get it closed?`

// Get the modal
var modal = document.getElementsByClassName('modal');

// Get the button that opens the modal
var btn = document.getElementsByClassName("course-link");


// Get the  element that closes the modal
var span = document.getElementsByClassName("close");

// When the user clicks the button, open the modal 
btn[0].onclick = function() {
    modal[0].style.display = "block";
}

btn[1].onclick = function() {
    modal[1].style.display = "block";
}

btn[2].onclick = function() {
    modal[2].style.display = "block";
}

btn[3].onclick = function() {
    modal[3].style.display = "block";
}

btn[4].onclick = function() {
    modal[4].style.display = "block";
}

btn[5].onclick = function() {
    modal[5].style.display = "block";
}

btn[6].onclick = function() {
    modal[6].style.display = "block";
}

btn[7].onclick = function() {
    modal[7].style.display = "block";
}

btn[8].onclick = function() {
    modal[8].style.display = "block";
}

btn[9].onclick = function() {
    modal[9].style.display = "block";
}

btn[10].onclick = function() {
    modal[10].style.display = "block";
}


// When the user clicks on  (x), close the modal
span[0].onclick = function() {
    modal[0].style.display = "none";
}

span[1].onclick = function() {
    modal[1].style.display = "none";
}


span[3].onclick = function() {
    modal[3].style.display = "none";
}

span[4].onclick = function() {
    modal[4].style.display = "none";
}

span[5].onclick = function() {
    modal[5].style.display = "none";
}

span[6].onclick = function() {
    modal[6].style.display = "none";
}

span[7].onclick = function() {
    modal[7].style.display = "none";
}

span[8].onclick = function() {
    modal[8].style.display = "none";
}

span[9].onclick = function() {
    modal[9].style.display = "none";
}

span[10].onclick = function() {
    modal[10].style.display = "none";
}


// When the user clicks anywhere outside of the modal, close it
window.onclick = function(event) {
    if (event.target == modal[0]) {
         modal[0].style.display = "none";
     }
    if (event.target == modal[1]) {
         modal[1].style.display = "none";
     }  
     if (event.target == modal[2]) {
        modal[2].style.display = "none";
    }  
    if (event.target == modal[3]) {
        modal[3].style.display = "none";
    }  
    if (event.target == modal[4]) {
        modal[4].style.display = "none";
    }  
    if (event.target == modal[5]) {
        modal[5].style.display = "none";
    }  
    if (event.target == modal[6]) {
        modal[6].style.display = "none";
    }  
    if (event.target == modal[7]) {
        modal[7].style.display = "none";
    }  
    if (event.target == modal[8]) {
        modal[8].style.display = "none";
    }  
    if (event.target == modal[9]) {
        modal[9].style.display = "none";
    }  
    if (event.target == modal[10]) {
        modal[10].style.display = "none";
    }  

}


              
                
                
                  ×
                   
                    
                    
                    
                  
                
          
              

    
        
          Marine life

`

user_1913729 · Answer

Your code could be simplified a bit to avoid repetition, using a for loop for example:

// Get the modal
var modal = document.getElementsByClassName('modal');
// Get the button that opens the modal
var btn = document.getElementsByClassName("course-link");
// Get the  element that closes the modal
var span = document.getElementsByClassName("close");

for (let i = 0; i < modal.length; i++) {
  // When the user clicks the button, open the modal 
  btn[i].onclick = function() {
    modal[i].style.display = "block";
  };
  // When the user clicks on the backdrop or close span, close it
  modal[i].onclick = function(e) {
    if ([modal[i], span[i]].includes(e.target)) {
      modal[i].style.display = "none";
    }
  };
}

.modal { position: fixed; top: 0; left: 0; width: 100%; height: 100%; background: rgba(0, 0, 0, .3); display: none; }
.modal-content { background: #fff; margin: 1em auto; max-width: 300px; padding: 1em; }
.close { float: right; cursor: pointer; }
.course-link { cursor: pointer; padding: 5px; background: #eceded; display: inline-block; margin: .2em; }


  ×Modal content 0

Open modal 0

  ×Modal content 1

Open modal 1

  ×Modal content 2

Open modal 2

  ×Modal content 3

Open modal 3

  ×Modal content 4

Open modal 4

  ×Modal content 5

Open modal 5

Note: I simplified the HTML to make the demo simpler, but your HTML should work as-is, provided that you don't use the same id="myModal" on every modal (IDs are meant to be unique - either use different ones, or don't use them - they're not needed for the purpose of your question)

user_4925548 · Answer

I don't know what is wrong with your code, but your approach is incorrect. Firstly, using index is a bad idea. I can suggest you using one of the 3 options down below:

Secondly, use button, not "a".

If it's possible, put each modal and its close button into the same parent. Then:

document.querySelectorAll(".course-link").forEach(btn => {
  btn.addEventListener('click', () => {
    let modal = btn.closest(".parent-class").querySelector(".modal");
    modal.style.display = "none"
    modal.classList.remove('is-opened')
  });
})

If you don't open multiple modals at the same time, create a single close button and use it to close the open modal. To do that, add a class to opened model:

document.querySelector(".course-link").addEventListener('click', () => {
  document.querySelector(".modal.is-opened").style.display = "none"
})

If the first 2 don't work for you, use a common class or data attribute to match modals and buttons:




document.querySelectorAll(".course-link").forEach(btn => {
  btn.addEventListener('click', () => {
    document.querySelector(`.modal[data-name="${btn.dataset.target}"]`).style.display = "none"
  });
})

Bonus: Instead of controlling display attribute in JS, toggling "is-opened" class and changing display in CSS is a way better idea:

.modal {
  display: none;
}

.modal.is-opened {
   display: block;
}

By doing so, you can have more control over opening styles with less code.

NICon_58186 · Answer

It is generally not a good idea to use inline event handlers. Here is a minimal reproducable example using Event Delegation.

// one handler for all
document.addEventListener("click", handle);

function handle(evt) {
  const origin = evt.target;
  if (origin.dataset.modalshow) {
  //         ∟ only do stuff if element contains [data-modalshow]        
    return showModal(
      document.querySelector(`#myModal${origin.dataset.modalshow}`), 
      origin.dataset.modalshow === "none" );
  }
}

function showModal(modalElem, hideAll) {
  document.querySelectorAll(".modal").forEach(el => el.style.display = "none");
  //                                  ∟ hide all '.modal'
  if (!hideAll) {
    modalElem.style.display = "block";  
  }
}

.modal {
  display: none;
}



  
    some content #1
  



  
    some content #2
  



  
    some content #3

Modal not closing anymore

3 Answers