Changing svg to a different color where it stays

How would I change the svg color on click, where it stays and doesn’t change back to the previous color it was before?

I click the svg, the video opens up, home button is clicked.

Button is now a different color than it was before.

That’s what I am trying to do.

https://jsfiddle.net/eq0ohp78/

Would I be doing something like this?

I just tried it, it did not work.

.playa.active{
   fill: green;
}

.playb.active{
   fill: green;
}

.playc.active{
   fill: green;
}

Svgs

playa,
.playb,
.playc {
  margin: auto 20px;
  width: 90px;
  height: 90px;
  border-radius: 50%;
  cursor: pointer;
  flex-shrink: 0;
  border: none;
  background: transparent;
  padding: 0;
}

.playa {
  fill: red;
  filter: drop-shadow(3px 3px 3px rgba(0, 0, 0, 0.7));
}

.playb {
  fill: blue;
  filter: drop-shadow(3px 3px 3px rgba(0, 0, 0, 0.7));
}

.playc {
  fill: orange;
  filter: drop-shadow(3px 3px 3px rgba(0, 0, 0, 0.7));
}

;Ok this is a hint in the right direction.

When you click the home button and then you remove the class of bg1 one from the body element you would need to add a new class to the body element so that you can change the svg.


    theBody.classList.add("newButtonColor");

e.g.

 function showBody() {
    const theBody = document.querySelector("body");
    theBody.classList.remove("bg1");
    theBody.classList.add("newButtonColor");
  }

Then in the CSS you use that class to change the button color.


.newButtonColor .playa{
   fill: green;
}

.newButtonColor .playb{
   fill: green;
}

.newButtonColor .playc{
   fill: green;
}

Of course your next question will be that you don’t want all 3 buttons to change color at the same time so instead you would need to remember which button was clicked and then use a class for that button only. You would then eventually end up with three extra classes on the body for the css to use.

e.g.

.newButtonColor1 .playa{
   fill: green;
}

.newButtonColor2 .playb{
   fill: green;
}

.newButtonColor3 .playc{
   fill: green;
}

Your task is now to work out which svg was clicked and then add the class to the body for that svg.. This is now indeed a question for the JS forum :slight_smile:

What if I do this?
https://jsfiddle.net/xb6k14sr/

  function addClickToButtons(playButtons) {
    playButtons.forEach(function addHandler(playButton) {
      playButton.addEventListener("click", function buttonHandler(e) {
        coverClickHandler(e);
        playButton.style.fill = "green";
      });
    });
  }

You can do that but I don’t like it. :slight_smile:

Css should take care of the colors not JS. You should be able to control the look of your design from css and not have to go onto the css and the js to change something.

If you used a css class then you can control it from the css and change the color to whatever you want.

2 Likes

What do you mean by that?

Not this:
https://jsfiddle.net/rom6xszy/

playButton.style.fill = ".played";

.played{
  fill:green;
}

You mean this?
https://jsfiddle.net/rom6xszy/1/

.played{
  fill:green;
}
  function addClickToButtons(playButtons) {
    playButtons.forEach(function addHandler(playButton) {
      playButton.addEventListener("click", function buttonHandler(e) {
        coverClickHandler(e);
        playButton.classList.toggle("played");
      });
    });
  }

What if instead I did this?
https://jsfiddle.net/2j5xt1zf/

.played {
   fill: green;
}
  function coverClickHandler(evt) {
    hideAll(config.containers);
    const cover = evt.currentTarget;
    showCovers(cover);
    showBody();
    evt.currentTarget.classList.toggle("played");
  }

Which of these ways should be used? @Paul_Wilkins

Both are inside the code.
https://jsfiddle.net/Loqp3mw0/

.played{
  fill: green;
}
 function showCovers(playButton) {
    playButton.classList.add("played");
  }

  function coverClickHandler(evt) {
    evt.currentTarget.classList.add("played");
  }

Did any of these scripts do what you wanted them to do?

Yes, they all do what they are intended to do.

The question then becomes, which is the preferred method or way.

So you are asking Paul which is the most css-legal way to do it?

Which way makes the most sense for how the code should be written.

Those are the 2 ways I was able to come up with that work best in the code.

.played{
  fill: green;
}

Code 1
playButton.classList.add("played");
https://jsfiddle.net/vwz0mxby/

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

Code 2
evt.currentTarget.classList.add("played");
https://jsfiddle.net/4tj8oyw9/

  function coverClickHandler(evt) {
    hideAll(config.containers);
    resetPage();
    const cover = evt.currentTarget;
    showCovers(cover);
    evt.currentTarget.classList.add("played");
  }

After a play svg is clicked.

That play svg would change to a different color.
That is what the code does.

Which of these would I be using?

Is one of these the most right?

or, would this be doing this a different way?

.played{
  fill: green;
}

Code 1

playButton.classList.add("played");
https://jsfiddle.net/xkts49o0/

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

Code 2

cover.classList.add("played");
https://jsfiddle.net/m4gcxhLy/

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

Code 3

evt.currentTarget.classList.add("played");
https://jsfiddle.net/0v5w7mqp/

  function coverClickHandler(evt) {
    hideAll(config.containers);
    resetPage();
    const cover = evt.currentTarget;
    showCovers(cover);
    evt.currentTarget.classList.add("played");
  }

I was just thinking, it should be written like this? @Paul_Wilkins

 function addPlayed(playedButton) {
    playedButton.classList.add("played");
  }

  function coverClickHandler(evt) {
    addPlayed(evt.currentTarget);
  }

What should the order of the functions be then?

Do you think
function addPlayed(playedButton) {

Should be below:
function resetPage() {

Like this:?
https://jsfiddle.net/brpe8o6s/4/

function hideAll(elements) {
function resetBackground(backgroundSelector) {
function resetPage() {
function addPlayed(playedButton) {
function showCovers(playButton) {
function coverClickHandler(evt) {

This is the other way:
https://jsfiddle.net/brpe8o6s/2/

function hideAll(elements) {
function addPlayed(playedButton) {
function resetBackground(backgroundSelector) {
function resetPage() {
function showCovers(playButton) {
function coverClickHandler(evt) {
function hideAll(elements) {
    elements.forEach(hide);
  }

  function addPlayed(playedButton) {
    playedButton.classList.add("played");
  }

  function resetBackground(backgroundSelector) {
    const allBackgrounds = document.querySelectorAll(backgroundSelector);

    function hideBackground(background) {
      background.classList.add("bg1");
    }
    allBackgrounds.forEach(hideBackground);
  }

  function resetPage() {
    resetBackground("body");
  }

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

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