Javascript - cannot change active element

Question

I'm completely new to JavaScript in frontend development, and I've got a really basic question I can't seem to figure out... I'm making a tabular navigation bar where the active element is highlighted.

I know there are other ways to accomplish the desired result, but what is wrong with my script here?

function navTabsClick(child) {
    getElementsByClassName("active")[0].classList.remove("active");
    child.getElementsByTagName("li")[0].className = "active";
}

ul{
  display: flex;
  list-style-type: none;
}
a{
  text-decoration: none;
}
a:link, a:visited{
  color: black;
}
a:hover{
  cursor: pointer;
}
li{
  margin: 0;
  padding: 10px 30px;
  background-color: lightgray;
}
li:hover{
  background-color: darkgray;
}
li.active{
  background-color: red;
}

<nav>
  <ul>
    <a onclick="navTabsClick(this)">
      <li class="active">1</li>
    </a>
    <a onclick="navTabsClick(this)">
      <li>2</li>
    </a>
    <a onclick="navTabsClick(this)">
      <li>3</li>
    </a>
  </ul>
</nav>

If I remove the top line of my JavaScript function, I can get other tabs to highlight, but as is I get the following error:

Uncaught ReferenceError: getElementsByClassName is not defined

What am I missing?

Also, as a secondary question, is there a better way to handle navbar JavaScript than this?

As you are new, take this advice, most of the examples you'll see on the web were not created by people who understand the code they are using. In many, many cases, they've copied someone else's code that seems to be working. If you follow that behavior, you are going to pick up some very bad habits. Don't use code like this in the first place: [`getElementsByClassName("active")[0]`](https://stackoverflow.com/questions/54952088/how-to-modify-style-to-html-elements-styled-externally-with-css-using-js/54952474#54952474) — Scott Marcus, Feb 23 '21 at 20:33
@ScottMarcus Thank you for the feedback! Can you point me to a better option for this use case? — YangTegap, Feb 23 '21 at 20:35
Just read through the link I shared. It explains that `document.querySelector()` is the better solution. — Scott Marcus, Feb 23 '21 at 20:38

score 3 · Answer 1 · answered Feb 23 '21 at 20:24

Try doing document.getElementByClassName

function navTabsClick(child) {
    document.getElementsByClassName("active")[0].classList.remove("active");
    child.getElementsByTagName("li")[0].className = "active";
}

ul{
  display: flex;
  list-style-type: none;
}
a{
  text-decoration: none;
}
a:link, a:visited{
  color: black;
}
a:hover{
  cursor: pointer;
}
li{
  margin: 0;
  padding: 10px 30px;
  background-color: lightgray;
}
li:hover{
  background-color: darkgray;
}
li.active{
  background-color: red;
}

<nav>
  <ul>
    <a onclick="navTabsClick(this)">
      <li class="active">1</li>
    </a>
    <a onclick="navTabsClick(this)">
      <li>2</li>
    </a>
    <a onclick="navTabsClick(this)">
      <li>3</li>
    </a>
  </ul>
</nav>

score 2 · Answer 2 · answered Feb 23 '21 at 20:23

2

You are missing the document before getElementsByClassName

answered Feb 23 '21 at 20:23

maurogandulfo

430
3
8

score 2 · Accepted Answer · answered Feb 23 '21 at 20:23

The function is document.getElementsByClassName, i.e.:

function navTabsClick(child) {
  document.getElementsByClassName("active")[0].classList.remove("active");
  child.getElementsByTagName("li")[0].className = "active";
}

ul {
  display: flex;
  list-style-type: none;
}

a {
  text-decoration: none;
}

a:link,
a:visited {
  color: black;
}

a:hover {
  cursor: pointer;
}

li {
  margin: 0;
  padding: 10px 30px;
  background-color: lightgray;
}

li:hover {
  background-color: darkgray;
}

li.active {
  background-color: red;
}

<nav>
  <ul>
    <a onclick="navTabsClick(this)">
      <li class="active">1</li>
    </a>
    <a onclick="navTabsClick(this)">
      <li>2</li>
    </a>
    <a onclick="navTabsClick(this)">
      <li>3</li>
    </a>
  </ul>
</nav>

D M · Answer 4 · 2021-02-23T20:46:09.017

Others have pointed out that getElementsByClassName must be called on an element, but that does not address the second part of your question.

Is there a better way to handle navbar JavaScript than this?

Right now, you are setting an event listener on each menu item which is unnecessary. The click event exposes the element that was clicked in the target property. You can set a single event listener on the menu itself and use this property to determine which item was clicked.

~~This also allows you to remove the a tags around your menu items.~~ I assume those will cause issues with screen readers. Edit: As pointed out by Scott, the click event could have been set directly on the li tag originally.

Here's a modified example with a single event for the menu.

function navClicked(nav, item) {
  
  /* Do not attempt to set the nav
     menu itself to the active item */
  if (nav === item) return;
  
  /* Do not change the active item if the
     item that was clicked is already active */
  if (item.classList.contains("active")) return;
  
  /* Remove the class from all nav items */
  Array.from(nav.children)
       .forEach(child => child.classList.remove("active"));
  
  /* Add the class to the item that was clicked */
  item.classList.add("active");
}

ul {
  display: flex;
  list-style-type: none;
}

a {
  text-decoration: none;
}

a:link,
a:visited {
  color: black;
}

a:hover {
  cursor: pointer;
}

li {
  margin: 0;
  padding: 10px 30px;
  background-color: lightgray;
}

li:hover {
  background-color: darkgray;
}

li.active {
  background-color: red;
}

<nav>
  <ul onclick="navClicked(this, event.target)"> <!-- 'this' is the ul element, 'event.target' is the li element that was clicked -->
    <li class="active">1</li>
    <li>2</li>
    <li>3</li>
  </ul>
</nav>

*This also allows you to remove the `a` tags around your menu items.* While I agree with your suggestion of event delegation, I want to point out that the `a` elements never needed to be there because you can add a `click` event handler on any visible DOM element, like the `li` elements themselves. — Scott Marcus, Feb 23 '21 at 20:40

Javascript - cannot change active element

4 Answers4