0

I usually write in Python on the back-end and am not seasoned on JS. This code looks pretty nasty to me as it repeats itself a lot. If I were writing in Python I think I would make a list of objects then loop over the list of objects to add the active class if one of the objects matches the URL.

Can I do that with JS? Or what is a good way to optimize this from someone who writes on the front-end normally?

JS:

var url = window.location.pathname;

var overview = document.getElementById("overview");
var cats = document.getElementById("categories");
var videos = document.getElementById("videos");
var tests = document.getElementById("tests");
var memos = document.getElementById("memos");

var overviewBtm = document.getElementById("overview-btm");
var videosBtm = document.getElementById("videos-btm");
var testsBtm = document.getElementById("tests-btm");
var memosBtm = document.getElementById("memos-btm");
var profileBtm = document.getElementById("profile-btm");

var superUsersBtm = document.getElementById("super-users-btm");
var superResultsBtm = document.getElementById("super-results-btm");
var superNewBtm = document.getElementById("super-new-btm");
var superInviteBtm = document.getElementById("super-invite-btm");
var superProfileBtm = document.getElementById("super-profile-btm");

if (url == '/overview/'){
    console.log(url);
    if (overview){
    overview.classList.add('nav-active');
    } else if (overviewBtm){
    overviewBtm.classList.add('nav-active');
    }
} else if (url == '/categories/'){
    console.log(url);
    if (cats){
    cats.classList.add('nav-active');
    }
} else if (url == '/videos/'){
    console.log(url);
    if (videos){
    videos.classList.add('nav-active');
    } else if (videosBtm){
    videosBtm.classList.add('nav-active');
    }
} else if (url == '/tests/'){
    console.log(url);
    if (tests){
    tests.classList.add('nav-active');
    } else if(testsBtm){
    testsBtm.classList.add('nav-active');
    }
} else if (url == '/memos/'){
    console.log(url);
    if (memos){
    memos.classList.add('nav-active');
    } else if (memosBtm){
    memosBtm.classList.add('nav-active');
    }
} else if (url == '/profile/'){
    console.log(url);
    if (profileBtm){
    profileBtm.classList.add('nav-active');
    } else if (superProfileBtm){
    superProfileBtm.classList.add('nav-active');
    }
} else if (url == '/tests/results/'){
    console.log(url);
    if (superResultsBtm){
    superResultsBtm.classList.add('nav-active');
    }
} else if (url == '/accounts/'){
    console.log(url);
    if (superUsersBtm){
    superUsersBtm.classList.add('nav-active');
    }
} else if (url == '/invite/'){
    console.log(url);
    if (superInviteBtm){
    superInviteBtm.classList.add('nav-active');
    }
} else if (url == null){
    // pass
}

Edit: Snippet of HTML in the base.html:

<div class="header-icon-container d-xs-none">
    <a class="nav-item nav-link d-xs-none" href="{% url 'overview' %}" id="
        <i class="fas fa-home fa-2x"></i>
        <span>Home</span>
    </a>
</div>
<div class="header-icon-container">
    <a class="nav-item nav-link" href="#" id="drills">
        <i class="fas fa-dumbbell fa-2x"></i>
        <span>Drills</span>
    </a>
</div>
<div class="header-icon-container d-xs-none">
    <a class="nav-item nav-link" href="{% url 'tests' %}" id="tests">
        <i class="fas fa-graduation-cap fa-2x"></i>
        <span>Testing</span>
    </a>
</div>
<div class="header-icon-container">
    <a class="nav-item nav-link" href="{% url 'videos' %}" id="videos">
        <i class="fas fa-video fa-2x"></i>
        <span>Videos</span>
    </a>
</div>
<div class="header-icon-container">
    <a class="nav-item nav-link" href="{% url 'memos' %}" id="memos">
        <i class="fas fa-paper-plane fa-2x"></i>
        <span>Memos</span>
    </a>
</div>
David Alford
  • 2,044
  • 3
  • 12
  • 35
  • 1
    Assuming this is about highlighting the current page in the navigation, there's probably a way to simply compare the `href` of the `` to the current URL, but we'd have to see the relevant HTML for that. And why doesn't Django do this on the backend? –  Feb 12 '20 at 22:26
  • The reason I have it written like this is I have a `base.html` which contains the navigation. While I could have a new navigation bar on every page which extends the `base.html` page it seems more logical to simply make one nav in the `base.html` then add an active class to it using JS. I might be wrong here. Always open to suggestions. I will update the post with the HTML. – David Alford Feb 12 '20 at 22:31
  • What is with the `else if` statements ? Are those there because when `#overview` is on the page `#overview-btm` isn't and the other way around ? – Titus Feb 12 '20 at 22:36
  • Yeah, this code was written very quickly and I threw `if` or `else` statements wherever I needed to check if the item exists rather than digging through the code to make sure each one exists in each scenario (was trying to avoid unnecessary bug chasing). – David Alford Feb 12 '20 at 22:39
  • Fwiw I found a better way to handle this using the Django backend (suggested by Chris G.) [here](https://stackoverflow.com/questions/9793576/how-to-render-menu-with-one-active-item-with-dry) but I am still interested in knowing what the optimized version of this would look like or how front-end devs would handle this. – David Alford Feb 12 '20 at 22:40

1 Answers1

2

Well, im not sure what are these overview-btm, super-profile-btm and stuff. But for the links, it is simple:

const links = document.querySelectorAll('.nav-item.nav-link');
links.forEach((link) => {
    if (link.pathname === window.location.pathname) {
        link.classList.add('nav-active');
    }
})
m51
  • 1,950
  • 16
  • 25
  • They are for different (mobile/supervisor/footer) menus and can be disregarded. This is the type of thing I was looking for. Thanks! – David Alford Feb 12 '20 at 22:53