getElementsByClassName doesn't work, but getElementById does? [duplicate]
Asked Answered
N

2

7

I've written a script, it's goal is to stop displaying images one and two, while allowing image 3 to remain displayed and move into their place. It works fine when I use div Id's instead of div Classes, but I would prefer to use div classes so I can group the elements like this:

 function myFunction() {
     var y = document.getElementsByClassName("firstimage secondimage");
     if (y.style.display === 'none') {
           y.style.display = 'block';
     } else {
           y.style.display = 'none';
     }
 }

rather than this (in order to save space should I choose to include more elements):

 function myFunction() {
     var x = document.getElementById("firstimage");
     if (x.style.display === 'none') {
          x.style.display = 'block';
     } else {
          x.style.display = 'none';
     }

     var y = document.getElementById("secondimage");
     if (y.style.display === 'none') {
          y.style.display = 'block';
     } else {
          y.style.display = 'none';
     }
}

I thought that just changing the div id's to div classes, and the #imagenumber's to .imagenumber's (in addition to the change in the javascript I described above) would work but the script stops working when I do. I need the script to function in the same way that the code I am pasting below does, but with div classes instead of div Id's. Please tell me where I am going wrong.

CSS:

#firstimage {
    width: 100px;
    height: 100px;
    padding: 0px 0;
    text-align: center;
    background-color: green;
    margin-top:20px;
    color: white;
}

#secondimage {
    width: 100px;
    height: 100px;
    padding: 0px 0;
    text-align: center;
    background-color: blue;
    margin-top:20px;
    color: white;
}

#thirdimage {
    width: 100px;
    height: 100px;
    padding: 0px 0;
    text-align: center;
    background-color: red;
    margin-top:20px;
    color: white;
}

HTML:

<button onclick="myFunction()">Try me</button>

<div id="firstimage">
    DIV element.
</div>

<div id="secondimage">
    A second DIV element.
</div>

<div id="thirdimage">
    A third DIV element.
</div>

Javascript:

function myFunction() {
     var x = document.getElementById("firstimage");
     if (x.style.display === 'none') {
          x.style.display = 'block';
     } else {
          x.style.display = 'none';
     }

     var y = document.getElementById("secondimage");
     if (y.style.display === 'none') {
          y.style.display = 'block';
     } else {
          y.style.display = 'none';
     }
}
Narcis answered 27/8, 2016 at 6:23 Comment(1)
It's not that hard. You fire up your debugger, place a breakpoint on the line after you call getElementsByClassName, then examine the variable y. Then try examining y.style. You should be able to figure out what's happening quite quickly. Or, you could search for getElementsByClassName not working on SO, since there are many questions on the topic here, starting six years ago.Names
A
4

You should use getElementsByClassName() or querySelectorAll() to collect all div.Klass (Klass being an arbitrary name). The following Snippet uses querySelectorAll() details are commented in source.

SNIPPET

function toggleDiv() {
  // Collect all .image into a NodeList
  var xs = document.querySelectorAll(".image");
  // Declare i and qty for "for" loop
  var i, qty = xs.length;
  // Use "for" loop to iterate through NodeList
  for (i = 0; i < qty; i++) {
    // If this div.image at index [i] is "none"...
    if (xs[i].style.display === 'none') {
      // then make it "block"... 
      xs[i].style.display = 'block';
    } else {
      // otherwise set display to "none"
      xs[i].style.display = 'none';
    }
  }
}
#firstimage {
  width: 100px;
  height: 100px;
  padding: 0px 0;
  text-align: center;
  background-color: green;
  margin-top: 20px;
  color: white;
}
#secondimage {
  width: 100px;
  height: 100px;
  padding: 0px 0;
  text-align: center;
  background-color: blue;
  margin-top: 20px;
  color: white;
}
#thirdimage {
  width: 100px;
  height: 100px;
  padding: 0px 0;
  text-align: center;
  background-color: red;
  margin-top: 20px;
  color: white;
}
<button onclick="toggleDiv()">Try me</button>

<div id="firstimage" class='image'>
  DIV element.
</div>

<div id="secondimage" class='image'>
  A second DIV element.
</div>

<div id="thirdimage" class='img'>
  A third DIV element.
</div>

In this function, just using an "array-like" object such as a NodeList demonstrated in the Snippet above. An array would be used in the same manner as it is in the Snippet. Should you want to do more advanced processing of the divs such as running a function on each of them and returned then converting an "array-like" object into an array would be necessary to run methods like map, forEach, slice, etc.

Appointive answered 27/8, 2016 at 6:49 Comment(4)
This is helpful. However, the goal of my code is to stop displaying images one and two, while allowing image 3 to remain displayed and move into their place.Narcis
I don't recall that in the question, but I'll update my answer to comply...Appointive
You are right, I will go back and include that, thanks for the catch and the help!.Narcis
@JackElwell ok, done.Appointive
O
10

document.getElementsByClassName returns an array of elements, so you would need to iterate through that array and operate on each element within that loop.

Outleap answered 27/8, 2016 at 6:27 Comment(7)
You may also be interested in document.querySelectorAllOutleap
It returns an array-like object of all child elements which is different from array of elementsLotic
@user2181397 - Yup. It returns an HTMLCollection. If you add the following line to your JS, you can still use the forEach method of arrays on them, which makes them quite a bit nicer to use. HTMLCollection.prototype.forEach = Array.prototype.forEach;Trudeau
@Trudeau modifying the prototypes of native objects is a horrible idea. Why not just use [].slice.call(document.getElementsByClassName('foobar')).forEach()? Better yet, use document.querySelector(), which will actually do what the OP wantsMonetary
@TinyGiant - because I'm yet to have someone extol to me the virtues of the more verbose approach over the one I've suggested. A quick read after seeing your comment indicates the problem to lie with the possibility of things breaking in the future. This seems little, if any, different to adding custom attributes to HTML elements, which then go on to become standardised. autosuggest and lang both spring (perhaps erroneously) to mind. This is why it's considered a horrible idea?Trudeau
@Trudeau Many problems can arise from extending native prototypes. This has been discussed to death countless times. There is plenty of information out there. If you refuse to acknowledge that information and insist on breaking things, that is your prerogative. If there is code other than your code on a page, and that code expects the native object to be a certain way, your code will interfere.Monetary
@TinyGiant - thank-you, I really do appreciate it. I've no recollection of hearing recommendations against it before your comment. Since I always get the whole page to myself, I hadn't even considered the "doesn't play nicely with others" part of the problem. I'm not sure if you're being snarky by implying that I've heard the info and ignored it, or if I've misinterpreted your words. In any case, it's news to me and I'm glad to have it. Fortunately, I've not received an angry phone-call because something has broken. Thanks for your assistance in avoiding them. That's today's biggest lesson. :)Trudeau
A
4

You should use getElementsByClassName() or querySelectorAll() to collect all div.Klass (Klass being an arbitrary name). The following Snippet uses querySelectorAll() details are commented in source.

SNIPPET

function toggleDiv() {
  // Collect all .image into a NodeList
  var xs = document.querySelectorAll(".image");
  // Declare i and qty for "for" loop
  var i, qty = xs.length;
  // Use "for" loop to iterate through NodeList
  for (i = 0; i < qty; i++) {
    // If this div.image at index [i] is "none"...
    if (xs[i].style.display === 'none') {
      // then make it "block"... 
      xs[i].style.display = 'block';
    } else {
      // otherwise set display to "none"
      xs[i].style.display = 'none';
    }
  }
}
#firstimage {
  width: 100px;
  height: 100px;
  padding: 0px 0;
  text-align: center;
  background-color: green;
  margin-top: 20px;
  color: white;
}
#secondimage {
  width: 100px;
  height: 100px;
  padding: 0px 0;
  text-align: center;
  background-color: blue;
  margin-top: 20px;
  color: white;
}
#thirdimage {
  width: 100px;
  height: 100px;
  padding: 0px 0;
  text-align: center;
  background-color: red;
  margin-top: 20px;
  color: white;
}
<button onclick="toggleDiv()">Try me</button>

<div id="firstimage" class='image'>
  DIV element.
</div>

<div id="secondimage" class='image'>
  A second DIV element.
</div>

<div id="thirdimage" class='img'>
  A third DIV element.
</div>

In this function, just using an "array-like" object such as a NodeList demonstrated in the Snippet above. An array would be used in the same manner as it is in the Snippet. Should you want to do more advanced processing of the divs such as running a function on each of them and returned then converting an "array-like" object into an array would be necessary to run methods like map, forEach, slice, etc.

Appointive answered 27/8, 2016 at 6:49 Comment(4)
This is helpful. However, the goal of my code is to stop displaying images one and two, while allowing image 3 to remain displayed and move into their place.Narcis
I don't recall that in the question, but I'll update my answer to comply...Appointive
You are right, I will go back and include that, thanks for the catch and the help!.Narcis
@JackElwell ok, done.Appointive

© 2022 - 2024 — McMap. All rights reserved.