3
\$\begingroup\$

This script creates a hamburger menu when the window is resized. It will also create a sticky navigation when the window is scrolled down, which will also create a hamburger menu when the window is resized.

I'm looking for advice on how I can best structure the Javascript in my code.

// When the user scrolls the page, execute myFunction
window.onscroll = function() { stickyNav(); };

// Get the navbar and the logo
let nav = document.getElementById('nav');
let logo = document.getElementById('logo');

// Get the offset position of the navbar
let sticky = nav.offsetTop;

let fixedNavContainer = 'fixed-nav-container';
let fixedNav = 'fixed-nav';

// create wrapper container
let createFixedNavContainer = document.createElement('div');
createFixedNavContainer.className = fixedNavContainer;

let createFixedNav = document.createElement('div');
createFixedNav.className = fixedNav;

function createIcon() {
    let createButton = document.createElement('button');
    let getthenav = document.querySelectorAll('#nav')[0];
    let createIcon = document.createElement('i');

    getthenav.insertBefore(createButton, getthenav.firstChild);

    createButton.appendChild(createIcon);
    
    createButton.setAttribute('class', 'icon');
    createIcon.setAttribute('class', 'fa fa-bars');
}
createIcon();

// Add the sticky class to the navbar when you reach its scroll position. Remove "sticky" when you leave the scroll position
function stickyNav() {
    if (window.pageYOffset >= sticky) {

        logo.removeAttribute('id');
        logo.setAttribute('id', 'sticky-logo');
        nav.removeAttribute('id');
        nav.setAttribute('id', 'sticky-nav');

        // Wrap the nav and logo into fixed div
        if (document.getElementsByClassName(fixedNavContainer).length === 0) {
            logo.parentNode.insertBefore(createFixedNavContainer, logo);
            nav.parentNode.insertBefore(createFixedNavContainer, nav);
    
            createFixedNavContainer.appendChild(logo);
            createFixedNavContainer.appendChild(nav);
        }

        // Wrap the nav and logo into a div that can be centered
        if (document.getElementsByClassName(fixedNav).length === 0) {
            logo.parentNode.insertBefore(createFixedNav, logo);
            nav.parentNode.insertBefore(createFixedNav, nav);
            
            createFixedNav.appendChild(logo);
            createFixedNav.appendChild(nav);
        }
        
        let getStickyNav = document.getElementById('sticky-nav');
        let divHeight = document.getElementsByClassName(fixedNavContainer)[0].offsetHeight;
        
        if (getStickyNav !== null) {
            console.log(nav);
            let getNavIcon = document.querySelectorAll('button.icon')[0];
            getNavIcon.onclick = function() { makeStickyNavResponsive(divHeight, event); };
        }

    } else {
        logo.removeAttribute('id');
        logo.setAttribute('id', 'logo');
        nav.removeAttribute('id');
        nav.setAttribute('id', 'nav');
    
        // Unwrap the nav and logo from the fixed div
        let getFixedNavContainer = document.querySelector('div.' + fixedNavContainer);
        if (getFixedNavContainer !== null) {
            let parent = getFixedNavContainer.parentNode;
            while (getFixedNavContainer.firstChild) parent.insertBefore(getFixedNavContainer.firstChild, getFixedNavContainer);
            parent.removeChild(getFixedNavContainer);
        }
        
        // Unwrap the nav and logo from the div that can be centered
        let wrap_cent = document.querySelector('div.' + fixedNav);
        if (wrap_cent !== null) {
            let parent_cent = wrap_cent.parentNode;
            while (wrap_cent.firstChild) parent_cent.insertBefore(wrap_cent.firstChild, wrap_cent);
            parent_cent.removeChild(wrap_cent);
        }
    }
}

let getNav = document.getElementById('nav');
let getNavIcon = document.querySelectorAll('button.icon')[0];
getNavIcon.onclick = function() { makeNavResponsive(getNav, event); };

function makeNavResponsive(getNav, event) {
    event.stopPropagation();
    if (getNav.id === 'nav') {
        if (getNav.className.length === 0) {
            getNav.setAttribute('class', 'responsive');
        } else if (getNav.className === 'responsive') {
            getNav.removeAttribute('class');
        }
        window.onclick = function() { removeNav(getNav, event); };
    }
}

// Make Sticky Nav Responsive
function makeStickyNavResponsive(divHeight, event) {
    event.stopPropagation();

    let getStickyNav = document.getElementById('sticky-nav');

    if (getStickyNav !== null && getStickyNav.id === 'sticky-nav') {
        if (getStickyNav.className.length === 0) {
            getStickyNav.setAttribute('class', 'responsive');
            let navMargin = document.querySelectorAll('#sticky-nav.responsive ul');

            if (typeof(navMargin) != 'undefined' && navMargin !== null && navMargin.length != 0) {
                navMargin[0].style.top = divHeight.toString() + 'px';
            } else {
                navMargin[0].style.top = '0px';
            }
        } else if (getStickyNav.className === 'responsive') {
            getStickyNav.removeAttribute('class');
    
            let navMargin = document.querySelectorAll('#sticky-nav ul');
            navMargin[0].style.top = '0px';
        }
        window.onclick = function() { removeStickyNav(getStickyNav, event); };
    } else {
        makeNavResponsive(getNav, event);
    }
}

function removeNav(getNav, event) {
    event.stopPropagation();
    if (getNav.className === 'responsive') {
        getNav.removeAttribute('class');
    }
}
function removeStickyNav(getStickyNav, event) {
    event.stopPropagation();
    if (getStickyNav.className === 'responsive') {
        getStickyNav.removeAttribute('class');
    }
}
html {
    font-family: Inter, system-ui, sans-serif;
}
#logo {
    text-align: center;
    margin: 50px 0px 50px 0px;
}
#nav {
    background-color: #343A40;
    width: 35%;
    display: flex;
    justify-content: center;
    margin: 0 auto;
    overflow: hidden;
    border-radius: 5px;
}
#nav ul {
    margin: 0px;
    padding: 0px;
}
#nav ul li {
    list-style: none;
    display: inline-block;
    padding: 14px 0px 14px 0px;
}
#nav li a {
    color: #f2f2f2;
    text-align: center;
    padding: 14px 16px;
    text-decoration: none;
    font-size: 17px;
}
#nav li a:hover {
    background-color: #6C757D;
    color: #fff;
}
#nav li a.active {
    background-color: #495057;
    color: #fff;
}
#nav .icon {
  display: none;
}
.fixed-nav-container {
    width: 100%;
    background: #343A40;
    position: fixed;
    top: 0;
    left: 0;
}
.fixed-nav {
    max-width: 900px;
    min-height: 65px;
    display: flex;
    justify-content: space-around;
    align-items: center;
    margin-left: auto;
    margin-right: auto;
}
#sticky-logo {
    margin: 15px 0px 15px 25px;
    width: 20%;
    font-size: 22px;
    font-weight: 600;
    letter-spacing: 0px;
    color: #fff;
}
#sticky-nav {
    width: 75%;
    display: flex;
    justify-content: flex-end;
}
#sticky-nav ul {
    margin: 0px 15px 0px 0px;
    padding: 0px;
}
#sticky-nav ul li {
    list-style: none;
    display: inline-block;
    padding: 14px 0px 14px 0px;
}
#sticky-nav li a {
    color: #f2f2f2;
    text-align: center;
    padding: 8px 16px;
    text-decoration: none;
    font-size: 17px;
}
#sticky-nav li a:hover {
    background-color: #6C757D;
    color: #fff;
    border-radius: 5px;
}
#sticky-nav li a.active {
    background-color: #495057;
    color: #fff;
    border-radius: 5px;
}
#sticky-nav button.icon {
    display: none;
}
.image-container {
    display: flex;
    justify-content: center;
    margin: 50px 0px 50px 0px;
}
.responsive-image {
    width: 100%;
    max-width: 900px;
    height: auto;
}
.article-wrap {
    width: 35%;
    margin: 0 auto 50px auto;
    font-size: 18px;
}
.article-wrap p {
    margin: 50px 0px 0px 0px;
}
@media screen and (max-width: 1000px) {
    #wrap-cont {
        width: 75%;
        background: red;
        position: sticky;
        margin: 0 auto 0 auto;
    }
    #logo {
        width: 50%;
        float: left;
        text-align: left;
        margin: 50px 0px 50px 0px;
    }
    #nav {
        position: absolute;
        top: 45px;
        right: 0;
        background: none;
        min-width: 300px;
        min-height: 50px;
    }
    /*
    #nav:not(.responsive) ul li:not(:first-child) {
        display: none;
    }*/
    #nav:not(.responsive) ul li {
        display: none;
    }
    #nav button.icon {
        display: block;
        background-color: #343A40;
        position: absolute;
        top: 0;
        right: 0;
        border-radius: 5px;
        padding: 14px 16px;
        border: 0px;
        color: #fff;
    }
}
@media screen and (max-width: 1000px) {
    #nav.responsive {
        background: none;
        min-width: 300px;
        position: absolute;
        top: 45px;
        right: 0;
        border-radius: 5px;
    }
    #nav.responsive button.icon {
        display: block;
        background-color: #343A40;
        position: absolute;
        top: 0;
        right: 0;
        border-radius: 5px 5px 0px 0px;
        padding: 14px 16px;
        border: 0px;
        color: #fff;
    }
    #nav.responsive a { 
        float: none;
        display: block;
        text-align: left;
    }
    #nav.responsive ul {
        min-width: 300px;
        margin: 44px 0px 0px 0px;
        overflow: hidden;
        background-color: #343A40;
        border-radius: 5px 0px 5px 5px;
        display: flex;
        flex-direction: column;
    }
    #nav.responsive ul li {
        margin: 0;
        padding: 0;
    
    }
    /*
    #nav.responsive ul li:first-child {
        order: 1;
    }
    #nav.responsive ul li:not(:last-child)  {
        width: 100%;
        order: 2;
    }*/
}
@media screen and (max-width: 600px) {
#sticky-logo {
    margin: 15px 0px 15px 25px;
    width: 20%;
    font-size: 22px;
    font-weight: 600;
    letter-spacing: 0px;
    color: #fff;
}
/*
#sticky-nav {
    width: 75%;
    display: flex;
    justify-content: flex-end;
}

    #sticky-nav:not(.responsive) ul li:not(:last-child) {
        display: none;
    }
    #sticky-nav ul li a.icon {
        display: block;
        background-color: #343A40;
        border-radius: 5px;
    }
    #sticky-nav ul li:last-child {
        display: block;
}*/
    #sticky-nav:not(.responsive) ul li {
            display: none;
        }
    #sticky-nav button.icon {
        display: block;
        background-color: #343A40;
        border-radius: 5px;
        margin: 0px 25px 0px 0px;
        padding: 14px 16px;
        border: 0px;
        color: #fff;
        font-size: 18px;
    }
}
@media screen and (max-width: 600px) {
    #sticky-nav.responsive {
     width: 75%;
    display: flex;
    justify-content: flex-end;
}
    #sticky-nav.responsive button.icon {
        display: block;
        background-color: #343A40;
        border-radius: 5px;
        margin: 0px 40px 0px 0px;
        padding: 14px 16px;
        border: 0px;
        color: #fff;
        font-size: 18px;
    }
    #sticky-nav.responsive a { 
        float: none;
        display: block;
        text-align: left;
    }
    #sticky-nav.responsive ul {
        min-width: 200px;
        margin: 0;
        padding: 0;
        background-color: #343A40;
        display: flex;
        align-items: flex-start;
        flex-direction: column;
        position: absolute;
    }
    #sticky-nav.responsive ul li {
        width: 100%;
        padding: 0px;
    }
    #sticky-nav.responsive li a {
        padding: 14px 18px;
    }
    /*
    #sticky-nav.responsive ul li:first-child {
        order: 1;
        background: red;
    }
    #sticky-nav.responsive ul li:not(:last-child)  {
        min-width: 300px;
        order: 2;
    }
    #sticky-nav.responsive ul li:last-child {
        display: block;
}*/
    #sticky-nav.responsive ul li a:hover {
    background-color: #6C757D;
    color: #fff;
    border-radius: 0px;
}
#sticky-nav.responsive ul li a.active {
    background-color: #495057;
    color: #fff;
    border-radius: 0px;
}
}
@media screen and (max-width: 1020px) {
    .article-wrap {
        width: 75%;
        clear: both;
    }
}
<div id="wrap-cont">
    <h1 id="logo">Navigation</h1>
    <div id="nav">
        <ul>
            <li><a href="" class="active">Home</a></li>
            <li><a href="">About</a></li>
            <li><a href="">Blog</a></li>
            <li><a href="">News</a></li>
            <li><a href="">FAQ</a></li>
            <li><a href="">Careers</a></li>
            <li><a href="">Contact Us</a></li>
        </ul>
    </div>
</div>

\$\endgroup\$
1
  • \$\begingroup\$ How would you describe what problem this code is meant to solve? It seems that this should be done in CSS. \$\endgroup\$
    – jme11
    Feb 22 at 0:41

1 Answer 1

1
\$\begingroup\$

Review

The code seems a bit over-complicated. There are multiple DOM lookups for the same elements (e.g. using document.getElementById(), document.querySelectorAll()). It also seems that the class name responsive is used to actually determine if the hamburger menu is displayed - not whether the page is responsive, as in layout and appearance altered because of various screen widths. I do like how equality comparisons are used (i.e. ===, !===).

In the CSS there are two instances of @media screen and (max-width: 1000px) { and two of @media screen and (max-width: 600px) { - those can be consolidated. There are some styles where a unit is used with 0 - e.g. margin: 50px 0px 50px 0px; and some without- e.g. margin: 0 auto;. It is optional1 and some argue the units should be omitted2 while others argue they should be included for scenarios like when calc() is used3. It is best to be consistent.

Modifying id attribute of elements

It seems a bit unorthodox to clear the id attribute and then set it to something different, at least for responsive design. E.g. calling logo.removeAttribute('id') and then logo.setAttribute('id', 'sticky-logo') is something that I haven't see before for responsive design code. Typically class names are applied to one or more base elements and the styles are adjusted accordingly with the various class names or other selectors. Styles can be changed simply by adding and removing class names. The classList API has methods for easily adding and removing classes - like add() and remove().

Event handlers

Assigning event handlers like onscroll works but it is a good habit to use Element.addEventHandler() to allow for the possibility of multiple callbacks being triggered for given events. So instead of:

window.onscroll = function() { stickyNav(); };

It can be simply:

window.addEventListener('scroll', function() { stickyNav(); });

Or since stickyNav takes no arguments, the lambda/anonymous function can be removed:

window.addEventListener('scroll', stickyNav)

Variable Declarations

As others have mentioned in answers to your previous questions, it is wise to use const for any variable that is only assigned once. This helps avoid accidental re-assignment and other bugs.

Variable names

The variable sticky contains an integer:

let sticky = nav.offsetTop;

Perhaps a better name for that variable would be something like navOffsetTop or stickyThreshold. Then anyone reading the code might have a clue that it is supposed to be an integer.

Then there is the string literal assigned to fixedNavContainer.

let fixedNavContainer = 'fixed-nav-container';

If I saw that name later in the code I might think it was a reference to a container element. Perhaps a more appropriate name would be something like fixedNavContainerClass.

Then there is createFixedNav:

let createFixedNav = document.createElement('div');

While it is true that the value assigned comes from calling the createElement method, it is a reference to an element so there is no sense in having the word create in the variable name. A more appropriate name might be container or something specific to what the element contains or will contain.

References to elements

At the start of the script is this line:

let nav = document.getElementById('nav');

Then in the createIcon() function, which is called immediately, there is this line:

let getthenav = document.querySelectorAll('#nav')[0];

Instead of querying the DOM again, it could simply use nav from earlier in the script.

Additionally, there is this line above the function definition for makeNavResponsive():

let getNav = document.getElementById('nav');

Again this queries the DOM for the same element that is stored in nav.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.