What should be used in the querySelector?

Should .container .video .wrap .video-launch be used in the querySelector?

or something else?

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

It appears that the .container class doesn’t need to be wrapped around anything in order for the code to work, so perhaps all I would do here is change the name of container to something more appropriate…

Works fine like this.
<div class="container"></div>

It just needs to be placed anywhere in the html between these 2 classes.

<div class="curtain">

<div class="jacket" title="Play">

All of these codes here work.

Code 1 .video-launch
https://jsfiddle.net/b36fx9yj/1/

          <div class="video-wrapper">
            <div class="video-ratio-keeper">
              <div class="video video-launch video-frame"></div> <!-- .video -->
            </div> <!-- .video-wrapper -->
          </div><!-- .video-ratio-keeper -->

const thewrap = cover.parentElement.querySelector(".video-launch");

Code 2 .wrap
https://jsfiddle.net/3upkzhyc/1/

          <div class="video-wrapper">
            <div class="video-ratio-keeper">
             <div class="wrap">
              <div class="video video-frame"></div> <!-- .video -->
              </div> <!-- .wrap -->
            </div> <!-- .video-wrapper -->
          </div><!-- .video-ratio-keeper -->

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

Code 3 .container
https://jsfiddle.net/2yd9thej/

<div class="container">
            <div class="video-wrapper">
              <div class="video-ratio-keeper">
                  <div class="video video-frame"></div> <!-- .video -->
              </div> <!-- .video-wrapper -->
            </div><!-- .video-ratio-keeper -->
          </div> <!-- .container -->

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

Code 4 .container
https://jsfiddle.net/g652htx8/

<div class="container"></div> <!-- .container -->

          <div class="video-wrapper">
            <div class="video-ratio-keeper">
              <div class="video video-frame"></div> <!-- .video -->
            </div> <!-- .video-wrapper -->
          </div><!-- .video-ratio-keeper -->

Code 5 .container
https://jsfiddle.net/xso1cv8d/

 <div class="video-wrapper">
            <div class="video-ratio-keeper">
              <div class="video video-frame"></div> <!-- .video -->
            </div> <!-- .video-wrapper -->
          </div><!-- .video-ratio-keeper -->

          <div class="container"></div> <!-- .container -->

Code 6 .video
https://jsfiddle.net/j3e4c8g1/

          <div class="video-wrapper">
            <div class="video-ratio-keeper">
              <div class="video video-frame"></div> <!-- .video -->
            </div> <!-- .video-wrapper -->
          </div><!-- .video-ratio-keeper -->

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

Correctly naming things is one of the most difficult things that programmers have to deal with.

Most names can be ranked on a scale from generic to specific. Your names in an approximate order are:

  • container
  • wrap
  • video
  • video-launch

Using a name that’s too generic doesn’t tell you anything useful, and a name that’s too specific has to be changed across different implementations because it no longer fits.

As all of your implementations about hiding something and then showing the video, video-wrapper seems to strike a suitable balance as the one to use.

2 Likes

If I set it to .video-wrapper, how do you explain it not working as a wrapper?

https://jsfiddle.net/u9dg5re2/

         <div class="video-ratio-keeper">
            <div class="video video-frame"></div> <!-- .video -->
          </div><!-- .video-ratio-keeper -->

          <div class="video-wrapper"> </div> <!-- .video-wrapper -->

Shouldn’t the code not be able to work with where .video-wrapper is positioned in the html?

It’s not even wrapping around the .video class that is only in the javascript.

  function initPlayer(wrapper) {
    videoPlayer.init({
      video: wrapper.querySelector(".video")
    });
  }

Shouldn’t the code be broken like that?

What does that tell you?

What does that mean?

I’m trying to figure that out.

Can you please provide a link to the previous working version of that code…

That would be this one.

https://jsfiddle.net/1baop5dz/2/

.container isn’t wrapped around anything and the code is still functioning.

Why is that?

          <div class="container"> </div> <!-- .container -->
          
            <div class="video-wrapper">
              <div class="video-ratio-keeper">
                  <div class="video video-frame"></div> <!-- .video -->
              </div> <!-- .video-wrapper -->
            </div><!-- .video-ratio-keeper -->

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

Both sets of code seem to work the same. Please provide clearer instructions about what doesn’t work.

.container isn’t wrapped around anything in the html part of the code and it is still functioning.

Why is that?

How is that explained? Doesn’t it need to be or it would be broken?

      <div class="container"> </div> <!-- .container -->
      
        <div class="video-wrapper">
          <div class="video-ratio-keeper">
              <div class="video video-frame"></div> <!-- .video -->
          </div> <!-- .video-wrapper -->
        </div><!-- .video-ratio-keeper -->

It seems that the container was from earlier code, and has no purpose in this code.

But the same thing can be said about .video-wrapper

It’s not wrapped around anything in the code and it still works.

https://jsfiddle.net/26g4L5wp/

          <div class="video-ratio-keeper">
            <div class="video video-frame"></div> <!-- .video -->
          </div><!-- .video-ratio-keeper -->

          <div class="video-wrapper"> </div> <!-- .video-wrapper -->

But the same thing can be said about .video-wrapper

It’s not wrapped around anything in the code and it still works.

https://jsfiddle.net/26g4L5wp/

          <div class="video-ratio-keeper">
            <div class="video video-frame"></div> <!-- .video -->
          </div><!-- .video-ratio-keeper -->

          <div class="video-wrapper"> </div> <!-- .video-wrapper -->

You are finding that you have more and more useless things left in there from your earlier work. It takes discipline as you work, to clean up after yourself. That comes with practice.

I can do 1 of 3 things.

Code 1 Add .wrap
https://jsfiddle.net/Lefcbvxs/7/

              <div class=".wrap">
                <div class="video video-frame"></div>
              </div>

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

Code 2 Use .video-frame
https://jsfiddle.net/Lefcbvxs/1/

<div class="video video-frame"></div>

const thewrap = cover.parentElement.querySelector(".video-frame");

.video-frame {
  position: absolute;
  top: 0;
  left: 0;
  width: 100%;
  height: 100%;
}

Code 3 Add .video-box
https://jsfiddle.net/Lefcbvxs/6/

<div class="video video-box video-frame"></div>

const thewrap = cover.parentElement.querySelector(".video-box");

What about just using video?

1 Like

I thought I wasn’t allowed to because it is used in the javascript?

  function initPlayer(wrapper) {
    videoPlayer.init({
      video: wrapper.querySelector(".video")
    });
  }

I see no reason why there that shouldn’t be allowed.

1 Like

What’s the difference between how both of these codes are written?

Both work.

I would also say that, looking at all my other previous cover codes, where there was only 1 video used,

They were all set up like this:

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

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

The main differences seem to be the same here.

Code 1

   function hide(el) {
      el.classList.add("hide");
        document.querySelector(".curtain").classList.add("slide");
   }

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

Code 2

  function show(el) {
    el.classList.remove("hide");
    document.querySelector(".curtain").classList.add("slide");
  }

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

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

Code 1
Can this code be written like this?

I think I can write it this way, right?
https://jsfiddle.net/yz0sawn5/

(function manageCurtain() {
   "use strict";

   function hide(el) {
      el.classList.add("hide");
        document.querySelector(".curtain").classList.add("slide");
   }

   function coverClickHandler(evt) {
      const cover = evt.currentTarget;
      hide(cover);
   }
   const cover = document.querySelector(".jacket");
   cover.addEventListener("click", coverClickHandler);
}());

Code 2 How it was written before.
https://jsfiddle.net/wsehq098/5/https://jsfiddle.net/wsehq098/5/

(function manageCurtain() {
   "use strict";

  function show(el) {
    el.classList.remove("hide");
    document.querySelector(".curtain").classList.add("slide");
  }

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

  function coverClickHandler(evt) {
    const cover = evt.currentTarget;
    const thewrap = cover.parentElement.querySelector(".video");
    hide(cover);
    show(thewrap);
  }
  const cover = document.querySelector(".jacket");
  cover.addEventListener("click", coverClickHandler);
}());

My main concern with that code is the curtain line. It doesn’t belong at all in show or hide functions at all. Please extract that curtain code and put it inside the clickHandler function instead.

1 Like

I thought the same thing.

This:
document.querySelector(".curtain").classList.add("slide");

Would go inside here:

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

Would it look something like this?

curtain.classList.add(slide);

No, there is no variable for curtain.

There are a few options:

You could leave it as it is:

document.querySelector(".curtain").classList.add("slide");

or, you could assign curtain to a local variable:

const curtain = document.querySelector(".curtain");
curtain.classList.add("slide");

I tend to prefer the second option.

1 Like

Like this?

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

or this way?

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