Having trouble moving function out

Changing this:

  const resetVideo = document.querySelectorAll(".exit");
  for (let i = 0; i < resetVideo.length; i++) {
    resetVideo[i].addEventListener("click", function resetVideoHandler() {
      resetVideo[i].parentElement.querySelector(".wrap").player.destroy();
    });
  }

To this:

 function resetVideoHandler() {
    resetVideo[i].parentElement.querySelector(".wrap").player.destroy();
  }

  const resetVideo = document.querySelectorAll(".exit");
  for (let i = 0; i < resetVideos.length; i++) {
    resetVideo[i].addEventListener("click", resetVideoHandler);
  }

What am I doing wrong, and what is the correct way to write that?

In your second snippet, inside resetVideoHandler function, i is not defined. You can add it to the function as an expected parameter. Then when adding the event listener, you can use an anonymous function and call resetVideoHandler in its body, passing i, like so:

resetVideo[i].addEventListener("click", ()=>resetVideoHandler(i))

At least i think that should work. An issue might raise if you need to remove the event listener however, as its an anonymous function and cant refer to it again :stuck_out_tongue:

I’d suggest making this an event handler, if you plan to use it to handle events. In particular, this means "a function that receives an Event object.

function resetVideoHandler(event){
  event.target.parentElement.querySelector(".wrap").player.destroy()
}
´´´

Using this, your addEventListener code should work as expected.
1 Like

This works: https://jsfiddle.net/ve64gw9f/

  const resetVideo = document.querySelectorAll(".exit");
  for (let i = 0; i < resetVideo.length; i++) {
    resetVideo[i].addEventListener("click", function resetVideoHandler() {
      resetVideo[i].parentElement.querySelector(".wrap").player.destroy();
      console.log("hit");
    });
  }

This does not work: https://jsfiddle.net/6hkp3fey/

How is this one fixed so that it works?

function resetVideoHandler(event) {
    event.target.parentElement.querySelector(".wrap").player.destroy();
    console.log("hit");
  }

  const resetVideo = document.querySelectorAll(".exit");
  for (let i = 0; i < resetVideo.length; i++) {
    resetVideo[i].addEventListener("click", resetVideoHandler);
  }

The issue is that the event.target is the SVG contained within the button, not the button itself. You could use one of two workarounds:

  1. As you’ve done elsewhere, use event.currentTarget rather than event.target. event.target is the thing that caused the event, while event.currentTarget is the thing that is currently handling the event.

  2. Rather than being quite so explicit with the parentElement tree-walking, use .closest() to find the appropriate containing element:

  function resetVideoHandler(event) {
    event.target.closest(".inner-container").querySelector(".wrap").player.destroy();
    console.log("hit");
  }

I tested with that one, works fine.

1 Like

Both of these work, which is better to use?

Code 1
https://jsfiddle.net/r1e5buqd/

  function resetVideoHandler(event) {
    event.target.closest(".inner-container").querySelector(".wrap").player.destroy();
    console.log("hit");
  }

Code 2
https://jsfiddle.net/4tprqu8x/

 function resetVideoHandler(event) {
    event.currentTarget.parentElement.querySelector(".wrap").player.destroy(); 
    console.log("hit");
  }

Depends entirely on you. I prefer using closest(<selector>), simply because I can then rearrange my content later and know it will still work.

But this is your project, and it should be the one you can understand. Functionally, they’re both doing the same thing.

1 Like