2
\$\begingroup\$

I know there is a better way to do the following but I havent had any luck trying to clean it up.

This is for a table of messages where the user can click a row, message-head, which will display the message-body-hidden to the user as well as change the class of message-head to message-head-active. It will also change the fontawesome icon. Along with that, any rows that are currently being displayed or are -active will be reverted back to their defaults.

How could I clean this up or more easily perform the same tasks?

$("table").on("click", "tr.message-head", function () {
    var active = $(this)
    active.toggleClass('message-head-active')

    if($("i",this).hasClass("fa-envelope-open")){
        $("i",this).removeClass("fa-envelope-open")
        $("i",this).addClass("fa-envelope")     
    }else{
        $("i",this).removeClass("fa-envelope")
        $("i",this).addClass("fa-envelope-open")
    }

    var showMe = active.next().find('.message-body-hidden')
    showMe.toggle()

    $("tr.message-head-active").not(active).removeClass('message-head-active')
    $("tr.message-head-active").not(active).addClass('message-head')

    $("tr.message-head").not(active).find('i').removeClass("fa-envelope-open")
    $("tr.message-head").not(active).find('i').addClass("fa-envelope")

    $("td.message-body-hidden").not(showMe).hide()
})  
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Move your revert all logic to the top of the function to avoid having to exclude the newly active item.

Use a stateful class such as .is-active instead of message-head-active. Means you only have to toggle 1 class rather than 2.

toggleClass accepts multiple arguments so you can toggle the FontAwesome classes in one line. This also removes the need for the hasClass check.

Store the result of $("tr.message-head-active") to a variable and reuse it when reverting the header and the icon.

\$\endgroup\$

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.