2
\$\begingroup\$

I have a menu done in jQuery that I'd like some feedback on:

$(function() {
  // main expansion element
  $(".expander").click(function() {
    var subShown = $("ul > li", this).hasClass("show");
    if (!subShown) {
      $(".indented", this).slideDown('100').addClass("show");
      $(".caret", this).addClass("reversedCaret");
    } else {
      $(".indented", this).slideUp('100').removeClass("show");
      $(".caret", this).removeClass("reversedCaret");
      $(".indented--sub").slideUp('100').removeClass("show");
      $(".sub-caret").removeClass("reversedCaret");
      $(".more-or-less").text("More");
    }
  });

  // sub expansion element
  $(".sub-expander").click(function() {
    var subSelectText = $(".more-or-less").text();
    if (subSelectText != "More") {
      $(".indented--sub").slideUp('100').removeClass("show");
      $(".sub-caret").removeClass("reversedCaret");
      $(".more-or-less").text("More");
    } else {
      $(".indented--sub").slideDown('100').addClass("show");
      $(".sub-caret").addClass("reversedCaret");
      $(".more-or-less").text("Show Less");
    }
  });

  // stop propagation on the link element within .expander class
  $(".indented").click(function(event) {
    event.stopPropagation();
  });
});
.expander:hover {
  cursor: pointer;
}

.sub-expander--indented {
  padding: 0 0 0 23px;
}

.sub-caret {
  margin-right: 75px;
}

.indented,
.indented--sub {
  display: none;
}

.show {
  display: block;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.10.2/jquery.min.js"></script>

<div class="expander">
  <span class="caret downCaret right visibleCaret">+</span>
  <ul>
    <li class="category">Item 1
      <a href="http://www.google.com"></a>
    </li>
    <li class="indented"><a href="http://www.google.com">Item 2</a></li>
    <li class="indented"><a href="http://www.google.com">Item 3</a>

      <ul class="sub-expander more" style="padding-top: 
0px;">
        <li class="indented--sub"><a href="http://www.google.com" class="moreLiAs">Chapter 5</a></li>
        <li class="indented--sub"><a href="http://www.google.com" class="moreLiAs">Chapter 6</a></li>
        <li class="indented--sub"><a href="http://www.google.com" class="moreLiAs">Chapter 7</a></li>
        <span class="sub-caret moreCaret visibleLessCaret right">+</span>
        <li class="more-or-less less sub-expander--
indented">More</li>
      </ul>
    </li>
  </ul>
</div>

This just seems like a really over the top way to do it, but the one thing I'm having a hard time with is differentiating the sub menu from the main expander without it's own class/ID.

Do you have any tips on how to improve this?

\$\endgroup\$
1
\$\begingroup\$

Feedback

The code makes good use of the jQuery slide functions, as well as basic CSS class addition/removal.

One could utilize event delegation to use a single event listener on a parent container element, along with .is() and tree traversal methods like .parent().

Tips to improve this

Utilize .toggle() and .toggleClass(), along with dynamic function call using a variable (e.g. $(".indented", this)[slideFunction]('100') where slidefunction is a variable containing the function to call) to condense the code.

Also, as mentioned above, use event delegation to register a single click handler. Then check if the target is a child of various elements (e.g. with class indented, with class sub-expander, etc.), and handle actions accordingly.

$(function() {
  // main expansion element
  $(".expander").click(function(event) {
    var target = $(event.target);
    // stop propagation on the link element within .expander class
    if (!target.parent('.indented').length) {
      // sub expansion element
      if (target.parent('.sub-expander').length) {
        var subSelectTextIsMore = $(".more-or-less").text() === "More";
        var slideFunction = subSelectTextIsMore ? 'slideDown' : 'slideUp';

        $(".indented--sub")[slideFunction]('100').toggleClass("show", subSelectTextIsMore);
        $(".sub-caret").toggleClass("reversedCaret", subSelectTextIsMore);
        $(".more-or-less").text(subSelectTextIsMore ? "Show Less" : "More");
      } else { //expander
        var subShown = $("ul > li", this).hasClass("show");
        var slideFunction = subShown ? 'slideUp' : 'slideDown';
        $(".indented", this)[slideFunction]('100').toggleClass("show", !subShown);
        $(".caret", this).toggleClass("reversedCaret", !subShown);
        if (subShown) {
          $(".indented--sub").slideUp('100').removeClass("show");
          $(".sub-caret").removeClass("reversedCaret");
          $(".more-or-less").text("More");
        }
      }
    }
  });
});
.expander:hover {
  cursor: pointer;
}

.sub-expander--indented {
  padding: 0 0 0 23px;
}

.sub-caret {
  margin-right: 75px;
}

.indented,
.indented--sub {
  display: none;
}

.show {
  display: block;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.10.2/jquery.min.js"></script>

<div class="expander">
  <span class="caret downCaret right visibleCaret">+</span>
  <ul>
    <li class="category">Item 1
      <a href="http://www.google.com"></a>
    </li>
    <li class="indented"><a href="http://www.google.com">Item 2</a></li>
    <li class="indented"><a href="http://www.google.com">Item 3</a>

      <ul class="sub-expander more" style="padding-top: 
0px;">
        <li class="indented--sub"><a href="http://www.google.com" class="moreLiAs">Chapter 5</a></li>
        <li class="indented--sub"><a href="http://www.google.com" class="moreLiAs">Chapter 6</a></li>
        <li class="indented--sub"><a href="http://www.google.com" class="moreLiAs">Chapter 7</a></li>
        <span class="sub-caret moreCaret visibleLessCaret right">+</span>
        <li class="more-or-less less sub-expander--
indented">More</li>
      </ul>
    </li>
  </ul>
</div>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.