Javascript not selecting semantic html ul

JavaScript
1

I posted a few days ago and got some awesome help which fixed my issue on selecting the ul within a <nav> tag without a specified class name.

I have now realised I should be using a class for the semantic html to future-proof any additional navs in the future.

Using querySelector for the <nav class="navigation-bar"> which selects correctly, however when I extend and use querySelector('.navigation-bar').getElementsByTagName('ul'); nothing happens.

Please see jsfiddle below.

2 
querySelector('.navigation-bar')

already gives you a list of all elements having the class “navigation-bar”. You do not need to do a getElementsByTagName.

3

Hi Thallius,

So when I had it as just querySelector('.navigation-bar');
The functionality comes back but it moves the nav-menu and anything within the flex container to the ul’s style rather than showing the .navigation-bar ul contents.

4

If you just have one ul within that section then you can just do this.

const menu = document.querySelector('.navigation-bar ul');

Why are you toggling nav-menu which is the hamburger and then applying the hamburger nav-menu class to the ul? Shouldn’t you just be adding something like a nav-open class to the ul instead.

e.g.

const toggleButton = document.querySelector('.nav-menu');
const menu = document.querySelector('.navigation-bar ul');
console.log(toggleButton);
toggleButton.addEventListener('click', () => {
  menu.classList.toggle('nav-open')
})

Then use that class to open the menu in css.

.navigation-bar ul.nav-open{display:flex;}

5

I think you meant ‘.querySelectorAll’ :wink:

1 Like
6

Of course you are right

1 Like
7

Something I think you may possibly need to consider is css specificity

Having your html and css formatted properly makes the javascript part of the equation a lot easier.

It was a bit tricky working through your css as there were duplicate rules in there — I appreciate you are experimenting at this stage :slight_smile:

It maybe an idea though, to take what you have to the css section of this site and get some expert advise.

I did do a bit of a refactor, and apologise if I have gone a bit overboard.

html

You mention semantics, so I have gone for what I think is more semantic html

<header>
  <nav>
	<a class="logo" href="#">
	  <span class="black">JS</span>
	  <span class="red">Test</span>
	</a>
	<div class="menu-toggler">
	  <i class="fa-solid fa-bars"></i>
	</div>
	<ul class='nav-menu'>
	  <li><a href="#">Home</a></li>
	  <li><a href="#">About</a></li>
	  <li><a href="#">Services</a></li>
	  <li><a href="#">Shop</a></li>
	  <li><a href="#">Contact</a></li>
	</ul>
  </nav>
</header>

css

This probably isn’t the best css, but a bit of a refactor anyway. I dropped some of the flex boxes you had. display: block; seemed just fine for the nav-menu — that’s just my take though.

/* CSS Resets */
* {
  box-sizing: border-box;
  margin: 0;
  padding: 0;
}

a {
  text-decoration: none;
}

li {
  list-style-type: none;
}
/* END CSS Resets */

body {
  background: #d2abab;
  font-family: 'Quicksand', sans-serif;
  height: 100vh;
}

/* Header Navigation */

header {
  position: relative;
  background: white;
  height: 6rem;
  padding: 0 25px;
}

nav {
  display: flex;
  justify-content: space-between;
  align-items: center;
  height: 100%;
  width: 100%;
}

.logo {
  font-family: '01 Digitall', sans-serif;
  font-size: 40px;
  text-transform: uppercase;
}

.black {
  color: #000000;
}

.red {
  color: #e74c3c;
}

.menu-toggler {
  padding: 8px 10px;
  background-color: #ecf0f1;
  cursor: pointer; 
}

.menu-toggler:hover {
  color: white;
  background-color: #e74c3c;
}

.nav-menu {
  display: none;
  position: absolute; /* relative to header */
  top: 100%;
  left: 0px;
  width: 100%;
  align-items: center;
  padding: 10px 0;
  background: #ecf0f1;
}

.nav-menu a {
  display: block;
  text-align: center;
  padding: 15px 0;
  cursor: pointer;
  color: black;
}

.nav-menu a:hover {
  color: white;
  background-color: #e74c3c;
}

/* .open will be toggled on the nav-menu */
.nav-menu.open {
  display: block;
}

/* END header navigation */

Javascript

window.addEventListener('DOMContentLoaded', () => {

  const menuToggler = document.querySelector('.menu-toggler')
  const menu = document.querySelector('.nav-menu')

  menuToggler.addEventListener('click', () => {
    menu.classList.toggle('open')
  })
})
2 Likes
8

Just for completeness, the element tag name should be in uppercase.

9

According to the documentation, lowercase should be used instead. https://developer.mozilla.org/en-US/docs/Web/API/Element/getElementsByTagName#syntax

10

From memory with a nodelist the nodeName is in uppercase.
So I only use uppercase to avoid confusion and errors as some method return htmlcollections and others return a nodelist.
And if you are developing in HTML5 why worry about XHTML?

11

Yes, that is when using nodeName. The recommendations are different when using getElementsByTagName.

Here are the HTML specs for getElementsByTagName.
https://dom.spec.whatwg.org/#dom-element-getelementsbytagname

That uses a qualifiedName, which is specified as being in lowercase.
https://dom.spec.whatwg.org/#concept-getelementsbytagname

I know that it’s unsettling for nodeName to be in uppercase and all other uses of element names being recommended to be in lowercase, but that’s the way things are.

Fortunately using all uppercase for the qualifiedName is supported in many cases. When there are multiple ways that are supported, that then tends to become a case for standards and styleguides to help resolve the differences.

1 Like
12

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.