Bug with .stop() and slideToggle and maybe other toggles too about jquery.animate

[baijunjie]

Toggle method is a little different with the original methods, mainly in the process of toggle stop, and again after the toggle, the plug-in will continue to complete, and the original jquery will reverse.

My plug-in is through the DOM elements "display" to determine the direction of the toggle.

But it is unclear whether the original jquery is through what to decide.

[KingSora]

Hm, maybe you could just provide the original jQuery functionality in this case. That would be a ok solution until you know how to fix it.
I've checked how jQuery is doing it (https://code.jquery.com/jquery-1.12.3.js and I've found this:

var isHidden = function( elem, el ) {

        // isHidden might be called from jQuery#filter function;
        // in that case, element will be second argument
        elem = el || elem;
        return jQuery.css( elem, "display" ) === "none" ||
            !jQuery.contains( elem.ownerDocument, elem );
    };

toggle: function( state ) {
        if ( typeof state === "boolean" ) {
            return state ? this.show() : this.hide();
        }

        return this.each( function() {
            if ( isHidden( this ) ) {
                jQuery( this ).show();
            } else {
                jQuery( this ).hide();
            }
        } );
    }


...

function defaultPrefilter( elem, props, opts ) {
    /* jshint validthis: true */
    var prop, value, toggle, tween, hooks, oldfire, display, checkDisplay,
        anim = this,
        orig = {},
        style = elem.style,
        hidden = elem.nodeType && isHidden( elem ),
        dataShow = jQuery._data( elem, "fxshow" );

    // handle queue: false promises
    if ( !opts.queue ) {
        hooks = jQuery._queueHooks( elem, "fx" );
        if ( hooks.unqueued == null ) {
            hooks.unqueued = 0;
            oldfire = hooks.empty.fire;
            hooks.empty.fire = function() {
                if ( !hooks.unqueued ) {
                    oldfire();
                }
            };
        }
        hooks.unqueued++;

        anim.always( function() {

            // doing this makes sure that the complete handler will be called
            // before this completes
            anim.always( function() {
                hooks.unqueued--;
                if ( !jQuery.queue( elem, "fx" ).length ) {
                    hooks.empty.fire();
                }
            } );
        } );
    }

    // height/width overflow pass
    if ( elem.nodeType === 1 && ( "height" in props || "width" in props ) ) {

        // Make sure that nothing sneaks out
        // Record all 3 overflow attributes because IE does not
        // change the overflow attribute when overflowX and
        // overflowY are set to the same value
        opts.overflow = [ style.overflow, style.overflowX, style.overflowY ];

        // Set display property to inline-block for height/width
        // animations on inline elements that are having width/height animated
        display = jQuery.css( elem, "display" );

        // Test default display if display is currently "none"
        checkDisplay = display === "none" ?
            jQuery._data( elem, "olddisplay" ) || defaultDisplay( elem.nodeName ) : display;

        if ( checkDisplay === "inline" && jQuery.css( elem, "float" ) === "none" ) {

            // inline-level elements accept inline-block;
            // block-level elements need to be inline with layout
            if ( !support.inlineBlockNeedsLayout || defaultDisplay( elem.nodeName ) === "inline" ) {
                style.display = "inline-block";
            } else {
                style.zoom = 1;
            }
        }
    }

    if ( opts.overflow ) {
        style.overflow = "hidden";
        if ( !support.shrinkWrapBlocks() ) {
            anim.always( function() {
                style.overflow = opts.overflow[ 0 ];
                style.overflowX = opts.overflow[ 1 ];
                style.overflowY = opts.overflow[ 2 ];
            } );
        }
    }

    // show/hide pass
    for ( prop in props ) {
        value = props[ prop ];
        if ( rfxtypes.exec( value ) ) {
            delete props[ prop ];
            toggle = toggle || value === "toggle";
            if ( value === ( hidden ? "hide" : "show" ) ) {

                // If there is dataShow left over from a stopped hide or show
                // and we are going to proceed with show, we should pretend to be hidden
                if ( value === "show" && dataShow && dataShow[ prop ] !== undefined ) {
                    hidden = true;
                } else {
                    continue;
                }
            }
            orig[ prop ] = dataShow && dataShow[ prop ] || jQuery.style( elem, prop );

        // Any non-fx value stops us from restoring the original display value
        } else {
            display = undefined;
        }
    }

    if ( !jQuery.isEmptyObject( orig ) ) {
        if ( dataShow ) {
            if ( "hidden" in dataShow ) {
                hidden = dataShow.hidden;
            }
        } else {
            dataShow = jQuery._data( elem, "fxshow", {} );
        }

        // store state if its toggle - enables .stop().toggle() to "reverse"
        if ( toggle ) {
            dataShow.hidden = !hidden;
        }
        if ( hidden ) {
            jQuery( elem ).show();
        } else {
            anim.done( function() {
                jQuery( elem ).hide();
            } );
        }
        anim.done( function() {
            var prop;
            jQuery._removeData( elem, "fxshow" );
            for ( prop in orig ) {
                jQuery.style( elem, prop, orig[ prop ] );
            }
        } );
        for ( prop in orig ) {
            tween = createTween( hidden ? dataShow[ prop ] : 0, prop, anim );

            if ( !( prop in dataShow ) ) {
                dataShow[ prop ] = tween.start;
                if ( hidden ) {
                    tween.end = tween.start;
                    tween.start = prop === "width" || prop === "height" ? 1 : 0;
                }
            }
        }

    // If this is a noop like .hide().hide(), restore an overwritten display value
    } else if ( ( display === "none" ? defaultDisplay( elem.nodeName ) : display ) === "inline" ) {
        style.display = display;
    }
}

Maybe it will help you :)

[baijunjie]

I have done a lot of research on Toggle jQuery animation,however,I don't remember the specific content since it was a long time ago.

Generally speaking,jQuery keeps the state of the element in “hide” or “show” through an object called data_privKeep.

But if the toggle animation stops in the middle, for example, the element is not hidden completely, what is the result if we use the toggle again?

Since different observers have different judgments,which may bring trouble to themselves,they will not know if it is better to “hide” or to “show”,especially when jQuery’s native toggle leads to some bugs.

Such as:

$elem.fadeToggle(5000);
window.setTimeout(function() {
    $elem.stop();
    window.setTimeout(function() {
        $elem.slideToggle(5000);
    }, 1000);
}, 2000);

As you see,it is not perfect since the problems is quite clear.
But the animate plugin can fix this problem easily.Although the effect seems to be inconsistent with the native jQuery,I still think it is much more perfect if we use the status of display as the judgment for toggle.

My suggestion is that we try our best to avoid using the toggle,and making it happen by using the specific corresponding animation function.

[KingSora]

My biggest problem with your suggestion is, I've used the toggle functionality quite a lot, because its comfortable and simple to use. Its one of the most used functions. thats why it would be better to provide some fix. Maybe not the best fix but a fix.

[baijunjie]

You try 1.6.10

[KingSora]

Nice job :)
Set the data with the string "bjj-toggle" is a good solution, as you said, jquery is doing it in kind of the same way.

Thank you!

There is just one more thing:
At the end of the toggle process the "Overflow: hidden" property is still active. I've checked the original jQuery behavior and the "Overflow: hidden" property is removed by the end of the process.

You toggle Process:
start -> toggle [ overflow: hidden ] -> end [overflow: hidden]

jQuery toggle Process
start -> toggle [overflow: hidden ] -> end

[KingSora]

Will you fix this bug?

[baijunjie]

jQuery toggle Process
start -> toggle [overflow: hidden ] -> end
I think this is a bug, the animate halfway through stop, child elements will overflow.

[KingSora]

Its not, because at the end of the toggle event you dont need the overflow property.
If you hide the element, the css property "display: none" prevent the child elements to show or to overflow. If you show the element the overflow property should be the "normal" overflow defined by the css. As you can see, the "overflow: hidden" property is just needed while the transition and not on the end or start of the transition.

[KingSora]

The main problem is just, that every element which was toggled with your plugin, has the css property "overflow : hidden" even if the element is shown and not hidden.

[baijunjie]

                var fn = callback;

        if (show === true) {
            clearStyles["overflow"] = $self[0].style.overflow;
            $self.css("overflow", "hidden").show();
        } else if (show === false) {
            clearStyles["overflow"] = $self[0].style.overflow;
            $self.css("overflow", "hidden");
            clearStyles["display"] = "none";
        }

        if (show !== undefined) {
            fn = function() {
                $self.css(clearStyles);
                if (typeof callback === "function") callback.call(this);
            };
        }

        return fn;

If you don't stop halfway animation, there would be no "overflow" residual.
But, if midway "stop ()", continue to animation, is already the second animation,The second animation will be resumed at the end to its initial state, and it existed prior to the start of "overflow", that's the problem.

[KingSora]

The fix could be:

      if (show === true) {
            clearStyles["overflow"] = $self[0].style.overflow;
            $self.css("overflow", "hidden").show();
            clearStyles["overflow"] = ""; //this is the fix
        } else if (show === false) {
            clearStyles["overflow"] = $self[0].style.overflow;
            $self.css("overflow", "hidden");
            clearStyles["display"] = "none";
        }

New Version:
https://jsfiddle.net/sgshLj9p/5/

Old Version:
https://jsfiddle.net/sgshLj9p/6/

[baijunjie]

If before the start of the animation elements set the "overflow",
When the end of the animation, "overflow" will be removed, this is a bug.

[KingSora]

Alright, this could be the fix for this issue:

if (show === true) {
    clearStyles["overflow"] = $self[0].style.overflow;
    $self.css("overflow", "hidden").show();
} else if (show === false) {
//FIX START
    if($self.data("bjj-overflow") === undefined) {
        $self.data("bjj-overflow", $self[0].style.overflow);
    }
    clearStyles["overflow"] = "";
//FIX END
    $self.css("overflow", "hidden");
    clearStyles["display"] = "none";
}

if (show !== undefined) {
    fn = function() {
//FIX START
        if(show) {
            clearStyles["overflow"] = $self.data("bjj-overflow");
            $self.removeData("bjj-overflow");
        }
//FIX END
        $self.css(clearStyles);
        if (typeof callback === "function") callback.call(this);
    };
}

Heres the new fiddle: https://jsfiddle.net/sgshLj9p/8/

[baijunjie]

Not enough comprehensive.
Must ensure that can cope with all sorts of scenarios.
You can try.
$("div").hide();
$("div").toggle(5000);

[KingSora]

Ok, I've posted some ideas, but this is your plugin and I am just trying to fix this....
Maybe you have some ideas too :)

[baijunjie]

Thank you for your support :)

[KingSora]

How is this fix:

if (show === true) {
    clearStyles["overflow"] = $self[0].style.overflow;
    //FIX START
    if($self.data("bjj-overflow") === undefined)
        $self.data("bjj-overflow", $self[0].style.overflow);
    //FIX END
    $self.css("overflow", "hidden").show();
} else if (show === false) {
    //FIX START
    if($self.data("bjj-overflow") === undefined)
        $self.data("bjj-overflow", $self[0].style.overflow);

    clearStyles["overflow"] = "";
    //FIX END
    $self.css("overflow", "hidden");
    clearStyles["display"] = "none";
}

if (show !== undefined) {
    fn = function() {
        //FIX START
        if(show) {
            clearStyles["overflow"] = $self.data("bjj-overflow");
            $self.removeData("bjj-overflow");
        }
        //FIX END
        $self.css(clearStyles);
        if (typeof callback === "function") callback.call(this);
    };
}

https://jsfiddle.net/sgshLj9p/9/

[baijunjie]

Good, did not find what problem at present, has been updated

[KingSora]

Thank you very much :)