Jquery accordion not removingClass and not swapping its Expand text for Collapse

Question

Thanks to @ifaour for a lot of his help! This script includes:

• Jquery accordion using unordered lists.
• Active and Inactive states with toggling bullet arrow images
• Expand All / Collapse All that swaps its text.
• Equal height columns that expand and collapse when the accordion expands and collapses

You can view a demo here http://jsbin.com/ucobo3/24/

(function($) {
$.fn.equalHeights = function(minHeight, maxHeight) {
tallest = (minHeight) ? minHeight : 0;
this.each(function() {
if($(this).height() > tallest) {
tallest = $(this).height();
}
});
if((maxHeight) && tallest > maxHeight) tallest = maxHeight;
return this.each(function() {
if ($.browser.msie && $.browser.version == 6.0) { $(this).height(tallest); }
$(this).css('min-height', tallest + 'px').css('overflow', 'auto');
});
}
})(jQuery);

jQuery.fn.initMenu = function() {
  var isCollapsed = true; // default value
  var collapseAll = 'Collapse All';
  var expandAll = 'Expand All';

  $('.swap').click(function() {
    if(!isCollapsed)
    {
      $('ul.menu li ul').slideUp('normal');
      $('ul.menu li').removeClass('active');
      $(this).text(expandAll);

      isCollapsed = true;
    } else {
      $('ul.menu li ul').slideDown('normal');
      $('ul.menu li').addClass('active');
      $(this).text(collapseAll);
      isCollapsed = false;
    }
    return false;
  });

  return this.each(function(){
    var theMenu = $(this).get(0);
    $('.acitem', this).hide();
    $('li.expand > .acitem', this).show();
    $('li.expand > .acitem', this).prev().addClass('active');
    $('li a', this).click(
    function(e) {
      e.stopImmediatePropagation();
      var theElement = $(this).next();
      var parent = this.parentNode.parentNode;
      if($(parent).hasClass('noaccordion')) {
        if(theElement[0] === undefined) {
          window.location.href = this.href;
        }
        $(theElement).slideToggle('normal', function() {
          if ($(this).is(':visible')) {
            $(this).parent().addClass('active');
          }
          else {
            $(this).prev().parent().removeClass('active');
          }    
        });
        return false;
      }
      else {
        if(theElement.hasClass('acitem') && theElement.is(':visible')) {
          if($(parent).hasClass('collapsible')) {
            $('.acitem:visible', parent).first().slideUp('normal',
            function() {
              $(this).prev().parent().removeClass('active');
            }
          );
            return false;  
          }
          return false;
        }
        if(theElement.hasClass('acitem') && !theElement.is(':visible')) {         
          $('.acitem:visible', parent).first().slideUp('normal', function() {
            $(this).prev().parent().removeClass('active');
          });
          theElement.slideDown('normal', function() {
            $(this).prev().parent().addClass('active');
          });
          return false;
        }


      }
    }
  );

  });
};

$(document).ready(function() {
  $('.menu').initMenu();          
  $('.column').equalHeights();
});

Thanks!!

Answer 1

Here's my try:

jQuery.fn.initMenu = function() {
    var isCollapsed = true; // default value
    var collapseAll = 'Collapse All';
    var expandAll = 'Expand All';

    $('.swap').click(function() {
        if(!isCollapsed)
        {
            $('ul.menu li ul').slideToggle('normal');
            $('ul.menu li a').removeClass('active');
            $(this).text(expandAll);
            isCollapsed = true;
        } else {
            $('ul.menu li ul').slideToggle('normal');
            $('ul.menu li a').addClass('active');
            $(this).text(collapseAll);
            isCollapsed = false;
        }
        return false;
    });

    return this.each(function(){
        var theMenu = $(this).get(0);
        $('.acitem', this).hide();
        $('li.expand > .acitem', this).show();
        $('li.expand > .acitem', this).prev().addClass('active');
        $('li a', this).click(
        function(e) {
            e.stopImmediatePropagation();
            var theElement = $(this).next();
            var parent = this.parentNode.parentNode;
            if($(parent).hasClass('noaccordion')) {
                if(theElement[0] === undefined) {
                    window.location.href = this.href;
                }
                $(theElement).slideToggle('normal', function() {
                    if ($(this).is(':visible')) {
                        $(this).addClass('active');
                    }
                    else {
                        $(this).prev().removeClass('active');
                    }    
                });
                return false;
            }
            else {
                if(theElement.hasClass('acitem') && theElement.is(':visible')) {
                    if($(parent).hasClass('collapsible')) {
                        $('.acitem:visible', parent).first().slideUp('normal',
                        function() {
                            $(this).prev().removeClass('active');
                        }
                    );
                        return false;  
                    }
                    return false;
                }
                if(theElement.hasClass('acitem') && !theElement.is(':visible')) {         
                    $('.acitem:visible', parent).first().slideUp('normal', function() {
                        $(this).prev().removeClass('active');
                    });
                    theElement.slideDown('normal', function() {
                        $(this).prev().addClass('active');
                    });
                    return false;
                }


            }
        }
    );

    });
};

$(document).ready(function() {$('.menu').initMenu();});

Example link.
Since the .swap link is common for all, no need to have it inside the each loop.

Answer 2

You're pretty close, the problem exists in where you're using .text() to set the link.

$(this).text($(this).text().prev() == 'Click to Collapse');

That line of code attempts to set the text on $(this) with the return of $(this).text().prev() == 'Click to Collapse');

$(this).text() returns the string "Click to Collapse".

.prev() is not a valid function on strings, thus will result in a javascript error.

What you want to do is $(this).text("Click to Expand");

The end result will be something like this:

$('.swap').click(function()
            {
                if($(this).text() == 'Click to Collapse')
                {
                    $('ul.menu li ul').slideToggle('normal');
                $('ul.menu li a').removeClass('active');
                    $(this).text('Click to Expand');
                }else{
                    $('ul.menu li ul').slideToggle('normal');
                $('ul.menu li a').addClass('active');
                    $(this).text('Click to Collapse');
                    }
            }
    );