Add/Remove CSS Class with JS Loop

I have a navigation menu that displays a group of notes from the clicked topic. The note can either have class note if it is visible or class invisible if it is hidden. I want only the notes from the topic clicked to show.

The problem is that some notes from other topics are also being shown. Even though the length of thisTopic is always 2.

I am new to JavaScript so maybe there is an error in my loop? Thanks in advance 🙂

function openTopic(evt, topicName) {
    var allNotes, thisTopic;
    
  /* Hide all notes */
    allNotes = document.getElementsByClassName("note");
    for (i = 0; i < allNotes.length; i++) {
    allNotes[i].classList.add("invisible");
        allNotes[i].classList.remove("note");
    }
    
  /* Show notes with correct topic */
    thisTopic = document.getElementsByClassName(topicName);
    for (i = 0; i < thisTopic.length; i++) {
        thisTopic[i].classList.add("note");
        thisTopic[i].classList.remove("invisible");
    }
}

.box {
    border-radius: 10px;
    box-shadow: 5px 5px 8px #999;
    margin: 10px;
    padding: 10px;
}

.note {
    display: block;
  background-color: #ddd;
}

.invisible {
  display: none;
}

<nav class='box'>
    <h3>Navigation</h3>
    <ul>
        <li onClick="openTopic(event, 'topic1')">Topic 1</li>
    <li onClick="openTopic(event, 'topic2')">Topic 2</li>
    <li onClick="openTopic(event, 'topic3')">Topic 3</li>
    </ul>
</nav>

<main>
  <section class='note topic1 box'>
    <p>First topic 1 note</p>
  </section>
  <section class='note topic1 box'>
    <p>Second topic 1 note</p>
  </section>
  <section class='note topic2 box'>
    <p>First topic 2 note</p>
  </section>
  <section class='note topic2 box'>
    <p>Second topic 2 note</p>
  </section>
  <section class='note topic3 box'>
    <p>First topic 3 note</p>
  </section>
  <section class='note topic3 box'>
    <p>Second topic 3 note</p>
  </section>
</main>

Answer

There are two issues with your code:

• You’re removing the note class from elements.
• You’re not giving elements that should be hidden by default the invisible class.

function openTopic(evt, topicName) {
    var allNotes, thisTopic;
    
  /* Hide all notes */
    allNotes = document.getElementsByClassName("note");
    for (i = 0; i < allNotes.length; i++)
        allNotes[i].classList.add("invisible");
    
  /* Show notes with correct topic */
    thisTopic = document.getElementsByClassName(topicName);
    for (i = 0; i < thisTopic.length; i++)
        thisTopic[i].classList.remove("invisible");
}

.box {
    border-radius: 10px;
    box-shadow: 5px 5px 8px #999;
    margin: 10px;
    padding: 10px;
}

.note {
  display: block;
  background-color: #ddd;
}

.note.invisible {
  display: none;
}

<nav class='box'>
    <h3>Navigation</h3>
    <ul>
        <li onClick="openTopic(event, 'topic1')">Topic 1</li>
        <li onClick="openTopic(event, 'topic2')">Topic 2</li>
        <li onClick="openTopic(event, 'topic3')">Topic 3</li>
    </ul>
</nav>

<main>
  <section class='note topic1 box'>
    <p>First topic 1 note</p>
  </section>
  <section class='note topic1 box'>
    <p>Second topic 1 note</p>
  </section>
  <section class='note topic2 box invisible'>
    <p>First topic 2 note</p>
  </section>
  <section class='note topic2 box invisible'>
    <p>Second topic 2 note</p>
  </section>
  <section class='note topic3 box invisible'>
    <p>First topic 3 note</p>
  </section>
  <section class='note topic3 box invisible'>
    <p>Second topic 3 note</p>
  </section>
</main>

With that in mind however, I’d strongly recommend a few things:

• Reverse your logic. (Hide by default, then activate.)
• Use the id attribute at a higher level instead of class for topic#.

Reverse Your Logic

Currently, you have 3 topics with 2 notes each. Imagine instead, that you have 5 topics with 5 notes each. With your current logic, you’ll need to assign the invisible class, by default to 20 section elements. Instead, use an active class and you’ll only need to assign it to 5 section elements:

.note {
    display: none;
    background-color: #ddd;
}
.note.active { display: block }

You can see how this impacts your snippet below when taken to a larger scale:

function openTopic(evt, topicName) {
    var allNotes, thisTopic;

    /* Hide all notes */
    allNotes = document.getElementsByClassName("note");
    for (i = 0; i < allNotes.length; i++)
        allNotes[i].classList.remove("active");

    /* Show notes with correct topic */
    thisTopic = document.getElementsByClassName(topicName);
    for (i = 0; i < thisTopic.length; i++)
        thisTopic[i].classList.add("active");
}

.box {
    border-radius: 10px;
    box-shadow: 5px 5px 8px #999;
    margin: 10px;
    padding: 10px;
}

.note {
    display: none;
    background-color: #ddd;
}

.note.active {
    display: block;
}

<nav class='box'>
    <h3>Navigation</h3>
    <ul>
        <li onClick="openTopic(event, 'topic1')">Topic 1</li>
        <li onClick="openTopic(event, 'topic2')">Topic 2</li>
        <li onClick="openTopic(event, 'topic3')">Topic 3</li>
    </ul>
</nav>

<main>
    <section class='note topic1 box active'>
        <p>Topic 1 Note 1</p>
    </section>
    <section class='note topic1 box active'>
        <p>Topic 1 Note 2</p>
    </section>
    <section class='note topic1 box active'>
        <p>Topic 1 Note 3</p>
    </section>
    <section class='note topic1 box active'>
        <p>Topic 1 Note 4</p>
    </section>
    <section class='note topic1 box active'>
        <p>Topic 1 Note 5</p>
    </section>
    <section class='note topic2 box'>
        <p>Topic 2 Note 1</p>
    </section>
    <section class='note topic2 box'>
        <p>Topic 2 Note 2</p>
    </section>
    <section class='note topic2 box'>
        <p>Topic 2 Note 3</p>
    </section>
    <section class='note topic2 box'>
        <p>Topic 2 Note 4</p>
    </section>
    <section class='note topic2 box'>
        <p>Topic 2 Note 5</p>
    </section>
    <section class='note topic3 box'>
        <p>Topic 3 Note 1</p>
    </section>
    <section class='note topic3 box'>
        <p>Topic 3 Note 2</p>
    </section>
    <section class='note topic3 box'>
        <p>Topic 3 Note 3</p>
    </section>
    <section class='note topic3 box'>
        <p>Topic 3 Note 4</p>
    </section>
    <section class='note topic3 box'>
        <p>Topic 3 Note 5</p>
    </section>
</main>

Use id and grouping

It’s common practice to group elements within a parent element. In your case, I recommend encapsulating your section elements into a div that is designed for each topic using the id attribute:

<div id="topic1Notes" class="topic-container active">
    <section class="note box">
        <p>Topic 1 Note 1</p>
    </section>
</div>

Use Bootstrap

If using third party tools like Bootstrap aren’t out of the question, they already handle this and provide very easy to follow code for doing so, taking the leg work off of you:

.box {
    border-radius: 10px;
    box-shadow: 5px 5px 8px #999;
    margin: 10px;
    padding: 10px;
}

.note {
  display: block;
  background-color: #ddd;
}

.note.invisible {
  display: none;
}

<script src="https://cdn.jsdelivr.net/npm/bootstrap@5.0.0-beta2/dist/js/bootstrap.bundle.min.js"></script>
<link href="https://cdn.jsdelivr.net/npm/bootstrap@5.0.0-beta2/dist/css/bootstrap.min.css" rel="stylesheet" />
<nav>
    <div class="nav nav-tabs" id="nav-tab" role="tablist">
        <button class="nav-link active" id="nav-home-tab" data-bs-toggle="tab" data-bs-target="#nav-home" type="button" role="tab" aria-controls="nav-home" aria-selected="true">Topic 1</button>
        <button class="nav-link" id="nav-profile-tab" data-bs-toggle="tab" data-bs-target="#nav-profile" type="button" role="tab" aria-controls="nav-profile" aria-selected="false">Topic 2</button>
        <button class="nav-link" id="nav-contact-tab" data-bs-toggle="tab" data-bs-target="#nav-contact" type="button" role="tab" aria-controls="nav-contact" aria-selected="false">Topic 3</button>
    </div>
</nav>
<div class="tab-content" id="nav-tabContent">
    <div class="tab-pane fade show active" id="nav-home" role="tabpanel" aria-labelledby="nav-home-tab">
        <section for="topic1" class='note topic1 box'>
            <p>First topic 1 note</p>
        </section>
        <section class='note topic1 box'>
            <p>Second topic 1 note</p>
        </section>
    </div>
    <div class="tab-pane fade" id="nav-profile" role="tabpanel" aria-labelledby="nav-profile-tab">
        <section class='note topic2 box'>
            <p>First topic 2 note</p>
        </section>
        <section class='note topic2 box'>
            <p>Second topic 2 note</p>
        </section>
    </div>
    <div class="tab-pane fade" id="nav-contact" role="tabpanel" aria-labelledby="nav-contact-tab">
        <section class='note topic3 box'>
            <p>First topic 3 note</p>
        </section>
        <section class='note topic3 box'>
            <p>Second topic 3 note</p>
        </section>
    </div>
</div>

---

Best of luck!