CODEWITHSUNDEEP
jqueryperformancenavigationscrollviewport
Sebsemillia
Sebsemillia

Reputation: 9486

jQuery improve performance of function

I wrote a custom script, which checks the current viewport position and then accordingly highlights the current item in the fixed header navigation.

The order of the navigation items is the same as the order of the offset variables declaration.

I call this function on scroll(), but I got the feeling this not the best performance wise, since it checks all the time when a user scrolls. Maybe a better approach would be for example to check at the end of the scroll event? But I couldn't find a way to implement that.

Do you have any idea how I could improve the performance of the functionality?

Here is my code:

$window = $(window);

$window.scroll(function() {
            activeState();
        }
    );

function activeState() {
        var offsetHome = $('#Home').position().top;
        var offsetLeistungen = $('#Leistungen').position().top;
        var offsetPortfolio = $('#Portfolio').position().top;
        var offsetUeber = $('#Ueber').position().top;
        var offsetKontakt = $('#Kontakt').position().top;
        var headerHeight = $('.header').height();
        $('#PrimNav li').removeClass('active');
        if($window.scrollTop() <= offsetLeistungen - headerHeight) {
            $('#PrimNav li:nth-child(1)').addClass('active');
        } else  if($window.scrollTop() <= offsetPortfolio - headerHeight) {
            $('#PrimNav li:nth-child(2)').addClass('active');
        } else  if($window.scrollTop() <= offsetUeber - headerHeight) {
            $('#PrimNav li:nth-child(3)').addClass('active');
        } else  if($window.scrollTop() <= offsetKontakt - headerHeight) {
            $('#PrimNav li:nth-child(4)').addClass('active');
        } else {
            $('#PrimNav li:nth-child(5)').addClass('active');
        }
    };

Thank you!

Upvotes: 0

Views: 102

Answers (1)

zfor
zfor

Reputation: 383

If the elements' position is fixed, then you can move the declaration out of your activateState() function, because every time the scroll event fires it declares those variables again which is not really efficient. You also don't have to get $(window).scrollTop() value in every if statement. Calculate it once on the scroll event and pass its value to activateState();

Updated:

var offsetHome = 0;
var offsetLeistungen = 0;
var offsetPortfolio = 0;
var offsetUeber = 0;
var offsetKontakt = 0;
var headerHeight = 0;
$window = $(window);

$window.scroll(function() {
            activeState($window.scrollTop());
        }
    );

function activeState(scrollTop) {
       offsetHome = $('#Home').position().top;
       offsetLeistungen = $('#Leistungen').position().top;
       offsetPortfolio = $('#Portfolio').position().top;
       offsetUeber = $('#Ueber').position().top;
       offsetKontakt = $('#Kontakt').position().top;
       headerHeight = $('.header').height();
        $('#PrimNav li').removeClass('active');
        if(scrollTop <= offsetLeistungen - headerHeight) {
            $('#PrimNav li:nth-child(1)').addClass('active');
        } else  if(scrollTop <= offsetPortfolio - headerHeight) {
            $('#PrimNav li:nth-child(2)').addClass('active');
        } else  if(scrollTop <= offsetUeber - headerHeight) {
            $('#PrimNav li:nth-child(3)').addClass('active');
        } else  if(scrollTop <= offsetKontakt - headerHeight) {
            $('#PrimNav li:nth-child(4)').addClass('active');
        } else {
            $('#PrimNav li:nth-child(5)').addClass('active');
        }
    };

Upvotes: 1

Related Questions