Maiya
Maiya

Reputation: 980

JavaScript event-handler: Toggle visibility of adjacent element. Code not working

I have nested unordered-lists. Each unordered-list has an h1 tag as its 'previous-sibling' tag. My goal is to click on an h1 tag and toggle the visibility of the unordered-list that comes right after it.

I am also trying to also assign classNames to each of unordered lists, based on the their title (h1) tag.

Is anyone able to help me understand why my code is not working?

Here is the code:

window.onload = function() {
  
  let titles = document.getElementsByTagName('h1');
  for (let i = 0 ; i < titles.length ; i++) {
    let title = titles[i];
    //grab the text in the h1 element, and add that text as a class to the h1:
    let className = title.innerHTML.toLowerCase();
    title.className += title.className.length ?  ' ' + className : className;
    let section = document.querySelectorAll(`.${className} + ul`);
    if(section.length) {
      section[0].className += section[0].className.length 
      ?  
      ` ${className} section` 
      : 
      `${className} section`;
      //ADD EVENT HANDLER TO THE TITLE.
      //SHOULD HIDE AND SHOW THE ADJASCENT UL ELEMENT:
      title.onclick = function(e) {
        section[0].classList.toggle('hide'); 
      };
    }
  } 
};
/* trying to toggle visibility with this class*/

.hide {
  display: none;
}

/*basic styles to separate elements:*/

h1 {
  color: olive;
}

ul {
  border: solid orange 1px;
}


li {
  //border: solid magenta 1px;
}
<div id="foods">
  <h1>Food</h1>
  <ul>
    <li>
      <h1>Fruit</h1>
      <ul>
        <li>
          <h1>tropical</h1>
          <ul>
            <li>banana</li>
            <li>pineapple</li>
            <li>mango</li>
          </ul>
        </li>
        <li>
          <h1>stone</h1>
          <ul>
            <li>peach</li>
            <li>pear</li>
            <li>appricot</li>
          </ul>
        </li>
      </ul>
    </li>
    <li>
      <h1>Vegetables</h1>
      <ul>
        <li>
          <h1>leafy</h1>
          <ul>
            <li>lettuce</li>
            <li>spinach</li>
            <li>kale</li>
           </ul>
        </li>
        <li>
          <h1>root</h1>
          <ul>
            <li>carrot</li>
            <li>turnip</li>
            <li>potato</li>
           </ul>
        </li>
      </ul>
    </li>
  </ul> 
</div>

Upvotes: 1

Views: 51

Answers (1)

T.J. Crowder
T.J. Crowder

Reputation: 1074495

The chief issue is you're using the wrong CSS property. You want display, not visibility:

.hide {
  display: none;
}

If you were using visibility, the value would be hidden, but it would continue to take up room in the layout, which I'm sure isn't what you want.

But separately, the code to hook up those event handlers and add those classes (you've said you're using them for things later) can be a bit simpler:

window.onload = function() {
  function toggleNext() {
    this.nextElementSibling.classList.toggle("hide");
  }
  let titles = document.getElementsByTagName('h1');
  for (let i = 0 ; i < titles.length ; i++) {
    const title = titles[i];
    const section = title.nextElementSibling;
    const titleClass = title.innerHTML.toLowerCase();
    title.classList.add(titleClass);
    section.classList.add(titleClass, "section");
    titles[i].addEventListener("click", toggleNext);
  } 
};

Live Example:

window.onload = function() {
  function toggleNext() {
    this.nextElementSibling.classList.toggle("hide");
  }
  let titles = document.getElementsByTagName('h1');
  for (let i = 0 ; i < titles.length ; i++) {
    const title = titles[i];
    const section = title.nextElementSibling;
    const titleClass = title.innerHTML.toLowerCase();
    title.classList.add(titleClass);
    section.classList.add(titleClass, "section");
    titles[i].addEventListener("click", toggleNext);
  } 
};
/* trying to toggle visibility with this class*/

.hide {
  display: none;
}

/*basic styles to separate elements:*/

h1 {
  color: olive;
}

ul {
  border: solid orange 1px;
}


li {
  //border: solid magenta 1px;
}
<div id="foods">
  <h1>Food</h1>
  <ul>
    <li>
      <h1>Fruit</h1>
      <ul>
        <li>
          <h1>tropical</h1>
          <ul>
            <li>banana</li>
            <li>pineapple</li>
            <li>mango</li>
          </ul>
        </li>
        <li>
          <h1>stone</h1>
          <ul>
            <li>peach</li>
            <li>pear</li>
            <li>appricot</li>
          </ul>
        </li>
      </ul>
    </li>
    <li>
      <h1>Vegetables</h1>
      <ul>
        <li>
          <h1>leafy</h1>
          <ul>
            <li>lettuce</li>
            <li>spinach</li>
            <li>kale</li>
           </ul>
        </li>
        <li>
          <h1>root</h1>
          <ul>
            <li>carrot</li>
            <li>turnip</li>
            <li>potato</li>
           </ul>
        </li>
      </ul>
    </li>
  </ul> 
</div>


Side note: I would recommend not using window.load unless you really need this code to wait for all images, stylesheets, etc., to fully load before running. Instead, just make sure the code is in a script tag at the end of the document, just prior to the closing </body> tag.

Upvotes: 3

Related Questions