Putting svg's into a list

HTML & CSS
1

If I wanted to put these into a list, how would that be achieved?

What would the setup look like, and would I need to add anything to

Here?

.outer.isOpen {
  display: flex;
  width: auto;
  align-content: stretch;
}

https://jsitor.com/Z5GyWhfNg

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

<div class="outer">
  <div class="container with-curtain">
    <svg class="playa thePlay" width="100%" height="100%" viewBox="0 0 64 64">
      <g id="play">
        <title>Play</title>
        <path d="M25.6,46.4L44.8,32L25.6,17.6V46.4z M32,0C14.3,0,0,14.3,0,32s14.3,32,32,32s32-14.3,32-32S49.7,0,32,0z
          M32,57.6C17.9,57.6,6.4,46.1,6.4,32S17.9,6.4,32,6.4S57.6,17.9,57.6,32S46.1,57.6,32,57.6z" />
      </g>
    </svg>
    <div class="inner-container curtain curtain1">
      <div class="ratio-keeper">
        <div class="wrapa">
          <div class="video video-frame"></div>
        </div>
        <div class="panel-left"></div>
        <div class="panel-right"></div>
      </div>
    </div>
  </div>
  <div class="container with-curtain">
    <svg class="playb thePlay" width="100%" height="100%" viewBox="0 0 64 64">
      <use href="#play" />
    </svg>
    <div class="inner-container curtain curtain2">
      <div class="ratio-keeper">
        <div class="wrapb">
          <div class="video video-frame" data-id="0dgNc5S8cLI"></div>
        </div>
        <div class="panel-left"></div>
        <div class="panel-right"></div>
      </div>
    </div>
  </div>
</div>

This was an attempt
https://jsfiddle.net/tz2pc48g/3/


<div class="outer">
  <ul class="nav">
    <li class="container with-curtain">
      <svg class="playa thePlay" width="100%" height="100%" viewBox="0 0 64 64">
        <g id="play">
          <title>Play</title>
          <path d="M25.6,46.4L44.8,32L25.6,17.6V46.4z M32,0C14.3,0,0,14.3,0,32s14.3,32,32,32s32-14.3,32-32S49.7,0,32,0z
          M32,57.6C17.9,57.6,6.4,46.1,6.4,32S17.9,6.4,32,6.4S57.6,17.9,57.6,32S46.1,57.6,32,57.6z" />
        </g>
      </svg>
      <div class="inner-container curtain curtain1">
        <div class="ratio-keeper">
          <div class="wrapa">
            <div class="video video-frame"></div>
          </div>
          <div class="panel-left"></div>
          <div class="panel-right"></div>
        </div>
      </div>
    </li>
    <li class="container with-curtain">
      <svg class="playb thePlay" width="100%" height="100%" viewBox="0 0 64 64">
        <use href="#play" />
      </svg>
      <div class="inner-container curtain curtain2">
        <div class="ratio-keeper">
          <div class="wrapb">
            <div class="video video-frame" data-id="0dgNc5S8cLI"></div>
          </div>
          <div class="panel-left"></div>
          <div class="panel-right"></div>
        </div>
      </div>
    </li>
  </ul>
</div>
2

Why did you add an extra element for the ul when none is needed?

These are the two elements to change and nothing needs to be added

<div class="outer">
  <div class="container with-curtain">

e.g. change the outer to a ul.

<ul class="outer">

Then change each container to list element.

<li class="container with-curtain">

Make sure you close the appropriate tags properly.

Then in the css just remove the bullets and default padding from .outer.

.outer {
list-style:none;
padding:0;
}
1 Like
3

I did that here:
https://jsfiddle.net/wexujtn5/1/

How come when list-style is removed, the styles can’t be viewed on the screen?

/*list-style:none;*/

or if I put:
list-style: square;

nothing appears.

Does that mean I did this wrong?


<ul class="outer">
    <li class="container with-curtain">
      <svg class="playa thePlay" width="100%" height="100%" viewBox="0 0 64 64">
        <g id="play">
          <title>Play</title>
          <path d="M25.6,46.4L44.8,32L25.6,17.6V46.4z M32,0C14.3,0,0,14.3,0,32s14.3,32,32,32s32-14.3,32-32S49.7,0,32,0z
          M32,57.6C17.9,57.6,6.4,46.1,6.4,32S17.9,6.4,32,6.4S57.6,17.9,57.6,32S46.1,57.6,32,57.6z" />
        </g>
      </svg>
      <div class="inner-container curtain curtain1">
        <div class="ratio-keeper">
          <div class="wrapa">
            <div class="video video-frame"></div>
          </div>
          <div class="panel-left"></div>
          <div class="panel-right"></div>
        </div>
      </div>
    </li>
  </ul>
4

It’s because the list-item ( <li class="container with-curtain">) is also display:flex which means it is no longer display:list-item and therefore has no list-styles although its still best to remove them just in case you change the display at some time.

1 Like
5

How come with-curtain is in the code, if it appears nowhere in the code except in the html?

Was that something extra that can be removed?

I removed it here:
https://jsfiddle.net/741sw0q9/1/

6

It was put there to identify the container that had curtains rather than the one that had buttons and would have enabled the js (or css) to target one or the other if need be. I believe there is an older version where the js targets .container.with-buttons but is no longer need.

    // I just hard coded it for example
    document.querySelector(".container.with-buttons").classList.add("hide");

It’s sometimes good to name things if they have different uses so that later on you can modify it more easily and indeed helps to identify items in the code. There are probably other classes you can get rid of but will end up making the code more complicated and unreadable.

1 Like
7

I think we removed that in the js, and replaced it with something else.

I bet this can be done the same way,
document.querySelector('.outer').classList.add('isOpen');

I will have to see how the other one was done.

I am not sure how it was able to be removed.

And what it was replaced with.

https://jsfiddle.net/741sw0q9/1/

  function showCovers(playButton) {
    const cover = playButton.parentElement;
    cover.classList.add("active");
    show(cover);
  }

  function coverClickHandler(evt) {
    hideAll(config.containers);
    const cover = evt.currentTarget;
    showCovers(cover);
  }
8

That was not in your last updated code:

It was in the code before that.

What did you replace this with?

document.querySelector(".container.with-curtain").classList.add("hide");

In your updated code?

I would be able to do that with this line:
It would be changed to something else, but I am not sure what.

document.querySelector('.outer').classList.add('isOpen');

9

You don’t have any other JS targeting .outer so the quickest way to select it is to select it directly rather than walking back up the dom from where you are. Of course if you had multiple outers you would only want to find your closest parent outer and then you’d need to find it by walking back up the dom to the nearest .outer.

e.g.something like this:

cover.parentElement.parentElement.classList.add('isOpen');

I wouldn’t be happy with that as its too fragile and assumes that outer is always 2 levels above. If you change the html you break the JS and that should be avoided.

It’s a question for @Paul_Wilkins though as I may be wrong :slight_smile:

1 Like
10

I didn’t replace it you got help in the JS forum and looped through all the containers instead. Mine was just a proof of concept.

11

This line was not in the code that was improved.:
document.querySelector(".container.with-curtain").classList.add("hide");

In that topic, this is the code that was worked on. See

The code you developed.

Code:

(function manageCovera(d) {
  const allPlayButtons = d.querySelectorAll(".thePlay");
  const allContainers = d.querySelectorAll(".container");

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

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

  function addClickToButtons() {
    for (let i = 0; i < allPlayButtons.length; i++) {
      allPlayButtons[i].addEventListener("click", coverClickHandler);
    }
  }

  function coverClickHandler(evt) {
    const cover = evt.currentTarget.parentElement;
    for (let i = 0; i < allContainers.length; i++) {
      allContainers[i].classList.add("hide");
    }
    cover.classList.add("active");
    cover.classList.remove("hide");
  }

  addClickToButtons();
})(document);
12

Maybe In AS test 3 you forgot to remove:

with-curtain from the html.

When you developed the new js code.

In saying that, none of the code that was worked on here had this line in it.

document.querySelector(".container.with-curtain").classList.add("hide");

In saying that, you developed the code somehow without including that line in it.

oh.

You never used: this line in the js of your code.
document.querySelector(".container.with-curtain").classList.add("hide");

13

I have a tendency to get too easily distracted with asasass. Thank you for the callout but I have a policy now of dealing with one thread at a time.

1 Like
14

And maybe I left it there because it lets me know that it had a curtain even though its not addressed in the code. Either way it doesn’t matter now. Use it or lose it. It’s up to you.:slight_smile:

2 Likes
15

I will leave it in incase there is a scenario which calls for it, and there is no harm being done with leaving it in.

16

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