Three divs placed side by side

Is this how it would be written?

Did I do this properly?

<div class="container-left-video">
<div class="container-middle-video">
<div class="container-right-video">

html,
body {
  height: 100%;
  padding: 0;
  margin: 0;
}

body {
  background: #353198;
}

.outer {
  display: table;
  height: 100%;
  margin: 0 auto;
}

.tcell {
  display: table-cell;
  vertical-align: middle;
}

.container {
  width: 936px;
  padding: 25px;
  margin: 100px auto;
  border-radius: 25px;
  border: 2px solid #0059dd;
  background: #000000;
}

.container-top {
  position: relative;
  height: 310px;
  margin: 0 0 45px 0;
  border-radius: 25px;
  border: 3px solid #0059dd;
  box-sizing: border-box;
  background: url("https://i.imgur.com/jEMIULl.jpg") no-repeat 0 0;
  background-size: cover;
}

.container-left-video,
.container-middle-video,
.container-right-video {
  position: relative;
  top: 50%;
  transform: translateY(-50%);
  float: left;
  margin-left: 25px;
  width: 277px;
  background: red;
  border-radius: 25px;
}

.container-left-video .jacket-left,
.container-middle-video .jacket-middle,
.container-right-video .jacket-right {
  position: relative;
  width: 80px;
  height: 80px;
  margin: auto;
  background: url("https://i.imgur.com/fzdYu8g.jpg") no-repeat 0 0;
  background-size: 100% 200%;
}

.container-middle-video .jacket-middle {
  background-position: 0 -80px;
  
}

.container-right-video .jacket-right {
  background-position: 0 -80px;
 
}

.container-left-video .jacket-left .circle,
.container-middle-video .jacket-middle .circle,
.container-right-video .jacket-right .circle {
  position: absolute;
  top: 25%;
  left: 25%;
  width: 50%;
  height: 50%;
  box-sizing: border-box;
  border: 1px solid #0059dd;
  border-radius: 50%;
  cursor: pointer;
  background: rgba(0, 0, 0, 0.5);
  transition: transform 0.1s ease 0s;
  box-shadow: rgba(0, 0, 0, 0.4) 0 0 10px;
}

.container-left-video .jacket-left .play,
.container-middle-video .jacket-middle .play,
.container-right-video .jacket-right .play {
  position: absolute;
  top: 0;
  left: 0;
  bottom: 0;
  right: 0;
  margin: auto;
  width: 38px;
  height: 22px;
  fill: #0059dd;
}

.container-left-video .jacket-left .circle:hover,
.container-middle-video .jacket-middle .circle:hover,
.container-right-video .jacket-right .circle:hover {
  border: 1px solid red;
  transform: scale(1.1);
}

.container-left-video .jacket-left .circle:hover .play,
.container-middle-video .jacket-middle .circle:hover .play,
.container-right-video .jacket-right .circle:hover .play {
  fill: red;
}

.container-left-video .wrap-left,
.container-middle-video .wrap-middle,
.container-right-video .wrap-right {
  position: relative;
  width: 277px;
  height: 207px;
  cursor: pointer;
  overflow: hidden;
  border-radius: 25px;
}

.container-left-video .wrap-left iframe,
.container-middle-video .wrap-middle iframe,
.container-right-video .wrap-right iframe {
  position: absolute;
  top: 0;
  left: 0;
  width: 277px;
  height: 207px;
  cursor: pointer;
}

.hide {
  display: none;
}

<div class="outer">
  <div class="tcell">
    <div class="container ">
      <div class="container-top">
        <div class="container-left-video">
          <div class="jacket-left">
            <div class="circle">
              <svg class="play" width="38" height="22" viewBox="0 0 26 26">
                <title>Play</title>
                <path d="M7.712 22.04a.732.732 0 0 1-.806.007.767.767 0 0 1-.406-.703V4.656c0-.31.135-.544.406-.703.271-.16.54-.157.806.006l14.458 8.332c.266.163.4.4.4.709 0 .31-.134.546-.4.71L7.712 22.04z" fill-rule="evenodd" />
              </svg>
            </div>
          </div>
          <div class="wrap-left hide">
            <div class="video" data-id="jMAShwisnQI"></div>
          </div>
        </div>
        <div class="container-middle-video">
          <div class="jacket-middle">
            <div class="circle">
              <svg class="play" width="38" height="22" viewBox="0 0 26 26">
                <title>Play</title>
                <path d="M7.712 22.04a.732.732 0 0 1-.806.007.767.767 0 0 1-.406-.703V4.656c0-.31.135-.544.406-.703.271-.16.54-.157.806.006l14.458 8.332c.266.163.4.4.4.709 0 .31-.134.546-.4.71L7.712 22.04z" fill-rule="evenodd" />
              </svg>
            </div>
          </div>
          <div class="wrap-middle hide">
            <div class="video" data-id="-SiGnAi845o"></div>
          </div>
        </div>
        <div class="container-right-video">
          <div class="jacket-right">
            <div class="circle">
              <svg class="play" width="38" height="22" viewBox="0 0 26 26">
                <title>Play</title>
                <path d="M7.712 22.04a.732.732 0 0 1-.806.007.767.767 0 0 1-.406-.703V4.656c0-.31.135-.544.406-.703.271-.16.54-.157.806.006l14.458 8.332c.266.163.4.4.4.709 0 .31-.134.546-.4.71L7.712 22.04z" fill-rule="evenodd" />
              </svg>
            </div>
          </div>
          <div class="wrap-right hide">
            <div class="video" data-id="-SiGnAi845o"></div>
          </div>
        </div>
      </div>
    </div>
  </div>
</div>

It could be written like that but it makes the CSS very verbose. It will work but the css could be shortened considerably.

You are using the same styles on all three elements and doing things like this:

.container-left-video .jacket-left,
.container-middle-video .jacket-middle,
.container-right-video .jacket-right {
  position: relative;
  width: 80px;
  height: 80px;
  margin: auto;
  background: url("https://i.imgur.com/fzdYu8g.jpg") no-repeat 0 0;
  background-size: 100% 200%;
}

Why not simply give all three elements an extra class called perhaps .jacket and then the css can be shortened to:

.jacket{	
	position: relative;
	width: 80px;
	height: 80px;
	margin: auto;
	background: url("https://i.imgur.com/fzdYu8g.jpg") no-repeat 0 0;
	background-size: 100% 200%;
}

That one change saves almost 100 characters (and you have loads of rules like that). It also makes the css easier to read and more maintainable. The html can stay mainly as you had it as I guess you may need to target individual items at some time so all you need to do to the html is add an extra class where needed.

Also be careful in your use of descendant selectors and only use when necessary or to avoid conflicts.

e.g. This:

.container-left-video .jacket-left .play

could just be this:

.play

It makes the code cleaner and easier to manage. It avoids specificity errors and makes the browser run faster (not that anyone would notice). Always keep descendant selectors as short as possible. If you find you are having long chains of selectors then your approach may be badly flawed unless you are working on some monstrous framework with third party code to consider.

I have gone through your stylesheet and html and added the classes as follows.

html,
body {
  height: 100%;
  padding: 0;
  margin: 0;
}

body {
  background: #353198;
}

.outer {
  display: table;
  height: 100%;
  margin: 0 auto;
}

.tcell {
  display: table-cell;
  vertical-align: middle;
}
.container {
	width: 936px;
 	padding: 25px;
 	margin: 100px auto;
 	border-radius: 25px;
 	border: 2px solid #0059dd;
 	background: #000000;
}
.container-top {
	position: relative;
	height: 310px;
	margin: 0;/* was margin: 0 0 45px 0 - why the 45px bottom margin ? */
	border-radius: 25px;
	border: 3px solid #0059dd;
	box-sizing: border-box;
	background: url("https://i.imgur.com/jEMIULl.jpg") no-repeat 0 0;
	background-size: cover;
}
/*
.container-left-video, 
.container-middle-video, 
.container-right-video {
*/
.v-contain{
	position: relative;
	top: 50%;
	transform: translateY(-50%);
	float: left;
	margin-left: 25px;
	width: 277px;
	background: red;
	border-radius: 25px;
}
/*
.container-left-video .jacket-left, 
.container-middle-video .jacket-middle, 
.container-right-video .jacket-right {
*/
.jacket{	
	position: relative;
	width: 80px;
	height: 80px;
	margin: auto;
	background: url("https://i.imgur.com/fzdYu8g.jpg") no-repeat 0 0;
	background-size: 100% 200%;
}
/*.container-middle-video .jacket-middle {*/
.jacket-middle {
	background-position: 0 -80px;
	background-size: 100% 200%;
}
/*.container-right-video .jacket-right {*/
.jacket-right {
	background-position: 0 -80px;
	background-size: 100% 200%;
}
/*
.container-left-video .jacket-left .circle, 
.container-middle-video .jacket-middle .circle, 
.container-right-video .jacket-right .circle {
*/
.circle{	
	position: absolute;
	top: 25%;
	left: 25%;
	width: 50%;
	height: 50%;
	box-sizing: border-box;
	border: 1px solid #0059dd;
	border-radius: 50%;
	cursor: pointer;
	background: rgba(0, 0, 0, 0.5);
	transition: transform 0.1s ease 0s;
	box-shadow: rgba(0, 0, 0, 0.4) 0 0 10px;
}
/*
.container-left-video .jacket-left .play, 
.container-middle-video .jacket-middle .play, 
.container-right-video .jacket-right .play {
*/
.play{
	position: absolute;
	top: 0;
	left: 0;
	bottom: 0;
	right: 0;
	margin: auto;
	width: 38px;
	height: 22px;
	fill: #0059dd;
}
/*
.container-left-video .jacket-left .circle:hover, 
.container-middle-video .jacket-middle .circle:hover, 
.container-right-video .jacket-right .circle:hover {
*/
.circle:hover {
	border: 1px solid red;
	transform: scale(1.1);
}
/*
.container-left-video .jacket-left .circle:hover .play, 
.container-middle-video .jacket-middle .circle:hover .play, 
.container-right-video .jacket-right .circle:hover .play {
*/
.circle:hover .play {
	fill: red;
}
/*
.container-left-video .wrap-left, 
.container-middle-video .wrap-middle, 
.container-right-video .wrap-right {
*/
.wrap{
	position: relative;
	width: 277px;
	height: 207px;
	cursor: pointer;
	overflow: hidden;
	border-radius: 25px;
}
/*
.container-left-video .wrap-left iframe, 
.container-middle-video .wrap-middle iframe, 
.container-right-video .wrap-right iframe {
*/
.wrap iframe {
	position: absolute;
	top: 0;
	left: 0;
	width: 277px;
	height: 207px;
	cursor: pointer;
}
.hide {
	display: none;
}

HTML:

<div class="outer">
  <div class="tcell">
    <div class="container ">
      <div class="container-top">
        <div class="v-contain container-left-video">
          <div class="jacket jacket-left">
            <div class="circle"> <svg class="play" width="38" height="22" viewBox="0 0 26 26">
              <title>Play</title>
              <path d="M7.712 22.04a.732.732 0 0 1-.806.007.767.767 0 0 1-.406-.703V4.656c0-.31.135-.544.406-.703.271-.16.54-.157.806.006l14.458 8.332c.266.163.4.4.4.709 0 .31-.134.546-.4.71L7.712 22.04z" fill-rule="evenodd" />
              </svg> </div>
          </div>
          <div class="wrap wrap-left hide">
            <div class="video" data-id="jMAShwisnQI"></div>
          </div>
        </div>
        <div class="v-contain container-middle-video">
          <div class="jacket jacket-middle">
            <div class="circle"> <svg class="play" width="38" height="22" viewBox="0 0 26 26">
              <title>Play</title>
              <path d="M7.712 22.04a.732.732 0 0 1-.806.007.767.767 0 0 1-.406-.703V4.656c0-.31.135-.544.406-.703.271-.16.54-.157.806.006l14.458 8.332c.266.163.4.4.4.709 0 .31-.134.546-.4.71L7.712 22.04z" fill-rule="evenodd" />
              </svg> </div>
          </div>
          <div class="wrap wrap-middle hide">
            <div class="video" data-id="-SiGnAi845o"></div>
          </div>
        </div>
        <div class="v-contain container-right-video">
          <div class="jacket jacket-right">
            <div class="circle"> <svg class="play" width="38" height="22" viewBox="0 0 26 26">
              <title>Play</title>
              <path d="M7.712 22.04a.732.732 0 0 1-.806.007.767.767 0 0 1-.406-.703V4.656c0-.31.135-.544.406-.703.271-.16.54-.157.806.006l14.458 8.332c.266.163.4.4.4.709 0 .31-.134.546-.4.71L7.712 22.04z" fill-rule="evenodd" />
              </svg> </div>
          </div>
          <div class="wrap wrap-right hide">
            <div class="video" data-id="-SiGnAi845o"></div>
          </div>
        </div>
      </div>
    </div>
  </div>
</div>

As you see we have saved thousands of characters in that small snippet. I left the old code commented out so you can see how it is achieved. Also note the few extra classes added to the html.

I’m not sure why you have that margin-bottom of 45px on the container-top as that pushes it off vertical center.

As you are using fixed widths and heights for the demo then float is OK but I would have preferred flex or even table-cell for the three columns. That would allow you to lose the position & transform hack and the float.

e.g.

.container-top {
	position: relative;
	display:flex;
	align-items:center;
	height: 310px;
	margin: 0;/* was margin: 0 0 45px 0 - why the 45px bottom margin ? */
	border-radius: 25px;
	border: 3px solid #0059dd;
	box-sizing: border-box;
	background: url("https://i.imgur.com/jEMIULl.jpg") no-repeat 0 0;
	background-size: cover;
}
/*
.container-left-video, 
.container-middle-video, 
.container-right-video {
*/
.v-contain{
	/*position: relative;
	top: 50%;
	transform: translateY(-50%);
	float: left;*/
	margin-left: 25px;
	width: 277px;
	background: red;
	border-radius: 25px;
}

More improvements would be to build responsive components rather than fixed width everywhere.

I’m guessing that your JS is now suffering from the same problem as the CSS in that you are duplicating sections of code for each of the items which is a waste of resources. I suggest you post that question in the JS forum if you cannot manage to tidy it up yourself. :slight_smile:

4 Likes

I don’t understand, there’s nothing wrong with the javascript.

It works. See.

There are 3 videos. I just never added in a third one yet because I was figuring this out first.

The html / css.

Just because it works doesn’t mean its best :slight_smile:

If we just take one snippet of your js.

(function manageCover() {
  "use strict";

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

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





(function manageCover() {
  "use strict";

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

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








(function manageCover() {
  "use strict";

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

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

You are using the same code in three places to do the same thing to three different elements.

I believe you could reduce the above code by a third to just this.

(function manageCover() {
  "use strict";

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

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

}());

However as I said above JS is not my thing so if you do want to tidy up the JS I suggest you ask the question in the JS forum as there is probably a lot more duplication in your code than I mentioned above :wink:

3 Likes

why the 45px bottom margin ?

This is why.
margin: 0 0 45px 0;

And I updated that javascript part as you suggested.

I found and listed in there 8 different ways it could be written.

4 ways forEach
4 ways for…of

Can the covers be hidden without the use javaScript, using the html?

The javascript part is removed here.

I think the answer is no.

Not as it stands. If you had used the hide class on a common parent then both items could have been toggled through css. However that would involve a lot of changes to the JS which are above my paygrade :slight_smile:

Why don’t you just add a class to hide the covers. (I’m sure you already had something similar in place before?).

I would use a CSS class like this:

.is-video.jacket{display:none;}

Then add that class with your js.

 function coverClickHandler(evt) {
        const wrapper = evt.currentTarget.nextElementSibling;
        show(wrapper);
        const theCover = wrapper.previousElementSibling;
        hideCover(theCover);
        
        initPlayer(wrapper);
    }

Then you’d need the extra hideCover function which would go here.

e.g.

function loadPlayer(opts) {
    "use strict";

    function show(el) {
        el.classList.remove("hide");
    }
     function hideCover(el) {
        el.classList.add("is-video");
    }


    function initPlayer(wrapper) {
        const video = wrapper.querySelector(".video");  
etc.........

The usual caveat applies that with regards to JS I don’t know what I;m talking about :slight_smile:

1 Like

I implemented that here and it worked.

1 Like

Do you see one of them as being an improvement over the other?

Wouldn’t the 2nd Way be more preferable because it’s utilizing the .hide {display: none;} that is already in the CSS?

and only one other line is needed to be added.
evt.currentTarget.classList.add('hide');

Way 1.) This way is your way.

.is-video.jacket,
.is-video.jacketc {
  display: none;
}

function hideCover(el) {
    el.classList.add("is-video");
}


function coverClickHandler(evt) {
    const wrapper = evt.currentTarget.nextElementSibling;
    show(wrapper);
    const theCover = wrapper.previousElementSibling;
    hideCover(theCover);
    initPlayer(wrapper);
}

Way 2.) This way uses one line of js code.

.hide {
  display: none;
}

function coverClickHandler(evt) {
    const wrapper = evt.currentTarget.nextElementSibling;
    show(wrapper);
    initPlayer(wrapper);
    // Hide the clicked cover so only the video below is visible.
    evt.currentTarget.classList.add('hide');
}

Yes that’s better but as you have show(wrapper) function which is also one line of code I thought you may need the same approach with a new function. (I also missed that it was the cover that was being clicked anyway so didn’t need the extra steps I added.)

1 Like

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