these my code i written for When the user clicks anywhere outside of the modal, close it but it is not working .what is the wrong in my code

Question

these my code i written for When the user clicks anywhere outside of the modal, close it but it is not working .what is wrong in my code . "box-search","login","register" element id names.

<script> 
   var modal = ["box-search","login","register"]; 
   for(var i=0;i<3;i++) { 
      window.onclick = function(event) {
         if (event.target == modal[i]) { 
            modal[i].style.display = "none";
         }
      }
   }
</script>

Possible duplicate of [JavaScript closure inside loops – simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) — Sebastian Simon, Feb 22 '17 at 17:51
Every time you iterate through your loop, you are wiping out the last value for `window.onclick` and setting a new function as its value. Use `.addEventListener()` to register event handlers. — Scott Marcus, Feb 22 '17 at 17:53

Scott Marcus · Accepted Answer · 2017-02-22T18:08:57.370

Every time you iterate through your loop, you are wiping out the last value for window.onclick and setting a new function as its value. Use .addEventListener() to register event handlers.

Now, having said that, you have a problem with i being used in your callback function because i is declared at a higher level, so a closure is created around it. You can read more about closures here. The closure is causing the event handling functions to all be looking for modal[3] because that is the last value i was set to when the loop exited.

To avoid the closure and to correct the overwriting of window.onclick, the script should be:

<script> 
   // Use modern standards to set up event handlers:
   window.addEventListener("click", testModal)

   function testModal(event){
     var modal = ["box-search","login","register"]; 

     // Use the built-in Array.prototype.forEach method to loop
     // through the array elements, thus avoiding a numeric counter
     modal.forEach(function(element){
         // Search the DOM for elements that have id'sthat 
         // match the modal array's id's
         var el = document.getElementById(element);

         // Check the found element
         if (event.target == el) { 
            el.style.display = "none";
         }
     }); 

   }    
</script>

@Baswaraj You're welcome. Don't forget to up vote the answers that helped you. — Scott Marcus, Feb 22 '17 at 19:11

score 0 · Answer 2 · answered Feb 22 '17 at 18:01

Your window.onClick is wrong:

1- you loop on your modals outside the function, you should do inside to let it work correctly

2- you compare event.target with the modal array elements, and not it's id

3- you try to assign a style to a string (modal[i]) and not to your event.target object

This is the rewritten function (it should work)

window.onclick = function(event) {

 for(var i=0;i<3;i++) { 
  if (event.target.id == modal[i]) { 
    event.target.style.display="none";
  }
 }
}

these my code i written for When the user clicks anywhere outside of the modal, close it but it is not working .what is the wrong in my code

2 Answers2