getElementsByClassName doesn't select all my Navigation elements

Question

I'm creating a Website with HTML, CSS, and JavaScript (no jQuery).

I have created the following script to change my Navigation Class when my pageoffset is more than 50 and change it back if its less then 50:

window.onscroll = function (event) {
var nav = document.getElementsByClassName('main-navigation');
var navscr = document.getElementsByClassName('main-navigation-scrolled');
 if (window.pageYOffset > 50) {
    for(var i = 0; i < nav.length; i++) {
    nav[i].className = 'main-navigation-scrolled';
    }
 }
 else {
 if (window.pageYOffset < 50) {
    for(var i = 0; i < navscr.length; i++) {
    navscr[i].className = 'main-navigation';
    }
 }
 }
}

For some reason, when I scroll very slowly or reload the page when my offset is more than 50 only half the li-elements change class.

Maybe there is a smarter solution which also has better performance?

This is my first question, go easy on me please :)

€dit: HTML

<div id="nav-menu-container-fix">
<ul>
  <li><a class="main-navigation" href="index.html">Home</a></li>
  <li><a class="main-navigation" href="about.html">About</a></li>
  <li><a class="main-navigation" href="#">Team</a></li>
  <li><a class="main-navigation" href="#">24 Weeks</a></li>
  <li><a class="main-navigation" href="#">Donate</a></li>
  <li><a class="main-navigation" href="#">Downloads</a></li>
  <li><a class="main-navigation" href="#">Forum</a></li>
  </ul> 
</div>

Aaaaand CSS

 a.main-navigation {
 padding:18px 15px 15px 15px;
 background-color:#222222;
 color:#bbbbbb;
 display:inline-block;
 text-decoration:none;
  -webkit-transition: all 600ms ease;
 -moz-transition: all 600ms ease;
 -ms-transition: all 600ms ease;
 -o-transition: all 600ms ease;
 transition: all 600ms ease;
 }

 a.main-navigation:hover {
 padding:18px 15px 15px 15px;
 background-color:#555555;
 color:#ffffff;
 display:inline-block;
 text-decoration:none;
  -webkit-transition: all 600ms ease;
 -moz-transition: all 600ms ease;
 -ms-transition: all 600ms ease;
 -o-transition: all 600ms ease;
 transition: all 600ms ease;
 }

 a.main-navigation-scrolled {
 padding:7.5px 15px 7.5px 15px;
 background-color:#604D9D;
 color:#eeeeee;
 display:inline-block;
 text-decoration:none;
  -webkit-transition: all 600ms ease;
 -moz-transition: all 600ms ease;
 -ms-transition: all 600ms ease;
 -o-transition: all 600ms ease;
 transition: all 600ms ease;
 }

 a.main-navigation-scrolled:hover {
 padding:7.5px 15px 7.5px 15px;
 background-color:#402c6c;
 color:#ffffff;
 display:inline-block;
 text-decoration:none;
  -webkit-transition: all 400ms ease;
 -moz-transition: all 600ms ease;
 -ms-transition: all 600ms ease;
 -o-transition: all 600ms ease;
 transition: all 600ms ease;
 }

That’s probably because of the _next_ scroll event firing and overwriting what you have done in handling the first one … — CBroe, Mar 18 '14 at 12:44

T J · Answer 1 · 2014-03-18T16:05:36.103

0

Scroll event will be called hundreds of times while you are scrolling. so you need to perform the actions one and only once when the conditions are matched.

the following will reduce dom accessing

var scrolled = false; // initially page is not scrolled

window.onscroll = function (event) {
    if (window.pageYOffset > 50 && !scrolled) { //perform following only if it's not done already
        var nav = document.querySelectorAll('main-navigation');
        for (var i = 0; i < nav.length; i++) {
            nav[i].className = 'main-navigation-scrolled';
            scrolled = true; // applied scroll class no need to do this again
        }
    } else if (window.pageYOffset < 50 && scrolled) { //perform the following only it's not done already
        var navscr = document.querySelectorAll('main-navigation-scrolled');
        for (var i = 0; i < navscr.length; i++) {
            navscr[i].className = 'main-navigation';
            scrolled = false; // applied no scroll class, no need to do it again
        }
    }
}

Update: working fiddle

edited Mar 18 '14 at 16:05

answered Mar 18 '14 at 12:52

T J

42,762
13
83
138

Hey, this look really legit. I can't seem to get this to work though. Will keep trying and report back. – DeeL Mar 18 '14 at 13:16
Me again, Just doesn't seem to work. Check this JSFiddle out, do you see why this doesnt work? http://jsfiddle.net/ftR6p/ – DeeL Mar 18 '14 at 13:28
@DeeL check this fiddle. http://jsfiddle.net/FxrvL/2/ it seems something is going wrong with the class selector! – T J Mar 18 '14 at 13:44
I would, but unfortunately this fiddle doesn't work for me at all :( – DeeL Mar 18 '14 at 14:16
@DeeL that's because there's no scroll, reduce the window size or open console to see the effect.. or add some more elements to create scroll – T J Mar 18 '14 at 14:27
I probably understand that i have to scroll to see the effect when im asking this question x) Could be a problem of PC i will try at home again. – DeeL Mar 18 '14 at 14:28

score 0 · Accepted Answer · answered Mar 19 '14 at 08:59

0

I don't really know why it didn't work, but I know it works now. I started using jquery and with the following code it works just fine.

$(document).scroll(function () {
    if (window.scrollY > 50) {
        $(".main-navigation").attr('class', 'main-navigation-scrolled');
    } else {
        $(".main-navigation-scrolled").attr('class', 'main-navigation');
    }
});

answered Mar 19 '14 at 08:59

DeeL

41
5

2

possibly the reason for this is: document.getElementsByTagName(), document.getElementsByTagNameNS and document.getElementsByClassName() returns “live” node lists. http://darcyclarke.me/development/live-vs-static-node-lists/. document.querySelectorAll(), was developed and standardized to return a static node list – Mar 19 '14 at 09:09

getElementsByClassName doesn't select all my Navigation elements

2 Answers2

Linked