1
\$\begingroup\$

I'm just starting to learn website coding with html, css, and JavaScript. To get a bit of a hang for it, I'm trying to build a complete web page.

So far, I've only done the navbar. I started with a basic Bootstrap 5 navbar and modified it to get it to look and behave like I want. It took me three days, but it looks good now.

Or at least, on the outside it does. I can't, however, shake the feeling that this could've been done way easier, with a lot less code.

Could anyone point out some examples of where I may have gone about it way too roundabout and what I could've done better?

I've pasted my code in a CodePen:

https://codepen.io/AlexanderSplat/pen/ZELLEeB

$(window).scroll(function() {
  var scroll = $(window).scrollTop();
  var width = $(window).width();
  if (scroll >= 50 && width <= 1000) {
    document.querySelector(".navbar").style.paddingTop = "0";
    document.querySelector(".navbar").style.paddingBottom = "0";
    document.querySelector(".navbar-brand").style.fontSize = "28px";
    document.querySelector(".navbar-brand").style.paddingTop = "0.4%";
    document.querySelector(".navbar-brand").style.paddingBottom = "0.3%";
    document.querySelector(".navbar-brand").style.height = "40px";
    document.querySelector(".navbar-brand").style.lineHeight = "40px";
    document.querySelector("#navhome").style.paddingTop = "15px";
    document.querySelector("#navabout").style.paddingTop = "15px";
    document.querySelector("#navport").style.paddingTop = "15px";
    document.querySelector("#navteam").style.paddingTop = "15px";
    document.querySelector("#navcont").style.paddingTop = "15px";
    document.querySelector("#navhome").style.paddingBottom = "14px";
    document.querySelector("#navabout").style.paddingBottom = "14px";
    document.querySelector("#navport").style.paddingBottom = "14px";
    document.querySelector("#navteam").style.paddingBottom = "14px";
    document.querySelector("#navcont").style.paddingBottom = "14px";
  } else if (scroll >= 50 && width >= 1001) {
    document.querySelector(".navbar").style.paddingTop = "0";
    document.querySelector(".navbar").style.paddingBottom = "0";
    document.querySelector(".navbar-brand").style.fontSize = "28px";
    document.querySelector(".navbar-brand").style.paddingTop = "0.4%";
    document.querySelector(".navbar-brand").style.paddingBottom = "0.3%";
    document.querySelector(".navbar-brand").style.height = 'initial';
    document.querySelector(".navbar-brand").style.lineHeight = 'initial';
    document.querySelector("#navhome").style.paddingTop = "15px";
    document.querySelector("#navabout").style.paddingTop = "15px";
    document.querySelector("#navport").style.paddingTop = "15px";
    document.querySelector("#navteam").style.paddingTop = "15px";
    document.querySelector("#navcont").style.paddingTop = "15px";
    document.querySelector("#navhome").style.paddingBottom = "14px";
    document.querySelector("#navabout").style.paddingBottom = "14px";
    document.querySelector("#navport").style.paddingBottom = "14px";
    document.querySelector("#navteam").style.paddingBottom = "14px";
    document.querySelector("#navcont").style.paddingBottom = "14px";
  } else {
    document.querySelector(".navbar").style.paddingTop = "8px";
    document.querySelector(".navbar-brand").style.fontSize = "40px";
    document.querySelector(".navbar-brand").style.paddingTop = "0.6%";
    document.querySelector(".navbar-brand").style.paddingBottom = "0.6%";
    document.querySelector(".navbar-brand").style.height = 'initial';
    document.querySelector(".navbar-brand").style.lineHeight = 'initial';
    document.querySelector("#navhome").style.paddingTop = "28px";
    document.querySelector("#navabout").style.paddingTop = "28px";
    document.querySelector("#navport").style.paddingTop = "28px";
    document.querySelector("#navteam").style.paddingTop = "28px";
    document.querySelector("#navcont").style.paddingTop = "28px";
    document.querySelector("#navhome").style.paddingBottom = "28px";
    document.querySelector("#navabout").style.paddingBottom = "28px";
    document.querySelector("#navport").style.paddingBottom = "28px";
    document.querySelector("#navteam").style.paddingBottom = "28px";
    document.querySelector("#navcont").style.paddingBottom = "28px";
  }
});
body {
  height: 200vh;
}

@media (min-width: 1001px) {
  .navbar-brand {
    color: white;
    font-family: 'Julius Sans One';
    font-style: normal;
    font-weight: 400;
    font-size: 40px;
    padding-left: 67px;
    margin-left: 1.9%;
    transition: 0.4s;
  }
  .navbar {
    background-color: #105565;
    position: fixed;
    width: 100%;
    transition: 0.4s;
  }
}

@media (max-width: 1000px) {
  .navbar-brand {
    color: white;
    font-family: 'Julius Sans One';
    font-style: normal;
    font-weight: 400;
    font-size: 40px;
    height: 52px;
    line-height: 52px;
    padding-top: 0;
    padding-bottom: 0;
    margin-left: 2.5%;
    transition: 0.4s;
  }
  .navbar {
    background-color: #105565;
    position: fixed;
    width: 100%;
    padding-top: 0px;
    padding-bottom: 0px;
    transition: 0.4s;
  }
  .fas {
    color: rgba(255, 255, 255, 1.00);
  }
}

.navbar-nav {
  margin-right: 4.4%;
}

.nav-item {
  font-family: 'roboto';
  font-size: 14px;
  font-weight: 300;
}

#navhome {
  color: white;
  background: #105565;
  padding-top: 28px;
  padding-bottom: 28px;
  padding-left: 12px;
  padding-right: 12px;
}

#navabout {
  color: white;
  background: #105565;
  padding-top: 28px;
  padding-bottom: 28px;
  padding-left: 12px;
  padding-right: 12px;
}

#navport {
  color: white;
  background: #105565;
  padding-top: 28px;
  padding-bottom: 28px;
  padding-left: 12px;
  padding-right: 12px;
}

#navteam {
  color: white;
  background: #105565;
  padding-top: 28px;
  padding-bottom: 28px;
  padding-left: 12px;
  padding-right: 12px;
}

#navcont {
  color: white;
  background: #105565;
  padding-top: 28px;
  padding-bottom: 28px;
  padding-left: 12px;
  padding-right: 12px;
}

#navhome:hover {
  color: #105565 !important;
  background: white;
  padding-top: 28px;
  padding-bottom: 28px;
  padding-left: 12px;
  padding-right: 12px;
}

#navabout:hover {
  color: #105565 !important;
  background: white;
  padding-top: 28px;
  padding-bottom: 28px;
  padding-left: 12px;
  padding-right: 12px;
}

#navport:hover {
  color: #105565 !important;
  background: white;
  padding-top: 28px;
  padding-bottom: 28px;
  padding-left: 12px;
  padding-right: 12px;
}

#navteam:hover {
  color: #105565 !important;
  background: white;
  padding-top: 28px;
  padding-bottom: 28px;
  padding-left: 12px;
  padding-right: 12px;
}

#navcont:hover {
  color: #105565 !important;
  background: white;
  padding-top: 28px;
  padding-bottom: 28px;
  padding-left: 12px;
  padding-right: 12px;
}
<nav class="navbar navbar-expand-lg navbar-dark m-0 p-0">
  <div class="container-fluid"> <a class="navbar-brand" href="#">Navbar Inc.</a> <button class="navbar-toggler" style="color: rgba(0,0,0,0.00);" type="button" data-bs-toggle="collapse" data-bs-target="#navbarSupportedContent" aria-controls="navbarSupportedContent" aria-expanded="false"
      aria-label="Toggle navigation"> <span class="fas fa-bars"></span> </button>
    <div class="collapse navbar-collapse" id="navbarSupportedContent">
      <ul class="navbar-nav ms-auto mb-2 mb-lg-0">
        <li class="nav-item"> <a class="nav-link active" id="navhome" style="color:white;" aria-current="page" href="#">HOME</a> </li>
        <li class="nav-item"> <a class="nav-link active" id="navabout" style="color:white;" aria-current="page" href="#">ABOUT</a> </li>
        <li class="nav-item"> <a class="nav-link active" id="navport" style="color:white;" aria-current="page" href="#">PORTFOLIO</a> </li>
        <li class="nav-item"> <a class="nav-link active" id="navteam" style="color:white;" aria-current="page" href="#">TEAM</a> </li>
        <li class="nav-item"> <a class="nav-link active" id="navcont" style="color:white;" aria-current="page" href="#">CONTACT</a> </li>
      </ul>
    </div>
  </div>
</nav>

I had the most trouble with getting the white selection boxes (that appear when you hover over each of the nav-links) to cover the entire height of the navbar and the entire width of the nav-item in each responsive size, and with the position of the text in each size. (There are basically four size states: scrolled up or down in a narrow (<1000px) or a wide window.)

By posting this question in the wrong section (I'm new here), I already got some great suggestions, but more tips to improve this code are very welcome.

It's true that I couldn't find a way to solve the padding conflicts without !important, so if someone can point me in the right direction there, I'd be much obliged.

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

First off, congrats on getting your navbar working and sticking with this over several days! 🎉🥂

There are a lot of things which could be improved, but at a glance the most glaring issue is that your styles are spread all across the HTML, CSS, and JS files. This makes it very difficult to reason about them. Here's how you could simplify the situation and make it easier to work with.

  1. Remove all style attribute assignments from your HTML and JS - put these style instructions in your CSS classes, instead.
  2. Replace the style attribute assignments in your JS code with class assignments to simplify things and reduce the number of (expensive!) DOM queries you are running whenever the user scrolls.
  3. Use media queries to handle all styles dependent on the horizontal size of the viewport. You are already doing this in your CSS (e.g. @media (max-width: 1000px)), but then you're also doing it separately in the JS code which confuses things. Just do it in CSS.
  4. (Optional) Don't use jQuery. The native web APIs are more than capable of doing what you're achieving with jQuery here, you can simplify a lot by removing it.

With the above-described changes, here's what your JS might look like:

// Query just once, and reuse the reference.
var navbar = document.querySelector(".navbar");
var isScrolledDownClass = "is-scrolled-down";
var scrollDownClassThreshold = 50;

window.addEventListener("scroll", function() {
  if (!navbar) return; // or treat the failure to query the navbar as an error
  if (window.scrollY >= scrollDownClassThreshold) {
    navbar.classList.add(isScrolledDownClass);
  } else {
    navbar.classList.remove(isScrolledDownClass);
  }
});

In CSS, you can use the descendent combinator to apply styles to children based on the state of their ancestors. This will help you apply certain styles only when the navbar element has the is-scrolled-down class:

#navabout {
  color: white;
  background: #105565;
  padding-top: 28px;
  padding-bottom: 28px;
  padding-left: 12px;
  padding-right: 12px;
}

.navbar.is-scrolled-down #navabout {
  padding-bottom: 14px;
  padding-top: 15px;
}
\$\endgroup\$
6
  • \$\begingroup\$ Thanks so much! This helps me a lot. I think I understand most of your suggestions. Although I do wonder why you need to add the "is-scrolled-down" class through a variable instead of directly. Why not navbar.classList.add("is-scrolled-down");? \$\endgroup\$ Commented Apr 1, 2021 at 12:47
  • 1
    \$\begingroup\$ Glad it helps! Feel free to ask if anything is unclear. You don't need use a variable, but I do out of habit for a number of reasons, the most pertinent in this case being that the string is used in two places, so using a variable protects you from a typo in one place and not the other. I habitually try to avoid using special strings directly in code for a number of reasons, this question sums it up reasonably well. \$\endgroup\$
    – Manningham
    Commented Apr 1, 2021 at 13:14
  • \$\begingroup\$ Makes sense! Sounds like good practice. There's actually one more line I don't understand: if (!navbar) return; (nor the comment after it, actually: or treat the failure to query the navbar as an error). What does the exclamation mark mean there, and what is being returned? \$\endgroup\$ Commented Apr 1, 2021 at 13:29
  • 2
    \$\begingroup\$ @AlexanderSplat it is the logical NOT operator- basically equivalent to a check for false, undefined or null . \$\endgroup\$ Commented Apr 1, 2021 at 13:43
  • 1
    \$\begingroup\$ Yes, and to add on to @SᴀᴍOnᴇᴌᴀ's answer this is because of the concept of truthy/falsy values in JS. WRT the return statement, it returns undefined. JS functions often return undefined in the absence of a meaningful return value. You can give yourself a good, solid foundation in all this by going through chapters 1-4 of Eloquent JavaScript :) \$\endgroup\$
    – Manningham
    Commented Apr 1, 2021 at 16:02

Your Answer

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

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