Popup image script works but develops lag

This is a script I wrote for a gallery website I’m building for a friend, the intent is to be able to click on any small image in the gallery and have it show up larger, then a second click anywhere in the window should remove the overlayed image and show the gallery again. It IS doing this, but only for the first several clicks, after 7 or 8 open and closings it develops a severe lag and becomes essentially unresponsive (over a minute before it responds to a click). Anything I try to rewrite it either makes it worse or breaks it completely, can anyone see what’s causing this?

        // track if the overlay is active
        var active = false;

        function bigPicture(){
          // listen for click and capture the image that is clicked on
          window.addEventListener('click', function open(e){
            var source = e.target;
            // ignore clicks that are on elements other than images
            if (!source.hasAttribute('src')){return};
            // make a copy of image element
            var copy = source.cloneNode();
            // select full page div to darken background
            var bg = document.getElementById('fullBackground')
            // select the container for full-size image
            var container = document.getElementById('fullContainer')
            // select rest of page behind overlay
            var page = document.querySelector('body');
            // insert the clicked image into the overlay container and set overlay classes
            container.insertAdjacentElement('afterbegin', copy);
            bg.className = "overlay";
            container.className = "overlay-image"
            // prevent scrolling while overlay is active
            page.style.overflow = "hidden";
            // track that overlay is displaying
            active = true;
            // add click listener to close overlay
            if (active) {
            // next click closes overlay and returns to gallery
              window.addEventListener('click', function close(e){
                bg.className = "";
                container.className = "";
                container.innerHTML = "";
                page.style.overflow = "scroll";
                active = false;
                bigPicture();
              });
            }
          });
        }

Explaining the slow-down is easy. What you’ve got there is a bona-fide memory leak. You’ve defined another event listener inside of the first, so each time you click it adds a new listener. Take the inner event listener out and just use the if statement.

As for why it’s not responding until you get a few clicks in, that’s not something we can answer from the code provided. When is this bigPicture function being called for the first time?

As a general rule, I say don’t add event listeners in response to other events. They should get defined once as the code first loads (if possible - and it most always is), then never touched again.

2 Likes

bigPicture is being called on page load. When I moved the if statement out the close function is no longer working, should I define it as a second function to also run on page load? Or should I make bigPicture a single if/else based on the active status? The lag is gone though without the nested listener, so you nailed the cause, thank you!

Just one big if/else. Adding more click handlers to the same object doesn’t do you any good because they’ll all fire at the same time in response to the same event.

1 Like

Awesome, thanks for the help. Here is what I wound up with, seems to be working:

    // track if the overlay is active
    var active = false;
    // select full page div to darken background
    var bg = document.getElementById('fullBackground')
    // select the container for full-size image
    var container = document.getElementById('fullContainer')
    // select rest of page behind overlay
    var page = document.querySelector('body');

    // first click opens image overlay
    function open(img){
      // ignore clicks that are on elements other than images
      if (!img.hasAttribute('src')){return};
      // make a copy of image element
      var copy = img.cloneNode();
      // insert the clicked image into the overlay container and set overlay classes
      container.insertAdjacentElement('afterbegin', copy);
      bg.className = "overlay";
      container.className = "overlay-image"
      // prevent scrolling while overlay is active
      page.style.overflow = "hidden";
      // track that overlay is displaying
      active = true
    };

    // next click closes overlay and returns to gallery
    function close(){
      bg.className = "";
      container.className = "";
      container.innerHTML = "";
      page.style.overflow = "scroll";
      active = false;
    };

    function overlay(){
      // listen for click
      window.addEventListener('click', function toggle(e) {
        // capture the image that is clicked on
        var source = e.target;
        if (!active) {
          open(source);
        } else {
          close();
        }
      });
    }
1 Like

Looks good! I like that you pulled some of that code out into functions.