1
\$\begingroup\$

The following code is working exactly as it is expected to work, the thing is that I'm sure that it can be written in a more efficient and less verbose way, any reviews?

JavaScript:

$(document).ready(function() {
    $('.hidden-content').hide();
    $(".hidden-content-reveal").click(function() {
        var $i = $(this).children();
        if ($i.is(".angle-down")){
            $i.removeClass("angle-down");
            $i.addClass("angle-up");
        } else {
            $i.removeClass("angle-up");
            $i.addClass("angle-down");
        }
        return false;
    });
});

HTML:

<div>
    <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit.</p>
    <a href="#" class="hidden-content-reveal">
        <i class="angle-down" style="font-size:24px"></i>
    </a>
    <div class="hidden-content">
        <p>Nulla facilisi. Cras non egestas lectus. Duis malesuada eleifend neque, eget tincidunt elit convallis sed.Nulla facilisi. Cras non egestas lectus.</p>
    </div>
</div>
\$\endgroup\$
4
  • 1
    \$\begingroup\$ .toggleClass() for example? api.jquery.com/toggleclass \$\endgroup\$ Commented Feb 22, 2017 at 8:35
  • \$\begingroup\$ @MarioSantini .toggleClass() is working, but the "trick" is that I had to target both classes like that: toggleClass("angle-down angle-up"). Post your comment as an answer so to be able to accept it. \$\endgroup\$
    – mchar
    Commented Feb 22, 2017 at 9:02
  • \$\begingroup\$ I believe this is your first post, could you add a summary of what your code does to your post. \$\endgroup\$
    – Tolani
    Commented Feb 22, 2017 at 9:09
  • \$\begingroup\$ Does exactly that @Tushar has written to the below answer. \$\endgroup\$
    – mchar
    Commented Feb 22, 2017 at 9:14

1 Answer 1

4
\$\begingroup\$

$('.hidden-content').hide(); can be moved to CSS. This will be better UX and user will not see any flickr on the screen if page take long time to load and thus hide the element by JavaScript.

.hidden-content {
    display: none;
}

To check if an element have a class, hasClass() can be used instead of is().

The methods can be chained $i.removeClass("angle-down").addClass("angle-up"); but there is better way to do this.

To toggle the class on an element, toggleClass() can be used. This also accepts callback function.

$('selector').toggleClass(function() {
    return $(this).children().hasClass('angle-down') ? 'angle-up' : 'angle-down';
});

toggleClass also accepts list of classes separated by space. If the element have already applied one of the class from it, toggleClass will remove it and add other classes. Now, the code will become,

$('selector').toggleClass('fa-angle-down fa-angle-up');

Demo:

Note that in Demo, I've added Font-Awesome to demonstrate the functionality.

$(document).ready(function() {
  $(".hidden-content-reveal").click(function() {
    $(this).children().toggleClass('fa-angle-down fa-angle-up');

    $(this).next('.hidden-content').slideToggle();
    return false;
  });
});
.hidden-content {
  display: none;
}
<link href="https://maxcdn.bootstrapcdn.com/font-awesome/4.7.0/css/font-awesome.min.css" rel="stylesheet" />
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div>
  <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit.</p>
  <a href="#" class="hidden-content-reveal">
    <i class="fa fa-angle-down" style="font-size:24px"></i>
  </a>
  <div class="hidden-content">
    <p>Nulla facilisi. Cras non egestas lectus. Duis malesuada eleifend neque, eget tincidunt elit convallis sed.Nulla facilisi. Cras non egestas lectus.</p>
  </div>
</div>

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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