Second toggle function not working

Hello

I have a toggle function in part of my code which works fine (displays text when clicked).

I am trying to implement a second function which does the same thing for a second button (I will maybe try to use just one function for all buttons in future, but just want to get it working at the moment).

Even though the code is the same for the second function, I am getting a message saying the code is not defined.

Would someone be able to give me a clue as to where I am going wrong please?

HTML

 <div class = "quickfact2" id = "quickfact2">
      <button class = "quickfact_button_2" 
              id = "quickfact_button_2" 
              onclick = "toggle2()" >Quick fact 2</button>
          <div class = "quickfact_container_2" 
          id = "quickfact_container_2" >
            <p>fact number 2</p>
        </div>
  </div>

CSS

.quickfact_container_2,
.quickfact_container_3,
.quickfact_container_4
{
  display: none;
  border-radius: 30px;
  height: fit-content;  
}

JS

let quickfactContainer2 = document.getElementById("quickfact_container_2"); 
  let quickfactButton2 = document.getElementById("quickfact_button_2");

  function toggle2() {
    if (quickfactContainer2.style.display === "none")
 {
      quickfactContainer2.style.display = "flex";
      quickfactButton2.style.display = "none";
    } else {
      quickfactContainer2.style.display = "none";
    }
  }

  • are you forgetting to include “button” display style adjustment here?
  • do you have that live on copdepen or repl or as such

happy coding :slight_smile:

hello

No, I do not have a button adjustment on the original function, which works as needed, so I do not have it in the second function either.

I don’t have it on codepen

What is the exact error message?

Without seeing all your code, including the HTML we can’t really help you.

it says that toggle2 is not defined

the code is quiet long and (very) draft (I need to edit it a lot). It is okay to put is all here (concerned it might be too long…)?

A live editor like Codepen or Stackblitz would be better. That or a GitHub repo with the code. But sure if nothing else you can just post it all.

How and where is the JS loaded?

The script element must be placed after the HTML content. If it is loaded from a file you can also use defer on the script element.

thanks. I don’t have codepen set up yet, so will post here:

<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <link rel="stylesheet" href="styles.css" />
    
    <title>Pol</title>
    
  </head>
  <body>
    
    
      <header>
        <h1>Put in title</h1>
      </header>
   
<div class = "nav_plus_bios">
    <nav id="navbar" class="navbar">
      <ul>
        <li class = "quicklinks">QUICKLINKS</li>
      <li><a href="#james_joyce">James Joyce</a></li>
        
      <li><a href="#natalie_portman">Natalie Portman</a></li>
        
      <li><a href="#jrr_tolkien">J.R.R. Tolkien</a></li>
      
      <li><a href="#shakira">Shakira</a></li>
      </ul>
    </nav>


    <section class = "main_section_excl_nav">
        <p id="intro_text" class="intro_text">
            'Polywhattery?' I hear you ask. Although it might sound like a potentially serious illness, this might be the type of illness you would wish to have. 'Polyglottery' is in fact derived from the Greek words 'polloi' (meaning 'many') and 'glotta', meaning 'languages' and refers to the ability to speak several languages.  Click on the following buttons to read about some well-known polyglots...
          </p>
          
              
             
              <div class = "james_joyce_container bio_container_hover" id = "james_joyce_container">
                <img src="Images/jamesjoyce.jpg" class = "james_joyce_img" alt = "Image of James Joyce" onclick="toggle4"/>
    
<!--<div class="biosPlusQuickFactsContainer">*-->
      <p class = "james_joyce_biography" id = "james_joyce_biography">
      <strong>James Joyce</strong> (1882-1941) The famous Irish writer was
        known for his love of languages and literature. He was initially a
        teacher and a translator; he taught English all over Europe and during
        his travels he was able to pick up a variety of other languages,
        including Italian, German, and French. It is hard to determine how many
        he actually spoke, but he’s most often credited with communicating in
        between 13 and 17 other languages. His Ulysses contains fragments in
        Hebrew, Latin, Greek, Spanish, Irish Gaelic, and more. Apart from that,
        he was also a scholar who studied the nature of languages, so he could
        probably read a wider variety of texts. Apparently he only learned
        Norwegian to read Ibsen in his original language.
      </p>
    </div>
    
    
  
  
  

  
    <div class="natalie_portman_container bio_container_hover" id="natalie_portman_container" >
    <img src="Images/natalie_portman.jpg" class = "natalie_portman_img" alt = "Image of Natalie Portman"/>
      <p class = "natalie_portman_biography">
        The famous Oscar-winning actress,
        <strong>Natalie Portman</strong> is one of the most widely-known
        polyglots among celebrities. Born in Israel to an Israeli father and
        American mother, she later moved to the United States; she therefore
        grew up speaking both Hebrew and English. Aside from her acting talent,
        she is known for her thirst for knowledge (she’s a Harvard graduate with
        a BA in psychology), and since she was raised as a bilingual-speaker,
        she’s also an avid learner of languages. Today she speaks not only
        English and Hebrew, but also Arabic, German, French, Spanish, and
        Japanese. She’s a native-speaker of the first two and has conversational
        abilities in the rest, giving interviews and participating in tv
        programs in all of them.
      </p>
    </div>
    <div id="jrr_tolkien_container" class="jrr_tolkien_container bio_container_hover">
    
    
      <img src="Images/jrr_Tolkien.png" class="jrr_tolkien_img" alt = "Image of JRR Tolkien">
      <p class = "jrr_tolkien_biography">
        It’s not an exaggeration to say that
        <strong>J.R.R. Tolkien</strong> is one of the most famous polyglots and
        most impressive linguists in the history of humankind. The father of
        Middle Earth was a remarkable figure even among the most talented
        polyglots – he could speak 35 (!) different languages, both modern and
        ancient. He became so proficient in learning languages that at some
        point he learned Finnish just for fun. But that’s not all – he used that
        impressive knowledge and ability to create languages of his own, the
        most widely-known being Quenya, one of the Elvish languages from his
        books.
      </p>
    </div>
    <div id="shakira_container" class="shakira_container bio_container_hover">
    
    
      <img src="Images/shakira.jpg" class = "shakira_img" alt = "Image of Shakira"/>
      <p class = "shakira_biography">
        <strong>Shakira</strong> was born in Columbia, so her native language is
        Spanish. Apart from that, she can also speak English and Portuguese,
        both fluently – the former because of her early boyfriend and later
        international career, and the latter because of an extended music tour
        she did in Brazil during her teens. Moreover, she’s reported to be able
        to communicate in Italian, Catalan, and Arabic. Right now, as a mother,
        she often stresses the importance of multilingualism and has tried to
        expose her sons to numerous languages from the very beginning.
      </p>
    </div>
  
<!-- QUICKFACTS BUTTONS-->
<div class="quickfacts">
  <p>Interesting facts about languages</p>
<div class = "quickfact1" id = "quickfact1">
  <button class = "quickfact_button_1" id = "quickfact_button_1" onclick = "toggle1()" >Quick fact 1</button>
              <div class = "quickfact_container_1" id = "quickfact_container_1" >
                <p>
                  The unclassified Busuu language, spoken in the Southern Bantoid of Cameroon, is <strong>spoken by only eight people!</strong> At least it was in 1986. By 2005, only three people spoke Busuu, making it an endangered language.
      </p>
    </div>
    <div class = "quickfact2" id = "quickfact2">
      <button class = "quickfact_button_2" 
              id = "quickfact_button_2" 
              onclick = "toggle2()" >Quick fact 2</button>
          <div class = "quickfact_container_2" 
          id = "quickfact_container_2" >
            <p>fact number 2</p>
        </div>
  </div>

  <div class = "quickfact3" id = "quickfact3">
    <button class = "quickfact_button_3" id = "quickfact_button_3" onclick = "toggle3()" >Quick fact 3</button>
                <div class = "quickfact_container_3" id = "quickfact_container_3" >
                  <p>
        fact number 3
        </p>
      </div>
</div>
<div class = "quickfact4" id = "quickfact4">
  <button class = "quickfact_button_4" id = "quickfact_button_4" onclick = "toggle4()" >Quick fact 4</button>
              <div class = "quickfact_container_4" id = "quickfact_container_4" >
                <p>
      fact number 4
      </p>
    </div>
</div>
    
    
  
</div>
<footer>
  
  <a href = "google" class = "footerLI">bla</a>
  <a href = "google"class = "footerLI">bla</a>
  <a href = "google" class = "footerLI">bla</a>
  <a href = "google" class = "footerLI">bla</a>
  <a href = "google" class = "footerLI">bla</a>
  
 
</footer>
</section>
<script src="script.js"> </script>
  </body>
</html>

CSS

* {
  margin: 0px;
  font-family: sans-serif;
}
@import url("https://fonts.googleapis.com/css2?family=Caveat:wght@400..700&family=Yesteryear&display=swap");

@import url("https://fonts.googleapis.com/css2?family=Just+Another+Hand&display=swap");

@import url("https://fonts.googleapis.com/css2?family=Caveat:wght@400..700&family=Indie+Flower&family=Yesteryear&display=swap");

@import url("https://fonts.googleapis.com/css2?family=Gloria+Hallelujah&family=Just+Another+Hand&display=swap")
body {
  margin-right: 50px;
  background-color: rgb(192, 194, 197);
  width: 100%;
  overflow-x: hidden;
}

.james_joyce_container,
.natalie_portman_container,
.jrr_tolkien_container,
.shakira_container {
  display: flex;
  gap: 40px;
  margin-bottom: 80px;
  color: rgb(70, 69, 69);
  /*border: solid 6px rgba(5, 90, 164, 0.9)*/
  /*width: fit-content;*/
  /*block-size: fit-content;*/
  margin-left: 100px;
  margin-right: 150px;
  /*background-color: rgb(198, 223, 249);*/
  padding: 20px;
  height: 250px;
  width: 900px;
}

.james_joyce_biography,
.natalie_portman_biography,
.jrr_tolkien_biography,
.shakira_biography {
  font-size: 19px;
}

/*
.james_joyce_button {
  background-image: url(Images/jamesjoyce.jpg);
}
.natalie_portman_button {
  background-image: url(Images/natalie_portman.jpg);
}
.jrr_tolkien_button {
  background-image: url(Images/jrr_Tolkien.png);
}
.shakira_button {
  background-image: url(Images/shakira.jpg);
}*/

h1 {
  color: rgb(6, 6, 6);
  font-size: 80px;
  text-shadow: 4px 4px 1px rgb(76, 75, 75);

  position: fixed;
  left: 0;
  right: 0;
  -webkit-text-stroke: 0.01px;
  -webkit-text-stroke-color: rgb(249, 161, 9);
}

header {
  height: 120px;
  width: 100vw;
  background-color: rgba(140, 139, 139, 40%);
  position: fixed;

  padding: 15px;

  text-align: center;
}

ul {
  list-style: none;
}

.intro_text {
  color: rgb(70, 69, 69);
  margin-left: 55px;
  margin-right: 400px;
  margin-top: 200px;
  margin-bottom: 50px;
  font-size: 22px;

  padding: 50px;
}
a {
  color: rgb(19, 19, 18);
  font-size: larger;
}

.quicklinks {
  color: rgb(14, 14, 14);
  font-size: 20px;
  font-weight: bold;
  font-weight: 30em;
  margin-top: 30px;
}

.nav_plus_bios {
  display: flex;
  margin-top: 0px;
}

.main_section_excl_nav {
  margin-right: -70px;
  margin-top: 0px;
}
li {
  padding-top: 20px;
  padding-left: 0px;
  padding-right: 80px;
}
footer {
  margin-top: 150px;
  height: 100px;
  /*background-color: rgb(32, 2, 139);*/
  width: 100vw;
  margin-right: 0px;
}
a:hover {
  color: black;
  background-color: yellow;
}
a:visited {
  color: grey;
}

.navbar {
  margin-top: 200px;
}

/*.james_joyce_button {
  margin-left: 50px;
  display: block;
  background: linear-gradient(90deg, hsl(240, 1%, 33%), #dbe1e1);
 
  letter-spacing: 2px;
  border-radius: 50px;

  transition: ease-out 0.3s;
  text-decoration: underline;
  width: 100px;
  height: 100px;
}*/

.footerLI {
  color: rgb(235, 84, 9);
  width: 100%;
  display: inline;
  margin-left: 100px;
}
.footerLI:first-child {
  margin-left: 500px;
}

.quickfact_container_1,
.quickfact_container_2,
.quickfact_container_3,
.quickfact_container_4 {
  display: none;
  border-radius: 30px;
  height: fit-content;
}

.james_joyce_container,
.natalie_portman_container,
.jrr_tolkien_container,
.shakira_container {
  border-radius: 30px;
  height: fit-content;
}
/*.jj_image_container {
  position: relative;
  align-items: normal;
}*/
img {
  border-radius: 30px;
}

.quickfacts {
  margin-left: 1200px;
  margin-top: -1650px;

  width: 200px;
  text-align: center;
}
.quickfact_container_1,
.quickfact_container_2,
.quickfact_container_3,
.quickfact_container_4 {
  margin-bottom: 30px;
  border-radius: 30px;
  background-color: aqua;
  height: fit-content;
  padding: 10px;
  text-align: left;
}

.quickfact_button_1,
.quickfact_button_2,
.quickfact_button_3,
.quickfact_button_4 {
  margin-bottom: 50px;
  border-radius: 30px;
}
.quickfact1 {
  margin-top: 30px;
}

/*.bio_container_hover:hover {
  background-color: rgba(140, 139, 139, 40%);
  color: black;
 
  visibility: visible;

}*/

button:hover {
  background-color: yellow;
}

Script


let quickfactContainer1 = document.getElementById(
  "quickfact_container_1"
); /*.innerText*/
let quickfactButton1 = document.getElementById("quickfact_button_1");

function toggle1() {
  if (quickfactContainer1.style.display === "none") {
    quickfactContainer1.style.display = "flex";
    quickfactButton1.style.display = "none";
  } else {
    quickfactContainer1.style.display = "none";
  }

  let quickfactContainer2 = document.getElementById(
    "quickfact_container_2"
  ); /*..................innerText........*/
  let quickfactButton2 = document.getElementById("quickfact_button_2");

  function toggle2() {
    if (quickfactContainer2.style.display === "none") {
      quickfactContainer2.style.display = "flex";
      quickfactButton2.style.display = "none";
    } else {
      quickfactContainer2.style.display = "none";
    }
  }

  /*function toggle() {
  if (x.style.display === "none") {
    x.style.display = "block";
  } else {
    x.style.display = "none";
  }
this ONE WORKS*/

  let jamesJoyceContainer = getElementById("james_joyce_container");
  function toggle4() {
    if ((jamesJoyceContainer.display = "hidden")) {
      jamesJoyceContainer.display = "flex";
    } else {
      jamesJoyceContainer = "hidden";
    }
  }
}

Don’t nest function definitions inside other function definitions because if you do that the most outer function has to run before the nested function gets set up. Move toggle2 out of the definition of toggle1 and remember to move its dependencies as well (the selector code).

I would suggest you move all (or most of) your selector code to the top of the file or at least have them in the most outer scope and not nested inside functions.

getElementById must be called on document it is not a stand-alone function but a method on the document object.


I’d suggest you come up with a single reusable way of toggling that can be used without having the create a new function every time because that isn’t the correct way of doing it. I’d also create a class and use classList with the .toggle() method instead of inline styles.

BTW, You really need to validate your HTML.


You might also just use the details element. It doesn’t need any code to work.

Thank you. I have not even looked at validating the html as I was just trying to get the page to look how I wanted before ‘cleaning it all up’.

I hear you on the functions as well and acknowledge this is not the best way to do things (DRY principle), but I just wanted to figure it out first and address that second I suppose.

I’m still relatively new to all this, but I will work through your suggestions and see how it goes.

thanks for the help.

As long as you understand why the location of function definitions matters.

// Selector code
const someElement = document.querySelector('someSelector')

// function definitions

function fn1() {
  // code that run when fn1 is called
  // if fn2 was defined inside here, fn1 would need to run first before you can use fn2
}

function fn2() {
  // code that run when fn2 is called
}

Here is a quick example.

I don’t know that I would necessarily suggest relying on the location of DOM elements (nextElementSibling) but it is just an example of the basic idea of reusable code.

Example code
<div class="container">
  <button>Quick fact 1</button>
  <div class="hidden">Fact 1</div>
</div>
<div class="container">
  <button>Quick fact 2</button>
  <div class="hidden">Fact 2</div>
</div>
<div class="container">
  <button>Quick fact 3</button>
  <div class="hidden">Fact 3</div>
</div>
.hidden {
  display: none;
}
const btns = document.querySelectorAll("button");

btns.forEach((btn) => {
  btn.addEventListener("click", (evt) => {
    evt.target.nextElementSibling.classList.toggle("hidden");
  });
});

ok, thanks again.

I see what you mean about putting functions inside of functions. I had a second look at my code though and it doesn’t seem like toggle2 is nested inside of toggle1?

Maybe I’m missing something?

It is nested.

What you need is proper formatted code. Use VS Code and install the Prettier extension.

VS Code will also color code the curly brackets which can help.

If you put your cursor right after an opening curly bracket it will highlight where the closing bracket is. It also gives you vertical lines as indicators.


Comment added
function toggle1() { /* toggle1 starting code block */
  if (quickfactContainer1.style.display === "none") {
    quickfactContainer1.style.display = "flex";
    quickfactButton1.style.display = "none";
  } else {
    quickfactContainer1.style.display = "none";
  }

  let quickfactContainer2 = document.getElementById(
    "quickfact_container_2"
  ); /*..................innerText........*/
  let quickfactButton2 = document.getElementById("quickfact_button_2");

  function toggle2() {
    if (quickfactContainer2.style.display === "none") {
      quickfactContainer2.style.display = "flex";
      quickfactButton2.style.display = "none";
    } else {
      quickfactContainer2.style.display = "none";
    }
  }

  /*function toggle() {
  if (x.style.display === "none") {
    x.style.display = "block";
  } else {
    x.style.display = "none";
  }
this ONE WORKS*/

  let jamesJoyceContainer = getElementById("james_joyce_container");
  function toggle4() {
    if ((jamesJoyceContainer.display = "hidden")) {
      jamesJoyceContainer.display = "flex";
    } else {
      jamesJoyceContainer = "hidden";
    }
  }
} /* toggle1 ending code block */

ok. I am already using Prettier, but I will take a closer look. Thanks again.

That’s good. Just make sure you organize your code as well. It is easy to nest code inside other code if you aren’t careful.

Especially, if you start to comment out code and miss the end, then get a syntax error which you end up correcting by closing other code in a new place.

The same happens with HTML. If you aren’t careful the elements will not be closed properly in the correct place.

1 Like

great. Thanks for all the tips, I will keep them in mind…

just saw this part. Great. Thanks for the help again. Appreciated.

I have been trying to get around the reusing the same function problem and initially tried it using arrays. The way I approached it did not work. However, I am thinking it should be possible to get it to work in this way?

The confusion I have now is how to only trigger the function for a specific input (i.e. which button is clicked) as up until now I have only really had some experience with looping through arrays, which does not work in this case.

I will keep trying, but if you know any way I could use an array method and only trigger the function when a specific element of the array (button) is pressed, I would be grateful if you could point me in the right direction?

Thanks again…

I can’t answer that question without seeing exactly how you are planning to use an array for this.

There are lots of different ways of doing this. I would suggest you go through the new freeCodeCamp JS curriculum as it teaches plain JS DOM manipulation.


The option I showed the example code for is using DOM traversal. So you traverse the DOM from the button click target to the element it should hide/show. It doesn’t scale well and is less maintainable but it is very quick to code.


Another option is to add identifiers to the elements, for example, id attributes and/or data attributes.

Same example as before but using id and data attributes. If the button with data-toggle="toggle-1" is clicked the element with the id toggle-1 is selected (or 2 or 3).

Example code
<div class="container">
  <button data-toggle="toggle-1">Quick fact 1</button>
  <div class="hidden" id="toggle-1">Fact 1</div>
</div>
<div class="container">
  <button data-toggle="toggle-2">Quick fact 2</button>
  <div class="hidden" id="toggle-2">Fact 2</div>
</div>
<div class="container">
  <button data-toggle="toggle-3">Quick fact 3</button>
  <div class="hidden" id="toggle-3">Fact 3</div>
</div>
.hidden {
  display: none;
}
const btns = document.querySelectorAll("button");

btns.forEach((btn) => {
  btn.addEventListener("click", (evt) => {
    const toggleElement = document.getElementById(evt.target.dataset.toggle);
    toggleElement.classList.toggle("hidden");
  });
});

Yet another option would be to create the elements as needed. So the toggle element doesn’t exist (or its location) until it is created by the button click and added to the DOM (wherever you want it placed in the DOM).

This would include using template strings and doing something akin to conditional rendering using plain JS.

Or you can of course use a view framework like Svelte, Vue, Solid, React, etc., and do conditional rendering.

Great. Thanks for all the information. I am now thinking that I cannot use the same toggle function for each of these buttons as they do not do the same thing in the sense that they reveal different ‘text windows’ depending on which one is pressed.

There probably is a way to instruct the function to reveal say textbox1/2/3/4 depending on which button is pressed, but for now, I will probably just write a separate function for each and see if I can get those to work for now and maybe see if I can somehow reuse one function at a later point.

Thanks for all the information. I am still very new to all this, so if I can’t figure something out fairly quickly even after reading around the topic a little, I am just going with the ‘less efficient way’ with the intention of hoping improving things as time goes on.

Isn’t that what all the examples I have shown do?