Issue with stop(true).slideDown() during slideUp(), is a jquery bug?

Question

I have the code for a collapse menu pasted below or see it on jsfiddle.

The issue is when the submenu is in the sliding up state, and it is clicked again, it stops but is not being animated backwards (instead the submenus are jumping out but their container is keeping its current height - no jump with jQuery 2.0.0.b1). Tested in Firefox and Chrome with the same result.

Is this a jQuery bug, or just me?

Update: I have filed a bug report for jQuery, because I think that this is a jQuery bug.

Update 2: See Brad M's accepted answer with the explanation of this behavior (but I still think that this is an issue with jQuery which needs to be solved internally).

HTML

<ul>
    <li><a href="#">Menu1</a></li>
    <li class="opened"><a href="#">Submenus 1</a>
        <ul class="submenu">
            <li><a href="#">submenu1-1</a></li>
            <li><a href="#">submenu1-2</a></li>
        </ul></li>
    <li class="closed"><a href="#">Submenus 2</a>
        <ul class="submenu">
            <li><a href="#">submenu2-1</a></li>
            <li><a href="#">submenu2-2</a></li>
        </ul></li>
    <li><a href="#">Menu4</a>
    </li>
</ul>

Javascript

var initMenu = function() {
    $("li.closed > ul.submenu").css("display", "none");

    var toggleMenu = function(li) {
        if (li.hasClass("opened") || li.hasClass("inslidedown")) {
            li.removeClass("opened inslidedown").addClass("inslideup").find("ul").first().stop(true).slideUp({
                duration: 1000,
                complete: function() {
                    li.addClass("closed").removeClass("inslideup");
                }
            });
        }
        else if (li.hasClass("closed") || li.hasClass("inslideup")) {
            li.removeClass("closed inslideup").addClass("inslidedown").find("ul").first().stop(true).slideDown({
                duration: 1000,
                complete: function() {
                    li.addClass("opened").removeClass("inslidedown");
                }
            });
        }
    };

    $("ul").on("click", "li.opened a, li.closed a, li.inslideup a, li.inslidedown a", function(e) {
        e.preventDefault();
        toggleMenu($(this).closest("li"));
    });
};

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

Answer 1

The reason .slideDown() isn't working is because the element's display is set to "block" and .slideDown() is generally intended to be called on elements with display "none". Include .hide() on the ul element in your else if statement to get the behavior you desire.

li
    .removeClass("closed inslideup")
    .addClass("inslidedown")
    .find("ul")
    .first()
    .hide()   //   <----
    .stop(true)
    .slideDown();

Answer 2

For now I am using a workaround that simply ignores the click event while the sliding animation is in progress. Just deleted the li.inslideup a, li.inslidedown a selectors from .on()'s selector filtering parameter on line 23 (left the other code intact in case that jQuery gets fixed, but it can be simpilfied).

The workaround is on jsfiddle.

Answer 3

The first issue is due to your usage of .stop()

According to the jquery docs,

.stop( [clearQueue ] [, jumpToEnd ] )

clearQueue Type: Boolean A Boolean indicating whether to remove queued animation as well. Defaults to false.

jumpToEnd Type: Boolean A Boolean indicating whether to complete the current animation immediately. Defaults to false.

Generally, you want to use .stop(false, true) when dealing with animations.

As for your second issue, the reason is due to the following jquery / css.

li.removeClass("closed inslideup")

li.closed > ul { display: none; }

When you remove the "closed" class, the ul element's display is set back to the default "block", and thus it shows instantly. The slidedown animation doesn't occur because the element is already showing (the callback still executes though).

Answer 4

Basically you should check whether all <li> nodes under <ul> have the css class say closed or opened, you can use jQuery selector to achieve that.

Here is a quick and dirty way to fix, you can definitely re-factor it to better logic.

if ($('li[class~="opened"]').length > 0 || $('li[class~="inslidedown"]').length > 0) {
    ...
}
else if ($('li[class~="closed"]').length > 0 || $('li[class~="inslideup"]').length > 0) {
    ...
}

Answer 5

Here is my version: http://jsfiddle.net/RF3xK/4/

Javascript:

$(document).ready(function() {
rotator($('#quotes .textItem:first'));

function rotator(elem) {
    elem.slideDown(800, function(){
      var me = $(this),
          next = me.next(),
          prev = me.prev();
      if (!next.length) {
          next = $('.textItem').first();
          next.insertAfter(me);
      }
      rotator(next);
    }).delay(800).slideUp(800);
}
});

CSS:

#quotes { background: #eee;}

#quotes .textItem { display: none; }

Note that I replaced the .hide() to those elements on the document.ready event, is unnecesary javascript, you can do it with css like I did, the other problem with the js way is that for a tiny moment the appear and disappear because you have to wait until all the document loads to hide those elements.