CODEWITHSUNDEEP
javascriptjqueryif-statement
theintellects
theintellects

Reputation: 1370

if statement not functioning correctly

I can't seem to get my arrows to show in the right direction when a collapsing item is collapsed or expanded (JSFiddle)

Code:

$('body').on("click", "span#expandcollapse", function ()
{
    $(this).text(function(i, currentText)
    {
        return currentText === 'Expand All' ? 'Collapse All' : 'Expand All';
    });

    if($(this).text() === 'Expand All')
    {
        //Currently collapsed
        if($('.projectscontainer').find("div.destarrow").hasClass('arrow-right'))
            $('.projectscontainer').find('div.destarrow').toggleClass("arrow-right arrow-down");
        $(".projectscontainer").find(".srcprojects").toggle(false);
    } 
    else 
    {
        //Currently expanded
        $(".projectscontainer").find(".srcprojects").toggle(true);
        if($('.projectscontainer').find("div.destarrow").hasClass('arrow-down'))
            $('.projectscontainer').find('div.destarrow').toggleClass("arrow-down arrow-right");
    }
});

This line is not working as intended:

    if($('.projectscontainer').find("div.destarrow").hasClass('arrow-down'))
        $('.projectscontainer').find('div.destarrow').toggleClass("arrow-down arrow-right");

When I click expand all, the items should expand (working), and the arrows should point down (not working). When I click collapse all, the opposite should occur. For example, if I click one of the green items, then click expand all, the arrow for the item clicked is in a different direction.

What am I doing wrong? And are there any performance problems here I need to fix if I have a huge list? Thanks!

Upvotes: 1

Views: 108

Answers (2)

castis
castis

Reputation: 8223

NOTE: tutiputes answer is correct, go with theirs.

Here is a working fiddle

You changed the text and then check for its previous state to act accordingly.

Store the text, and check again after the change.

var currText = $(this).text();
$(this).text(function(i, currentText){
    return currentText === 'Expand All' ? 'Collapse All' : 'Expand All';
});
if(currText === 'Expand All'){//Currently collapsed

Also, your toggles were backwards. You want to expand when collapsed, and vice versa.

You asked about performance issues.

http://jsfiddle.net/wuqes/2/

You might think about storing your jQuery selectors so you're not constantly rooting through the DOM. It's not a huge deal but depending on the size of future projects, it will come in handy.

Upvotes: 2

sagibb
sagibb

Reputation: 956

Here is a working JSFiddle.

Your issue was actually with the fact that the below code

$('.projectscontainer').find("div.destarrow").hasClass('arrow-right')

always returns true in this specific case since $('.projectscontainer').find("div.destarrow") returns the array of arrow elements that contains some arrows with arrow-right and some arrows with the arrow-down class, hence always returning true. I've re-factored the code to use a filter.

Upvotes: 1

Related Questions