What should be used in the querySelector?

Paul_Wilkins · July 14, 2021, 1:22am

Both would work, but I prefer hiding the cover first before sliding the curtain.

Paul_Wilkins · July 14, 2021, 1:25am

Which can look like this. Notice how I’m keeping the cover assignment with the code that uses cover, and the curtain assignment with the code that uses curtain.

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

Those could even be split up into two separate functions, but they’re not complex enough yet to warrant that.

asasass · July 14, 2021, 1:28am

Would splitting the function look like this?

    function showCoverCurtain(curtain) {
     curtain.classList.add("slide");
   }

asasass · July 14, 2021, 1:31am

I tried that before but couldn’t get it to work.
https://jsfiddle.net/pLycsh19/1/

(function manageCurtain() {
  "use strict";

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

  function coverClickHandler(evt) {
    const cover = evt.currentTarget;
    hide(cover);
  }

  function showCoverCurtain(curtain) {
    curtain.classList.add("slide");
  }

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

Paul_Wilkins · July 14, 2021, 1:34am

Here is how it might be done:

Going from:

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

to this:

  function hideCover(cover) {
    hide(cover);
  }
  function slideCurtain(curtain) {
    curtain.classList.add("slide");
  }
  function coverClickHandler(evt) {
    hideCover(evt.currentTarget);
    slideCurtain(document.querySelector(".curtain"));
  }

asasass · July 14, 2021, 8:11am

Originally, I had it like this.

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

So, as an example:

It should be set like this?

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

Not this?

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

Paul_Wilkins · July 14, 2021, 8:12am

The variables don’t need to be grouped at the top of the function. In fact, it’s better when related things are placed together. The two statements that use cover should be together, and the two statements that use the wrap should be together.

asasass · July 15, 2021, 5:20pm

Should this be set up like this:

What should the order be?

These would stay together:

    const cover = evt.currentTarget;
    hide(cover);
    const thewrap = cover.parentElement.querySelector(".container");
    show(thewrap);

Code 1 I think it would be this way.
https://jsfiddle.net/hsyd9m2a/16/

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

or like this?

Code 2
https://jsfiddle.net/hsyd9m2a/14/

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

asasass · July 15, 2021, 6:29pm

How would that be done with this code?
https://jsfiddle.net/edv1fqjt/

Going from this to what?

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

Paul_Wilkins · July 15, 2021, 10:35pm

asasass:

Code 2
https://jsfiddle.net/hsyd9m2a/14/

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

They all work, but it seems to make better sense when the order is hide, slide, show.

asasass · July 15, 2021, 10:59pm

So, this is the order which is the better one. ok.

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

asasass · July 15, 2021, 11:15pm

Changing it over to the other version would look like?

This?

or, something different?

https://jsfiddle.net/edv1fqjt/2/

  function hideCover(cover) {
    hide(cover);
  }

  function slideCurtain(curtain) {
    curtain.classList.add("slide");
  }

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

Changing this:

const thewrap = cover.parentElement.querySelector(".container");

to this?
const thewrap = document.parentElement.querySelector(".container");

Original:
https://jsfiddle.net/hsyd9m2a/14/

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

Paul_Wilkins · July 15, 2021, 11:17pm

Why are you broadening the search for the container to the whole document?

Isn’t it supposed to be within the cover?

asasass · July 15, 2021, 11:21pm

This is the right way then?
https://jsfiddle.net/0dq27hco/4/

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

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

  function hideCover(cover) {
    hide(cover);
  }

  function slideCurtain(curtain) {
    curtain.classList.add("slide");
  }

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

Paul_Wilkins · July 15, 2021, 11:24pm

Oh right I see, it is outside of the cover area.

<div class="curtain">
  <div class="outer">
    <div class="tcell">

      <div class="container hide">

What you’ve done there works well for now, but I would use curtain instead of document so that it continues to work well when other containers are on the same page.

  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);
  }

asasass · July 15, 2021, 11:31pm

So it would go from that to this?
https://jsfiddle.net/gymxw4tq/2/

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

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

  function hideCover(cover) {
    hide(cover);
  }

  function slideCurtain(curtain) {
    curtain.classList.add("slide");
  }

  function coverClickHandler(evt) {
    hideCover(evt.currentTarget);
    slideCurtain(document.querySelector(".curtain"));
    const curtain = evt.currentTarget;
    const thewrap = curtain.parentElement.querySelector(".container");
    show(thewrap);
  }

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

Paul_Wilkins · July 16, 2021, 12:07am

I think that there are cleaner ways to deal with that.

The event listener is on the jacket:

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

So the currentTarget is that cover. We should refer to that when getting the currentTarget:

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

I would also keep everything in reference to that cover. So to get the curtain, we walk up the parents until we find one that contains the curtain classname. jQuery has a simple parents() method to do that, but with vanilla JavaScript we use a while loop until we find what we need.

  function getCurtain(cover) {
    function isCurtain(el) {
      return curtain.classList.contains("curtain");
    }
    let el = cover.parentElement;
    while (el && !isCurtain(el)) {
      el = el.parentElement;
    }
    return el;
  }

The coverClickHandler function now simplifies to the following code, all with reference to the cover:

  function coverClickHandler(evt) {
    const cover = evt.currentTarget;
    hideCover(cover);
    const curtain = getCurtain(cover);
    slideCurtain(curtain);
    const thewrap = cover.parentElement.querySelector(".container");
    show(thewrap);
  }

system · October 15, 2021, 7:07am

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.