Removing multiple resets from createResetHandler

The idea here would be removing the Foreach loop from the code. https://jsfiddle.net/cdost128/

 function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
        video.addEventListener("click", function resetVideoHandler() {
            player.destroy();
            console.log("hit");
          }
        );
      }
    )
  }

1 option I found that works is this: https://jsfiddle.net/Lvejqks9/

1, 2, 3, 4, 5

player.getIframe() Can be found in YouTube’s api.
https://developers.google.com/youtube/iframe_api_reference

This method returns the DOM node for the embedded <iframe>

Also, this line used to be this long: const exitButton = player.getIframe().closest(".inner-container").querySelector(".exit");

I was able to make it shorter.

  function createResetHandler(player) {
    const exitButton = player.getIframe().closest(".inner-container")
    const resetVideo = exitButton.querySelector(".exit");
    resetVideo.addEventListener("click", function resetVideoHandler() {
      resetVideo.removeEventListener("click", resetVideoHandler);
      player.destroy();
      console.log("hit");
    });
  }

2nd option I found that works is this: https://jsfiddle.net/b0729udj/

1, 2, 3, 4, 5

function createResetHandler(player) {
    document.addEventListener("click", function({
      target
    }) {
      if (target.closest(".exit")) {
        player.destroy();
        console.log("hit");
      }
    }, {
      once: true
    });
  }

Why would you want to remove that? What is the problem that you are having with it?

Everytime the exit button is clicked, the createResetHandler gets called multiple times.

To reproduce, clicking on the Exit button 1 time, after another, produces these numbers inside console.log . https://jsfiddle.net/xcq0L372/

Clicking on the exit button one time gives 1
a 2nd time gives, 3
3rd time: 6
4th time: 10
5th time: 15

1, 3, 6, 10, 15

Those numbers instead should be.

1, 2, 3, 4, 5

function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", function resetVideoHandler() {
        player.destroy();
        console.log("hit");
      });
    })
  }

I recommend then restructuring the code so that a very clear init section is used for things that should happen only once.

How would that be done?

I’m not quite sure I understand what you mean by that.

These go together:

  function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", function resetVideoHandler() {
        player.destroy();
        console.log("hit");
      });
    })
  }

  function onPlayerReady(event) {
    const player = event.target;
    player.setVolume(100);
    createResetHandler(player);
  }

Some Solutions I Found

1 option is this: https://jsfiddle.net/236fs8bn/

  let resetHandlerDone = false;
  function createResetHandler(player) {
    if (resetHandlerDone) return;
    resetHandlerDone = true;

    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", function resetVideoHandler() {
        player.destroy();
        console.log("hit");
      });
    });
  }

2nd option is this: https://jsfiddle.net/roeas4hj/

const resetVideo = document.querySelectorAll(".exit");
  resetVideo.forEach(function resetVideoHandler(video) {
    video.addEventListener("click", function resetVideoHandler() {
      video.parentElement.querySelector(".wrap").player.destroy();
      console.log("hit");
    });
  })

3rd option: https://jsfiddle.net/L6whfrmx/

  function Once(fn) {
    let run = true
    return function(args) {
      if (!run) return
      run = false
      return fn(args)
    }

  }

  const createResetHandler = Once(function(player) {
    const resetVideo = document.querySelectorAll(".exit")
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", function resetVideoHandler() {
        player.destroy();
        console.log("hit");
      })
    })
  })

To help diagnose this I’ve added a console.log before each addEventHandler statement, telling me what is being added for example:

      console.log("video reset click handler");
      video.addEventListener("click", function resetVideoHandler() {

When I run the page, I see in the log 9 cover handlers, 9 play button handlers, and 9 exit button handlers. There are no video reset handlers seen at that stage.

On playing one of them, that is when 9 video reset handlers are added. That happens each time that one of them is played, and is what is causing the problem.

When playing the video, that’s not a good time to add those reset handlers. Some web browsers cannot remove event handlers, so removing them afterwards is not an option either.

The appropriate behaviour is to add the video remove handler with the other handlers. A problem exists though because the handler seems to need a direct reference to the player.

  function createResetHandler(player) {
    ...
      video.addEventListener("click", function resetVideoHandler() {
        player.destroy();
        console.log("hit");
      });
    })
  }

For the reset handler to work without having that reference to the player, the player needs to be available in some other way instead, and I think that we have already made that option a reality by adding the player to the wrapper.

  function initPlayer(wrapper, playerOptions) {
    show(wrapper);
    const player = createPlayer(wrapper, playerOptions);
    wrapper.player = player;
  }

Thanks to that, we can gain a reference to the player from the wrapper instead.

There are two main things that need to be done to resolve this problem:

  • Move the call to createResetHandler out of onPlayerReady and instead to somewhere else when the page loads with access to the wrapper
  • Update createResetHandler so that it uses wrapper to get the player

And most important of all, achieve all of that without breaking the good structure of the code that helps to make it easier to keep it updated.

And all of that doesn’t mean removing the forEach loop in any way.

I am having difficulty understanding the instructions you gave me.

First:

Move the call to createResetHandler out of onPlayerReady and instead to somewhere else when the page loads with access to the wrapper

Where does this get placed?
createResetHandler(player);

I don’t know where that is getting placed.

Somewhere in here?


  function initPlayer(wrapper, playerOptions) {
    show(wrapper);
    const player = createPlayer(wrapper, playerOptions);
    wrapper.player = player;
  }

  return {
    adder: playerAdder,
    createCallback
  };
}());

I don’t know, or understand what I am supposed to be doing.

It is all very confusing to me.

Some things I am able to understand very easily, this, what you are having me do here I am having difficulty with implementing.

I don’t understand what this means either:

Update createResetHandler so that it uses wrapper to get the player

How do I do that?

 function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", function resetVideoHandler() {
        player.destroy();
        console.log("hit");
      });
    })
  }

First we will move createResetHandler to somewhere that’s more suitable.

The exit button is a part of the UI so some of the code belongs in manageUI, and other parts of the code removes the video and so belongs in the managePlayer code.

Here is how the createResetHandler function currently looks, color-coded to show the different sections that the code relates to.

image

I’m thinking that the best way to solve this is for some of the code to be in manageUI and some of it to be in managePlayer. That way we can give the manageUI code a link to the managePlayer function that removes the video.

After doing that I’ll update the resetVideoHandler so that it can get the player from the wrapper, which means that we also need to get the wrapper from exit button.

But first the code gets to be split up into different sections for manageUI and the managePlayer modules, which will be my next post.

1 Like

Starting from the inside of the code, the managePlayer module has another method added to it to remove (using destroy) a player.

 function removePlayer(player) {
   player.destroy();
 }
...
 return {
   adder: playerAdder,
   remove: removePlayer
 };

We can now use that removePlayer code from the resetVideoHandler function.

      video.addEventListener("click", function resetVideoHandler() {
        managePlayer.remove(player);
      });

The next stage for that code is to replace the resetVideoHandler code with managePlayer.removeHandler instead, but before doing that we need to use an evt variable to gain the player.

We can add evt to the function:

      video.addEventListener("click", function resetVideoHandler(evt) {
        ...
      });

and use the evt target to gain the wrapper. The evt.target refers to the exit button so we can get the highest-level parent for each video, which is the container element.

        const el = evt.target;
        const container = el.closest(".container");
        const wrapper = container.querySelector(".wrap");
        managePlayer.remove(wrapper.player);

That resetVideoHandler function can now be moved out to the managePlayer code as a function called removePlayerHandler

  function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", managePlayer.removeHandler)
    });
  }

And from there, the player parameter isn’t needed anymore, so a callback variable can be used there instead to refer to managePlayer.removeHandler

  // function createResetHandler(player) {
  function createResetHandler(callback) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", callback)
    });
  }
...
    // createResetHandler(player);
    createResetHandler(managePlayer.removeHandler);

And lastly, that createResetHandler shouldn’t be run every time a player is added, but only once when the page is loaded. Also the createResetHandler function doesn’t belong in the videoPlayer code because it deals with the exit buttons, so moving createResetHandler to the manageUI code needs to be done, along with adding a public method to the end of the manageUI code so that it can be accessed. This is also a good time to rename createResetHandler to addExitHandlers too.
so moving that to the start of the onYouTubeIframeAPIReady function is a good solution to that.

Some of the work that needs to be done is described in the text so you may need some help with that, but all of the information is there to successfully achieve a good and working solution.

1 Like

Thank you for describing how each step is done.

First Step: https://jsfiddle.net/5hgzrpau/

Where inside:

const managePlayer = (function makeManagePlayer() {

Should removePlayer function be placed?

 function removePlayer(player) {
   player.destroy();
 }

I was thinking here, below show.

That was done here: https://jsfiddle.net/qf374ojx/

  function show(el) {
    el.classList.remove("hide");
  }

  function removePlayer(player) {
    player.destroy();
  }

As we also have playerAdder in the managePlayer module, it makes good sense for removePlayer to be placed below the playerAdder function.

1 Like

ok, I did that here: https://jsfiddle.net/a3fuwg96/

function playerAdder(parent, playerOptions) {
    const wrapper = parent.querySelector(".wrap");
    return function callback() {
      initPlayer(wrapper, playerOptions);
    };
  }

  function removePlayer(player) {
    player.destroy();
  }

This is the last step where I knew what I was doing. https://jsfiddle.net/a3fuwg96/

After that, I don’t know what I am supposed to be doing.

Is this right? https://jsfiddle.net/n6zhso3u

 function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", function resetVideoHandler(evt) {
        const el = evt.target;
        const container = el.closest(".container");
        const wrapper = container.querySelector(".wrap");
        managePlayer.remove(wrapper.player);
      });
    })
  }

After that I did this: where the code stops working. https://jsfiddle.net/n6zhso3u/2/

const managePlayer = (function makeManagePlayer() {

   function removePlayerHandler(callback) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", function resetVideoHandler(evt) {
        const el = evt.target;
        const container = el.closest(".container");
        const wrapper = container.querySelector(".wrap");
        managePlayer.remove(wrapper.player);
      });
    })
  }
   removePlayerHandler(managePlayer.removeHandler);

I am getting confused and stuck somewhere.

As I understand it, this is supposed to be in manageUI

I did that here: https://jsfiddle.net/em5ycgqt/1/

 function addExitHandlers (callback) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", callback)
    });
  }
    addExitHandlers (managePlayer.removeHandler);
  return {
    addExitHandlers,
    init
  };
}());

What was supposed to happen to this?

I’m getting confused and don’t know what I am stuck on.

video.addEventListener("click", function resetVideoHandler(evt) {
        const el = evt.target;
        const container = el.closest(".container");
        const wrapper = container.querySelector(".wrap");
        managePlayer.remove(wrapper.player);
      });

Please don’t expect me to scour my way through your code attempting to match it up with the earlier instructions.

You are expected to explain in words about things like where what you got up to and what was done when it stops working.

1 Like

I am having trouble understanding how to do Step 3 post #10

Step 2 https://jsfiddle.net/n4umbqsa/

  function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", function resetVideoHandler(evt) {
        const el = evt.target;
        const container = el.closest(".container");
        const wrapper = container.querySelector(".wrap");
        managePlayer.remove(wrapper.player);
      });
    })
  }

Step 3: Last working code: https://jsfiddle.net/n4umbqsa/

Next you want me to do this:

Trying to follow these instructions:

I’m stuck trying to understand what you want me to do here:

That resetVideoHandler function can now be moved out to the managePlayer code as a function called removePlayerHandler

Just this part gets removed? and changed to: removePlayerHandler

resetVideo.forEach(function removePlayerHandler(video) {
      video.addEventListener("click", function removePlayerHandler(evt) {
        const el = evt.target;
        const container = el.closest(".container");
        const wrapper = container.querySelector(".wrap");
        managePlayer.remove(wrapper.player);

I don’t understand how to remove half a function from here leaving the other half of the function in videoPlayer

 function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", function resetVideoHandler(evt) {
        const el = evt.target;
        const container = el.closest(".container");
        const wrapper = container.querySelector(".wrap");
        managePlayer.remove(wrapper.player);
      });
    })
  }

That would mean, this stays inside videoPlayer?

 function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    }

And this piece gets removed and added to managePlayer?

resetVideo.forEach(function removePlayerHandler(video) {
      video.addEventListener("click", function removePlayerHandler(evt) {
        const el = evt.target;
        const container = el.closest(".container");
        const wrapper = container.querySelector(".wrap");
        managePlayer.remove(wrapper.player);

I don’t think I am understanding this correctly.

Does all of this get deleted and removed from videoPlayer?

  function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", function resetVideoHandler(evt) {
        const el = evt.target;
        const container = el.closest(".container");
        const wrapper = container.querySelector(".wrap");
        managePlayer.remove(wrapper.player);
      });
    })
  }

The next thing you say is this:

And from there, the player parameter isn’t needed anymore, so a callback variable can be used there instead to refer to managePlayer.removeHandler

But in here, resetVideoHandler() was never changed to: removePlayerHandler()

So, I’m confused.

  // function createResetHandler(player) {
  function createResetHandler(callback) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", callback)
    });
  }
...
    // createResetHandler(player);
    createResetHandler(managePlayer.removeHandler);

Which leads me to believe this gets deleted and removed, and is no-longer being used.

        const el = evt.target;
        const container = el.closest(".container");
        const wrapper = container.querySelector(".wrap");
        managePlayer.remove(wrapper.player);

Well here is the code in the createResetHandler function.

  function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", function resetVideoHandler(evt) {
        const el = evt.target;
        const container = el.closest(".container");
        const wrapper = container.querySelector(".wrap");
        managePlayer.remove(wrapper.player);
      });
    });
  }

A suitable way to move a function around is to first move it out of the code to somewhere else, such as above the code. That way you can still easily refer to the function.

  function resetVideoHandler(evt) {
    const el = evt.target;
    const container = el.closest(".container");
    const wrapper = container.querySelector(".wrap");
    managePlayer.remove(wrapper.player);
  }
  function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      // video.addEventListener("click", function resetVideoHandler(evt) {
      //   ...
      // });
      video.addEventListener("click", resetVideoHandler);
    });
  }

We can now rename the function to be something else, such as removePlayerHandler

  // function resetVideoHandler(evt) {
  function removePlayerHandler(evt) {
    const el = evt.target;
    const container = el.closest(".container");
    const wrapper = container.querySelector(".wrap");
    managePlayer.remove(wrapper.player);
  }
  function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      // video.addEventListener("click", resetVideoHandler);
      video.addEventListener("click", removePlayerHandler);
    });
  }

The removePlayerHandler function can now be easily moved to a different module such as managePlayer (with a suitable public method also being added to the module returns)

const managePlayer = (...
  ...
  function removePlayerHandler(evt) {
    const el = evt.target;
    const container = el.closest(".container");
    const wrapper = container.querySelector(".wrap");
    managePlayer.remove(wrapper.player);
  }

  return {
    ...
    removePlayerHandler
  };
}());

and you only need to update the addEventListener line with a reference to the new location.

    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", maangePlayer.removePlayerHandler);
    });
2 Likes

I followed those instructions here:

Step 3 https://jsfiddle.net/3ej1vz0w/

const videoPlayer = (function makeVideoPlayer() {

  function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", managePlayer.removePlayerHandler);
    });
  }

Here I placed function removePlayerHandler(evt) {
under function removePlayer(player) if that is a good spot for it.

 function removePlayer(player) {
    player.destroy();
  }

  function removePlayerHandler(evt) {
    const el = evt.target;
    const container = el.closest(".container");
    const wrapper = container.querySelector(".wrap");
    managePlayer.remove(wrapper.player);
  }

In Step 4, player.destroy(); stops working.

The last time it was working was Step 3 post #25

Step 4 Is reflected at this link: https://jsfiddle.net/4g7qop8L/

This then becomes:

  function createResetHandler(player) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", managePlayer.removePlayerHandler);
    });
  }

This:

function createResetHandler(callback) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", callback)
    });
  }

Then I did this: Added: createResetHandler(managePlayer.removeHandler);

To here:

function onPlayerReady(event) {
    const player = event.target;
    player.setVolume(100);
    createResetHandler(managePlayer.removeHandler);
  }

To know if player.destroy(); is working or not, the video has to be playing, then the X button gets clicked.

If the video shuts off, that means, player.destroy(); is working.

After Step 4 was done, player.destroy(); stops working in the code.
https://jsfiddle.net/4g7qop8L/

This was Step 4’s instructions. post #10

And from there, the player parameter isn’t needed anymore, so a callback variable can be used there instead to refer to managePlayer.removeHandler

  // function createResetHandler(player) {
  function createResetHandler(callback) {
    const resetVideo = document.querySelectorAll(".exit");
    resetVideo.forEach(function resetVideoHandler(video) {
      video.addEventListener("click", callback)
    });
  }
...
    // createResetHandler(player);
    createResetHandler(managePlayer.removeHandler);