Emma Earl Kent
Emma Earl Kent

Reputation: 578

How can i refactor my JS to use fewer if else statements and be less repetitive

The issue i'm facing is this code feels way too repetitive/heavy. I would like to simplify this or refactor to make it better, any suggestions welcome. I am still JS beginner, so thank you for input and explanations.

The goal of this code is to change the position of my side navigation based on where the user is on the page. Here's the code:

/** Position for side navigation when page has hero carousel AND subnav */
if (heroCarousel && subNav) {
    // Changes position for side navigation based on user scroll
    let positionChanges = function positionChangesOnScroll() {
        // Define variables
        var toTop = $(window).scrollTop(),
            elementOffset = $('footer').offset().top,
            distance = (elementOffset - toTop),
            jumpNav = $('.jn-publish');
        // Bahavior for side navigation on page load
        if (toTop < 200 ) {
            // Adding class for first position
            jumpNav.addClass('pos1-carousel-subnav');
        }
        // Bahavior for side navigation immediatly after sub navigation sticks
        if (toTop >= 200 && toTop < 330 ) {
            // Adding class for second position
            jumpNav.addClass('pos2-carousel-subnav');
        } else {
            jumpNav.removeClass('pos2-carousel-subnav');
        }
        // Behavior for side navigation (fixed) when page content scrolss
        if (toTop >= 330 && distance > 380) {
            // Adding class for fixed position
            jumpNav.addClass('pos3-carousel-subnav');
        } else {
            jumpNav.removeClass('pos3-carousel-subnav');
        }
        // Stop fixed position at footer
        if (distance <= 380) {
            // Adding class for fadding out nav at footer position
            jumpNav.addClass('pos4-carousel-subnav');
        } else {
            jumpNav.removeClass('pos4-carousel-subnav');
        }
    };
    /** Executes functions during events 'load' and 'scroll,' */
    window.addEventListener('load', positionChanges);
    window.addEventListener('scroll', positionChanges);
}

Upvotes: 0

Views: 46

Answers (1)

CertainPerformance
CertainPerformance

Reputation: 370689

You can use toggleClass instead of conditionally using addClass or removeClass: pass a second argument to indicate whether the class should be added or removed. This:

if (toTop >= 200 && toTop < 330 ) {
    // Adding class for second position
    jumpNav.addClass('pos2-carousel-subnav');
} else {
    jumpNav.removeClass('pos2-carousel-subnav');
}
// Behavior for side navigation (fixed) when page content scrolss
if (toTop >= 330 && distance > 380) {
    // Adding class for fixed position
    jumpNav.addClass('pos3-carousel-subnav');
} else {
    jumpNav.removeClass('pos3-carousel-subnav');
}
// Stop fixed position at footer
if (distance <= 380) {
    // Adding class for fadding out nav at footer position
    jumpNav.addClass('pos4-carousel-subnav');
} else {
    jumpNav.removeClass('pos4-carousel-subnav');
}

can be

jumpNav.toggleClass('pos2-carousel-subnav', toTop >= 200 && toTop < 330);
jumpNav.toggleClass('pos3-carousel-subnav', toTop >= 330 && distance > 380);
jumpNav.toggleClass('pos4-carousel-subnav', distance <= 380);

This same shortcut works with the native classList.toggle method as well as in jQuery with toggleClass.

Upvotes: 5

Related Questions