4
\$\begingroup\$

Below is some jQuery I recently wrote and I'm quite sure there is a more elegant and/or efficient way to write it. The gist is, when a user clicks on a Primary Nav link, its Secondary menu slides down. When a user then clicks another Primary Nav link, instead of the jumpiness if the prev menu sliding up and the new one sliding down, they just toggle. This works (see below) but there must be a better way.

The code in question:

 if ($(this).attr('class') == 'primary-item-a') {
var siblingOpen = 0;
$parent_li.siblings().find('ul').each(function(index) {
  if ($(this).css('display') == 'block') {
    siblingOpen++;
  }
  return siblingOpen;
});

if (siblingOpen >= 1) {
  $parent_li.children('ul').toggle();
} else {
  $parent_li.children('ul').slideToggle('fast');
}

} else {
    $parent_li.children('ul').slideToggle('fast');
  }

All of the HTML/CSS/jQuery in a Stack Snippet:

function closeTertiaryMenus() {
  $('ul.tertiary').each(function(index) {
    if ($(this).css('display') == 'block') {
      $(this).hide();
    }
  });
}

function removeSecActBtnClr() {
  $('nav li.secondary-item > a').each(function(index) {
    $(this).removeClass();
  });
}

$('nav li.primary-item > a').click(function() {
  closeTertiaryMenus();
  removeSecActBtnClr();
});
///////////////  THE CODE IN QUESTION  ///////////////
$('nav a').click(function() {
  $parent_li = $(this).parent('li');
  // ************** ALTHOUGH MAINLY THIS CODE **************   
  if ($(this).attr('class') == 'primary-item-a') {
    var siblingOpen = 0;
    $parent_li.siblings().find('ul').each(function(index) {
      if ($(this).css('display') == 'block') {
        siblingOpen++;
      }
      return siblingOpen;
    });

    if (siblingOpen >= 1) {
      $parent_li.children('ul').toggle();
    } else {
      $parent_li.children('ul').slideToggle('fast');
    }

  } else {
    $parent_li.children('ul').slideToggle('fast');
  }
  //***************************************
  // Hide the active menus of siblings 
  $parent_li.siblings().find('ul').hide();
});
////////////////////////////////////////////
 body,
        nav,
        ul,
        li,
        a {
          margin: 0;
          padding: 0;
        }

        a {
          text-decoration: none;
          color: #fff;
        }

        nav {
          background: #005838;
        }

        ul {
          list-style: none;
        }

        ul.primary,
        ul.secondary {
          padding: 0 10px;
        }

        nav ul.primary {
          position: relative;
        }

        li.primary-item,
        li.secondary-item {
          display: inline;
        }

        nav a {
          display: inline-block;
          text-transform: uppercase;
          color: #fff;
          font-weight: bold;
          padding: 0.75rem;
          margin: 0.5rem 0;
          font-size: 1.125rem;
        }

        nav ul li.primary-item > a:hover {
          text-decoration: none;
          background-color: #008050;
        }

        nav li.primary-item > a.btn-active {
          background-color: #008050;
        }

        nav ul li.secondary-item > a:hover {
          text-decoration: none;
          background-color: #9dcb6b;
        }

        nav li.secondary-item > a.btn-active {
          background-color: #9dcb6b;
        }

        nav ul li.tertiary-item > a {
          display: block;
        }

        nav ul li.tertiary-item > a:hover {
          text-decoration: none;
          background-color: #769950;
        }

        nav ul.secondary {
          display: none;
          position: absolute;
          left: 0;
          width: 100%;
          background: #008050;
        }

        nav ul.secondary li.secondary-item a {
          padding: 0.3rem 0.75rem;
          ;
        }

        nav ul.tertiary {
          display: none;
          position: absolute;
          z-index: 9999;
          background: #9dcb6b;
        }

        nav ul.tertiary li.tertiary-item {
          border-bottom: 1px solid #7EA356;
        }

        nav ul.tertiary li.tertiary-item a {
          padding: 0.75rem;
          margin: 0;
        }
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.6.4/jquery.min.js"></script>
<nav>
  <ul class="primary">
    <li class="primary-item">
      <a class="primary-item-a" href="#">Primary 1</a>
      <ul class="secondary">
        <li class="secondary-item"><a href="#">P1 - Secondary 1</a>
          <ul class="tertiary">
            <li class="tertiary-item"><a href="#">P1 - S1 - Tertiary 1</a></li>
            <li class="tertiary-item"><a href="#">P1 - S1 - Tertiary 2</a></li>
          </ul>
        </li>
        <li class="secondary-item"><a href="#">P1 - Secondary 2</a>
          <ul class="tertiary">
            <li class="tertiary-item"><a href="#">P1 - S2 - Tertiary 1</a></li>
            <li class="tertiary-item"><a href="#">P1 - S2 - Tertiary 2</a></li>
          </ul>
        </li>
        <li class="secondary-item"><a href="#">P1 - Secondary 3</a>
          <ul class="tertiary">
            <li class="tertiary-item"><a href="#">P1 - S3 - Tertiary 1</a></li>
            <li class="tertiary-item"><a href="#">P1 - S3 - Tertiary 2</a></li>
          </ul>
        </li>
      </ul>
    </li>
    <li class="primary-item">
      <a class="primary-item-a" href="#">Primary 2</a>
      <ul class="secondary">
        <li class="secondary-item"><a href="#">P2 - Secondary 1</a>
          <ul class="tertiary">
            <li class="tertiary-item"><a href="#">P2 - S1 - Tertiary 1</a></li>
            <li class="tertiary-item"><a href="#">P2 - S1 - Tertiary 2</a></li>
          </ul>
        </li>
        <li class="secondary-item"><a href="#">P2 - Secondary 2</a>
          <ul class="tertiary">
            <li class="tertiary-item"><a href="#">P2 - S2 - Tertiary 1</a></li>
            <li class="tertiary-item"><a href="#">P2 - S2 - Tertiary 2</a></li>
          </ul>
        </li>
        <li class="secondary-item"><a href="#">P2 - Secondary 3</a>
          <ul class="tertiary">
            <li class="tertiary-item"><a href="#">P2 - S3 - Tertiary 1</a></li>
            <li class="tertiary-item"><a href="#">P2 - S3 - Tertiary 2</a></li>
          </ul>
        </li>
      </ul>
    </li>
  </ul>
</nav>

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Here's the equivalent code.

// Check if this element has the specified class
if ($(this).hasClass('primary-item-a')) {

    // If there are one or more visible `ul`s
    if ($parent_li.siblings().find('ul:visible').length >= 1) {
        $parent_li.children('ul').toggle();
    } else {
        $parent_li.children('ul').slideToggle('fast');
    }
} else {
    $parent_li.children('ul').slideToggle('fast');
}
  1. Use hasClass() to check if an element is having a class. attr('class') = 'something' will fail if the element is having more than one class
  2. Instead of iterating over uls and counting visible uls, use :visible pseudo-selector. The same applies for the closeTertiaryMenus() and removeSecActBtnClr()See Live code snippet
  3. You cannot return from the each(). Only when return false; is used in each(), the loop is exited.

function closeTertiaryMenus() {
  $('ul.tertiary:visible').hide();
}

function removeSecActBtnClr() {
  $('nav li.secondary-item > a').removeClass();
}

$('nav li.primary-item > a').click(function() {
  closeTertiaryMenus();
  removeSecActBtnClr();
});


$('nav a').click(function() {
  $parent_li = $(this).parent('li');
  // ************** ALTHOUGH MAINLY THIS CODE **************
  if ($(this).hasClass('primary-item-a')) {
    if ($parent_li.siblings().find('ul:visible').length >= 1) {
      $parent_li.children('ul').toggle();
    } else {
      $parent_li.children('ul').slideToggle('fast');
    }
  } else {
    $parent_li.children('ul').slideToggle('fast');
  }

  //***************************************
  // Hide the active menus of siblings
  $parent_li.siblings().find('ul').hide();
});
////////////////////////////////////////////
body,
nav,
ul,
li,
a {
  margin: 0;
  padding: 0;
}
a {
  text-decoration: none;
  color: #fff;
}
nav {
  background: #005838;
}
ul {
  list-style: none;
}
ul.primary,
ul.secondary {
  padding: 0 10px;
}
nav ul.primary {
  position: relative;
}
li.primary-item,
li.secondary-item {
  display: inline;
}
nav a {
  display: inline-block;
  text-transform: uppercase;
  color: #fff;
  font-weight: bold;
  padding: 0.75rem;
  margin: 0.5rem 0;
  font-size: 1.125rem;
}
nav ul li.primary-item > a:hover {
  text-decoration: none;
  background-color: #008050;
}
nav li.primary-item > a.btn-active {
  background-color: #008050;
}
nav ul li.secondary-item > a:hover {
  text-decoration: none;
  background-color: #9dcb6b;
}
nav li.secondary-item > a.btn-active {
  background-color: #9dcb6b;
}
nav ul li.tertiary-item > a {
  display: block;
}
nav ul li.tertiary-item > a:hover {
  text-decoration: none;
  background-color: #769950;
}
nav ul.secondary {
  display: none;
  position: absolute;
  left: 0;
  width: 100%;
  background: #008050;
}
nav ul.secondary li.secondary-item a {
  padding: 0.3rem 0.75rem;
  ;
}
nav ul.tertiary {
  display: none;
  position: absolute;
  z-index: 9999;
  background: #9dcb6b;
}
nav ul.tertiary li.tertiary-item {
  border-bottom: 1px solid #7EA356;
}
nav ul.tertiary li.tertiary-item a {
  padding: 0.75rem;
  margin: 0;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.6.4/jquery.min.js"></script>
<nav>
  <ul class="primary">
    <li class="primary-item">
      <a class="primary-item-a" href="#">Primary 1</a>
      <ul class="secondary">
        <li class="secondary-item"><a href="#">P1 - Secondary 1</a>
          <ul class="tertiary">
            <li class="tertiary-item"><a href="#">P1 - S1 - Tertiary 1</a>
            </li>
            <li class="tertiary-item"><a href="#">P1 - S1 - Tertiary 2</a>
            </li>
          </ul>
        </li>
        <li class="secondary-item"><a href="#">P1 - Secondary 2</a>
          <ul class="tertiary">
            <li class="tertiary-item"><a href="#">P1 - S2 - Tertiary 1</a>
            </li>
            <li class="tertiary-item"><a href="#">P1 - S2 - Tertiary 2</a>
            </li>
          </ul>
        </li>
        <li class="secondary-item"><a href="#">P1 - Secondary 3</a>
          <ul class="tertiary">
            <li class="tertiary-item"><a href="#">P1 - S3 - Tertiary 1</a>
            </li>
            <li class="tertiary-item"><a href="#">P1 - S3 - Tertiary 2</a>
            </li>
          </ul>
        </li>
      </ul>
    </li>
    <li class="primary-item">
      <a class="primary-item-a" href="#">Primary 2</a>
      <ul class="secondary">
        <li class="secondary-item"><a href="#">P2 - Secondary 1</a>
          <ul class="tertiary">
            <li class="tertiary-item"><a href="#">P2 - S1 - Tertiary 1</a>
            </li>
            <li class="tertiary-item"><a href="#">P2 - S1 - Tertiary 2</a>
            </li>
          </ul>
        </li>
        <li class="secondary-item"><a href="#">P2 - Secondary 2</a>
          <ul class="tertiary">
            <li class="tertiary-item"><a href="#">P2 - S2 - Tertiary 1</a>
            </li>
            <li class="tertiary-item"><a href="#">P2 - S2 - Tertiary 2</a>
            </li>
          </ul>
        </li>
        <li class="secondary-item"><a href="#">P2 - Secondary 3</a>
          <ul class="tertiary">
            <li class="tertiary-item"><a href="#">P2 - S3 - Tertiary 1</a>
            </li>
            <li class="tertiary-item"><a href="#">P2 - S3 - Tertiary 2</a>
            </li>
          </ul>
        </li>
      </ul>
    </li>
  </ul>
</nav>

\$\endgroup\$
1
  • \$\begingroup\$ Thank you so much for your time and your answer. This seems to be way more streamlined and easier to read. I learned something new today, this makes it a good day! :) \$\endgroup\$ Commented Aug 19, 2016 at 21:29

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.