Can currentTarget; be removed from this code?

JavaScript
#1

Is it needed in the code?

Will I run into issues with it if it’s not there, or does it need to be there?

Because it works without it in the code, does that mean it is not needed at this time?

Removed:

const cover = evt.currentTarget;
hide(cover);

https://jsfiddle.net/xagd48z5/


(function iife() {
  "use strict";

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

  function hide(el) {
    el.classList.add("hide");
  }

  function coverClickHandler(evt) {
    const curtain = document.querySelector(".curtain");
    curtain.classList.add("slide");
    const thewrap = curtain.parentElement.querySelector(".container");
    show(thewrap);
  }

    const cover = document.querySelectorAll('.jacketa');
    cover.forEach(function(el) {
      el.addEventListener('click', coverClickHandler)
    });
    }());

Also,

This gets used instead here because there are 2 .jacketa classes in the html/css.

    const cover = document.querySelectorAll('.jacketa');
    cover.forEach(function(el) {
      el.addEventListener('click', coverClickHandler)
    });
    }());

Instead of this:

  const cover = document.querySelector(".jacketa");
  cover.addEventListener("click", coverClickHandler);

That’s a change I made to it.

If the play image wasn’t splitting apart, then it would go back to the way it was before, and there would only be 1 class.

#2

There is only one cover there and the cover variable is defined in the same scope as the handler. So strictly speaking it can work without it, but only because it relies on a fancy technique called closure which is easy to get wrong.

It’s also true that you can communicate with someone without first getting their attention, but it’s more polite and reliable to let them know that you want to communicate with them before launching forth with what you want to say.

In the same way, it’s better to use the information from the click event to get the element that was clicked. That way it’s more reliable and provides better assurance that the same code will work when multiple covers are on the same page.

Keep on using evt.currentTarget.

1 Like
#3

What if this is meant to have only 1 cover, and that’s it?

In that instance, then it is ok?

#4

It is not okay to ignore the event information from an event handler. That is a bad practice that leaves land-mines in your wake.

#5

You want me to agree with you that doing a bad thing is okay because it still works.

There are many things that can be done in JavaScript that still work. That does not make them okay.

2 Likes
#6

If I add it back in, look what happens.

https://jsfiddle.net/n8afgqx5/

Click on the play image.

 function coverClickHandler(evt) {
    const cover = evt.currentTarget;
    hide(cover);
    const curtain = document.querySelector(".curtain");
    curtain.classList.add("slide");
    const thewrap = curtain.parentElement.querySelector(".container");
    show(thewrap);
  }

  const cover = document.querySelectorAll('.jacketa');
  cover.forEach(function(el) {
    el.addEventListener('click', coverClickHandler)
  });
}());
#7

Please supply the working link that you obtained it from.

#8

I found the issue.
https://jsfiddle.net/1axmLkr6/

It was missing this.

.jacketa {
  display: block !important;
}
1 Like
#9