2
\$\begingroup\$

I'm toggling the class of an element with this code but I'm pretty sure it can be written more efficiently.

$('.expandable_header').click(function() {
    toggle_expandable_icon($(this).find('i')[0]);
    $(this).nextUntil('tr.expandable_header').slideToggle(100, function(){
    });
});

function toggle_expandable_icon(elem) {
    var toOpen = 'fa-plus-circle';
    var toClose = 'fa-minus-circle';
    if (elem.classList.contains(toOpen)) {
        elem.classList.replace(toOpen, toClose);
    } else {
        elem.classList.replace(toClose, toOpen);
}};

Not pointless to say js is not my mother tongue.

\$\endgroup\$
3
\$\begingroup\$

Use toggle methods

Presuming that you are just toggling those two classes, you could just use classList.toggle() with those two class names:

function toggle_expandable_icon(elem) {
    elem.classList.toggle('fa-plus-circle');
    elem.classList.toggle('fa-minus-circle');
}

And it appears jQuery is used, so the .toggleClass() method could be used on a jQuery collection (i.e. remove the [0] in toggle_expandable_icon($(this).find('i')[0]);):

function toggle_expandable_icon(collection) {
    collection.toggleClass('fa-plus-circle').toggleClass('fa-minus-circle');
}

Those class names could also be added together in a single toggleClass() call:

function toggle_expandable_icon(collection) {
    collection.toggleClass('fa-plus-circle fa-minus-circle');
}

Demo

Without knowing the HTML structure it was challenging to know exactly how the toggles operated, but it wasn't very difficult to generate something.

$('.expandable_header').click(function() {
    toggle_expandable_icon($(this).find('i'));
    $(this).nextUntil('tr.expandable_header').slideToggle("slow");
});

function toggle_expandable_icon(collection) {
    collection.toggleClass('fa-plus-circle fa-minus-circle');
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<link href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/5.8.2/css/all.css" rel="stylesheet" />
<table>
  <tr class="expandable_header">
    <td>
      <i class="fa fa-plus-circle"></i>
    </td>
  </tr>
  <tr style="display: none;" height="250px">
    <td>                                                           
                                                                                                                               <a href="https://codereview.meta.stackexchange.com/a/1511/120114" target="_blank">😱 Zombies exist! 😱</a>
    </td>
  </tr>
</table>

\$\endgroup\$
  • \$\begingroup\$ Excellent ! And neither I was aware for the zombies... \$\endgroup\$ – Emilio Conte Jun 14 at 7:54

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.