Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Change CSS based on scroll position -- Refactoring bad code

I have written a jQuery function that changes the css value of a nav menu item based on if its reference section is viewable in the window.

$(window).scroll(function() {

    var scroll = $(window).scrollTop();

    if (scroll <= 590) {
        $("#menu-item-25 a").addClass('blue');
        $("#menu-item-26 a").removeClass('blue');
        $("#menu-item-22 a").removeClass('blue');
        $("#menu-item-23 a").removeClass('blue');
        $("#menu-item-24 a").removeClass('blue');
    }
    else if (scroll >= 591 && scroll <= 1380) {
        $("#menu-item-26 a").addClass('blue');
        $("#menu-item-25 a").removeClass('blue');
        $("#menu-item-22 a").removeClass('blue');
        $("#menu-item-23 a").removeClass('blue');
        $("#menu-item-24 a").removeClass('blue');
    }
    else if (scroll >= 1381 && scroll <= 2545) {
        $("#menu-item-22 a").addClass('blue');
        $("#menu-item-25 a").removeClass('blue');
        $("#menu-item-26 a").removeClass('blue');
        $("#menu-item-23 a").removeClass('blue');
        $("#menu-item-24 a").removeClass('blue');
    }
    else if (scroll >= 2546 && scroll <= 2969) {
        $("#menu-item-23 a").addClass('blue');
        $("#menu-item-25 a").removeClass('blue');
        $("#menu-item-26 a").removeClass('blue');
        $("#menu-item-22 a").removeClass('blue');
        $("#menu-item-24 a").removeClass('blue');
    }
    else if (scroll >= 2970) {
        $("#menu-item-24 a").addClass('blue');
        $("#menu-item-25 a").removeClass('blue');
        $("#menu-item-26 a").removeClass('blue');
        $("#menu-item-22 a").removeClass('blue');
        $("#menu-item-23 a").removeClass('blue');
    }
});

It looks awfully ugly. Is there a better way to achieve this goal?

like image 263
Seth Johnson Avatar asked Jan 09 '23 12:01

Seth Johnson


1 Answers

All the previous answers will work just fine, because you have multiple ways to make this better just making some changes to your CSS selectors, but if you will do all this calculations in the scroll event, you should read this John Resign Post about how to deal with scroll event, specially this part:

It’s a very, very, bad idea to attach handlers to the window scroll event. Depending upon the browser the scroll event can fire a lot and putting code in the scroll callback will slow down any attempts to scroll the page (not a good idea). Any performance degradation in the scroll handler(s) as a result will only compound the performance of scrolling overall. Instead it’s much better to use some form of a timer to check every X milliseconds OR to attach a scroll event and only run your code after a delay (or even after a given number of executions – and then a delay).
- John Resign

So, in your case I would go like this:

HTML:

<div class="menu">
    <a id="menu-item-22">Link 1</a>
    <a id="menu-item-23">Link 2</a>
    <a id="menu-item-24">Link 3</a>
    <a id="menu-item-25">Link 4</a>
    <a id="menu-item-26">Link 5</a>
</div>

Jquery:

var didScroll = false;

$(window).scroll(function() {
    didScroll = true;
});

setInterval(function() {
    if ( didScroll ) {
        didScroll = false;
        var $el;

        //Same that all the if else statements
        switch (true) {
            case (scroll >= 591 && scroll <= 1380):
                $el = $("#menu-item-26 a");
                break;
            case (scroll >= 1381 && scroll <= 2545):
                $el = $("#menu-item-22 a");
                break;
            case (scroll >= 2546 && scroll <= 2969):
                $el = $("#menu-item-23 a");
                break;
            case (scroll >= 2970):
                $el = $("#menu-item-24 a");
                break;
            default: //scroll<=590
                $el = $("#menu-item-25 a");
        }

        //Removing blue class from all links
        $('.menu a').removeClass('blue');

        //Adding blue class to the matched element
        $el.addClass('blue');
    }
}, 50);
like image 69
Tom Sarduy Avatar answered Jan 11 '23 02:01

Tom Sarduy