Is there a more optimal way to change multiple classes and ids with javascript than this?

Question

I'm creating a meal customisation feature on a website where the customer can choose a variety of different ingredients to configure their meal. I have the back end working perfectly, but I'm having a bit of trouble with the front end.

I've used a variety of id's, classes and functions to create a hover overlay and then apply a border around the food images once they are clicked (selected) but it's quite a long method. It works, but as there are a lot more ingredients than this (another 30) I'm a bit worried about the functions getting too long.

HTML

<img id="food" onclick="select()" src="https://website.com/uploads/2020/03/Curried-Chicken-Breast-1-scaled.jpg" style="width:250px;">

<img id="food1" onclick="select1()" src="https://website.com/uploads/2020/03/Curried-Chicken-Breast-1-scaled.jpg" style="width:250px;">

CSS:

#food:hover, #food1:hover, #food2:hover, #food3:hover, #food4:hover, #food5:hover, #food6:hover, #food7:hover{
    filter: brightness(75%); !important;
}

.selected {
  border: 3px solid #186472 !important;
}

JavaScript

<script>
function select(){
    document.getElementById("food").classList.add('selected');
    document.getElementById("food1").classList.remove('selected');
    document.getElementById("food2").classList.remove('selected');
    document.getElementById("food3").classList.remove('selected');
    document.getElementById("food4").classList.remove('selected');
    document.getElementById("food5").classList.remove('selected');
    document.getElementById("food6").classList.remove('selected');
    document.getElementById("food7").classList.remove('selected');
}

function select1(){
    document.getElementById("food").classList.remove('selected');
    document.getElementById("food1").classList.add('selected');
    document.getElementById("food2").classList.remove('selected');
    document.getElementById("food3").classList.remove('selected');
    document.getElementById("food4").classList.remove('selected');
    document.getElementById("food5").classList.remove('selected');
    document.getElementById("food6").classList.remove('selected');
    document.getElementById("food7").classList.remove('selected');
}

function select2(){
    document.getElementById("food").classList.remove('selected');
    document.getElementById("food1").classList.remove('selected');
    document.getElementById("food2").classList.add('selected');
    document.getElementById("food3").classList.remove('selected');
    document.getElementById("food4").classList.remove('selected');
    document.getElementById("food5").classList.remove('selected');
    document.getElementById("food6").classList.remove('selected');
    document.getElementById("food7").classList.remove('selected');
}

function select3(){
    document.getElementById("food").classList.remove('selected');
    document.getElementById("food1").classList.remove('selected');
    document.getElementById("food2").classList.remove('selected');
    document.getElementById("food3").classList.add('selected');
    document.getElementById("food4").classList.remove('selected');
    document.getElementById("food5").classList.remove('selected');
    document.getElementById("food6").classList.remove('selected');
    document.getElementById("food7").classList.remove('selected');
}

function select4(){
    document.getElementById("food").classList.remove('selected');
    document.getElementById("food1").classList.remove('selected');
    document.getElementById("food2").classList.remove('selected');
    document.getElementById("food3").classList.remove('selected');
    document.getElementById("food4").classList.add('selected');
    document.getElementById("food5").classList.remove('selected');
    document.getElementById("food6").classList.remove('selected');
    document.getElementById("food7").classList.remove('selected');
}

function select5(){
    document.getElementById("food").classList.remove('selected');
    document.getElementById("food1").classList.remove('selected');
    document.getElementById("food2").classList.remove('selected');
    document.getElementById("food3").classList.remove('selected');
    document.getElementById("food4").classList.remove('selected');
    document.getElementById("food5").classList.add('selected');
    document.getElementById("food6").classList.remove('selected');
    document.getElementById("food7").classList.remove('selected');
}

function select6(){
    document.getElementById("food").classList.remove('selected');
    document.getElementById("food1").classList.remove('selected');
    document.getElementById("food2").classList.remove('selected');
    document.getElementById("food3").classList.remove('selected');
    document.getElementById("food4").classList.remove('selected');
    document.getElementById("food5").classList.remove('selected');
    document.getElementById("food6").classList.add('selected');
    document.getElementById("food7").classList.remove('selected');
}

function select7(){
    document.getElementById("food").classList.remove('selected');
    document.getElementById("food1").classList.remove('selected');
    document.getElementById("food2").classList.remove('selected');
    document.getElementById("food3").classList.remove('selected');
    document.getElementById("food4").classList.remove('selected');
    document.getElementById("food5").classList.remove('selected');
    document.getElementById("food6").classList.remove('selected');
    document.getElementById("food7").classList.add('selected');
}
</script>

Image example

Just some hints: use a common class (.food) instead of numbered ids, use .addEventListener() and make use of this in the event handler, use a loop to remove .selected — Andreas
– Andreas, Commented Mar 4, 2020 at 15:25
... + keep book of the currently active element in a variable, it's easy to refer it when the class needs to be removed. — Teemu
– Teemu, Commented Mar 4, 2020 at 15:26
You don't need to store the active one actually... just select the ones that have the class "selected" (technically there should only be 1 at a time) and remove the class, then add it to whichever was clicked. — Laurent S.
– Laurent S., Commented Mar 4, 2020 at 15:28
@LaurentS. Yes, but it's really easy and cheap to "keep book of the currently active element(s)" escpecially when the browser does all the hard work: const selectedElements = document.getElementsByClassName("selected"); — Andreas
– Andreas, Commented Mar 4, 2020 at 15:32
@Andreas > Yes it's cheap, but by doing it you still have a risk that the variable is not updated for some edge case. Also regarding your example, you're not supposed to re-assign a const variable so this would throw an error I think. — Laurent S.
– Laurent S., Commented Mar 4, 2020 at 15:48

Sean · Accepted Answer · 2020-03-04 18:30:21Z

If you're asking users to select items from a series of things, you should consider using radio buttons. Using radio buttons will make things easier from a code standpoint by removing the need to set the selected state with javascript, and it will be more accessible for users.

Here's one way to approach it with no javascript needed, making use of the :checked CSS pseudo-class and CSS adjacent sibling combinator (+) to add a red border to checked food items.

.food {
  display: inline-block;
}
.food input {
  visibility: hidden;
}
.food img {
  border: 5px solid white;
}
.food input:checked + label img {
  border: 5px solid red;
}
.food input:not(:checked) + label {
  cursor: pointer;
}
.food input:not(:checked) + label:hover img {
  opacity: 0.75;
}

<div class="food">
  <input type="radio" id="food1" name="food" value="food1" />
  <label for="food1"><img alt="food1" src="https://placehold.it/100/100/?text=food1"/></label>
</div>
<div class="food">
  <input type="radio" id="food2" name="food" value="food2" />
  <label for="food2"><img alt="food2" src="https://placehold.it/100/100/?text=food2"/></label>
</div>
<div class="food">
  <input type="radio" id="food3" name="food" value="food3" />
  <label for="food3"><img alt="food3" src="https://placehold.it/100/100/?text=food3"/></label>
</div>

Ahmed Tag Amer · Accepted Answer · 2020-03-04 18:21:10Z

0

Read JS comments:

window.onload = function() {
  var foods = document.getElementsByClassName('food'); // To get length of all food pictures.
        for(var i = 0; i < foods.length; i++) { // for loop to create click function on each image.
            var food = foods[i]; // get current class index.
            food.onclick = function() { // DOM onclick function.
                Array.from(foods).forEach(function(e){e.classList.remove('selected')}) // remove 'selected' class from all 'food' classes. 
                this.classList.add('selected'); // add selected class to current '.food'.
            }
        }
    }

.food:hover {
    filter: brightness(75%); !important;
}

.selected {
  border: 3px solid #186472 !important;
}

<img class="food" src="https://picsum.photos/200">

<img class="food" src="https://picsum.photos/200">

<img class="food" src="https://picsum.photos/200">

<img class="food" src="https://picsum.photos/200">

<img class="food" src="https://picsum.photos/200">

<img class="food" src="https://picsum.photos/200">

<img class="food" src="https://picsum.photos/200">

<img class="food" src="https://picsum.photos/200">

<img class="food" src="https://picsum.photos/200">

edited Mar 4, 2020 at 18:21

answered Mar 4, 2020 at 15:53

Ahmed Tag Amer

1,4221 gold badge10 silver badges24 bronze badges

6 Comments

Sean Over a year ago

@Zerion this method only lets you select one item at a time.

Andreas Over a year ago

Why document.querySelectorAll('.food') when there's already var foods = document.getElementsByClassName('food')?

Ahmed Tag Amer Over a year ago

@Sean yes you right, i think this is what he want. If you want to select more than one item at a time tell me.

Ahmed Tag Amer Over a year ago

@Andreas because document.getElementsByClassName returns a HTML collection not an array, i can use it like Array.from(foods).forEach(function(e){...} but i don't prefer this.

Andreas Over a year ago

If you would add a loop you wouldn't have to re-query the DOM on every click to get the .food items.

|

user3814024 · Accepted Answer · 2020-03-04 15:33:38Z

0

you could aways make it as a function.

function select( val ){

    for (let i = 0; i < 7; ++i){
        document.getElementById(`food${i}`).classList.remove('selected');
    } 

    document.getElementById(`food${val}`).classList.add('selected');
}

// select(1)

answered Mar 4, 2020 at 15:33

user3814024

Comments

Marty1452 · Accepted Answer · 2020-03-04 15:34:18Z

0

Use classes like selector:

<img class="food" onclick="select(this)" src="https://website.com/uploads/2020/03/Curried-Chicken-Breast-1-scaled.jpg" style="width:250px;">

<img class="food" onclick="select1(this)" src="https://website.com/uploads/2020/03/Curried-Chicken-Breast-1-scaled.jpg" style="width:250px;">

JavaScript will looks like this:

<script>
$food_els = document.getElementByClassName("food");
function select(e) {
    for (var i = 0; i < $food_els.length; i++) {
        if ($food_els[i] == e) {
            $food_els[i].classList.add("selected");
        }
        else {
            $food_els[i].classList.remove("selected");
        }
    }
}
</script>

answered Mar 4, 2020 at 15:34

Marty1452

4055 silver badges19 bronze badges

3 Comments

Andreas Over a year ago

And what's with select1?

Andreas Over a year ago

You could also reduce the "complexity" of your loop if you remove the .selected class from every element in the list. Then just add it to e after the loop.

Laurent S. Over a year ago

The least complex would be to not loop on a bunch of elements while knowing only 1 of them is supposed to have that class. Sure you won't see the difference in most cases, but as your number of elements grows the impact on performance will become more and more visible.

Andreas · Accepted Answer · 2020-03-04 16:02:11Z

0

You have to use
- a better CSS
- the div for group images
- event delegation

HTML

<div id="Group-Images">
  <img id="food1" src="Curried-Chicken-Breast-1-scaled.jpg" >
  <img id="food2" src="Curried-Chicken-Breast-2-scaled.jpg" >
  <img id="food3" src="Curried-Chicken-Breast-3-scaled.jpg" >
</div>

CSS

#Group-Images > img {
  width: 250px;
}
#Group-Images > img:hover {
  filter: brightness(75%) !important;
}
#Group-Images > img.selected {
  border: 3px solid #186472 !important;
}

JS

const GroupImages = document.getElementById('Group-Images')
  ,   AllImages   = GroupImages.querySelectorAll('img')
  ;
GroupImages.onclick=e=>
  {
  if (!e.target.matches('img')) return

  // console.log( e.target.id) // --> food1, food2,... foodN

  AllImages.forEach(imh=>
    {
    if (img===e.target) { img.classList.add('selected')   }
    else                { img.classList.remove('selected') }
    })
  }

edited Mar 4, 2020 at 16:02

Andreas

21.9k7 gold badges52 silver badges58 bronze badges

answered Mar 4, 2020 at 15:52

Mister Jojo

22.9k6 gold badges28 silver badges45 bronze badges

3 Comments

Andreas Over a year ago

Why the <div>? What if the layout doesn't allow such a change?

Andreas Over a year ago

There's a "convention" that only constructor functions that have to be used with the new operator should start with a capital letter.

Mister Jojo Over a year ago

@Andreas a <div> or anything else you want, but event delegation needs a parent element to catch any events of their child elements

Collectives™ on Stack Overflow

Is there a more optimal way to change multiple classes and ids with javascript than this?

5 Answers 5

Comments

6 Comments

Comments

3 Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

6 Comments

Comments

3 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related