Effects: Handle the .hide/show/toggle( options ) signatures from core properly. Fixes...

diff --git a/tests/unit/effects/effects.html b/tests/unit/effects/effects.html
index 7ddacf7bb665a79da1a254b351f3a8eeb25054ff..4538ecb0300f847af4d95370ff6e7d663b756cc8 100644 (file)
--- a/tests/unit/effects/effects.html
+++ b/tests/unit/effects/effects.html
@@ -96,6 +96,8 @@
 <h2 id="qunit-userAgent"></h2>
 <ol id="qunit-tests"></ol>
 <div id="qunit-fixture">
+<div id="elem" class="test">
+</div>
 <div class="hidden test">
        <div>.</div>
 </div>

diff --git a/tests/unit/effects/effects_core.js b/tests/unit/effects/effects_core.js
index 928d4c15bb7574be18962dbf2ffe2664309eaab6..11e9d0b456ec84827be53e5aafd8f5ce497e52fd 100644 (file)
--- a/tests/unit/effects/effects_core.js
+++ b/tests/unit/effects/effects_core.js
@@ -16,6 +16,24 @@ var minDuration = 15,
 
 module( "effects.core" );
 
+// TODO: test all signatures of .show(), .hide(), .toggle().
+// Look at core's signatures and UI's signatures.
+asyncTest( ".hide() with step", function() {
+       expect( 1 );
+       var element = $( "#elem" ),
+               step = function() {
+                       ok( true, "step callback invoked" );
+                       step = $.noop;
+               };
+
+       element.hide({
+               step: function() {
+                       step();
+               },
+               complete: start
+       });
+});
+
 test( "Immediate Return Conditions", function() {
        var hidden = $( "div.hidden" ),
                count = 0;

diff --git a/ui/jquery.ui.effect.js b/ui/jquery.ui.effect.js
index 41aeb4819ab983a5c1254857b8de5ccc0bcfabc0..f3d9929b029e0f6ccd47887412ea52bca9407b94 100644 (file)
--- a/ui/jquery.ui.effect.js
+++ b/ui/jquery.ui.effect.js
@@ -1106,14 +1106,29 @@ function _normalizeArguments( effect, options, speed, callback ) {
        return effect;
 }
 
-function standardSpeed( speed ) {
-       // valid standard speeds
-       if ( !speed || typeof speed === "number" || $.fx.speeds[ speed ] ) {
+function standardAnimationOption( option ) {
+       // Valid standard speeds (nothing, number, named speed)
+       if ( !option || typeof option === "number" || $.fx.speeds[ option ] ) {
                return true;
        }
 
-       // invalid strings - treat as "normal" speed
-       return typeof speed === "string" && !$.effects.effect[ speed ];
+       // Invalid strings - treat as "normal" speed
+       if ( typeof option === "string" && !$.effects.effect[ option ] ) {
+               return true;
+       }
+
+       // Complete callback
+       if ( $.isFunction( option ) ) {
+               return true;
+       }
+
+       // Options hash (but not naming an effect)
+       if ( typeof option === "object" && !option.effect ) {
+               return true;
+       }
+
+       // Didn't match any standard API
+       return false;
 }
 
 $.fn.extend({
@@ -1163,39 +1178,41 @@ $.fn.extend({
                return queue === false ? this.each( run ) : this.queue( queue || "fx", run );
        },
 
-       _show: $.fn.show,
-       show: function( speed ) {
-               if ( standardSpeed( speed ) ) {
-                       return this._show.apply( this, arguments );
-               } else {
-                       var args = _normalizeArguments.apply( this, arguments );
-                       args.mode = "show";
-                       return this.effect.call( this, args );
-               }
-       },
+       show: (function( orig ) {
+               return function( option ) {
+                       if ( standardAnimationOption( option ) ) {
+                               return orig.apply( this, arguments );
+                       } else {
+                               var args = _normalizeArguments.apply( this, arguments );
+                               args.mode = "show";
+                               return this.effect.call( this, args );
+                       }
+               };
+       })( $.fn.show ),
 
-       _hide: $.fn.hide,
-       hide: function( speed ) {
-               if ( standardSpeed( speed ) ) {
-                       return this._hide.apply( this, arguments );
-               } else {
-                       var args = _normalizeArguments.apply( this, arguments );
-                       args.mode = "hide";
-                       return this.effect.call( this, args );
-               }
-       },
+       hide: (function( orig ) {
+               return function( option ) {
+                       if ( standardAnimationOption( option ) ) {
+                               return orig.apply( this, arguments );
+                       } else {
+                               var args = _normalizeArguments.apply( this, arguments );
+                               args.mode = "hide";
+                               return this.effect.call( this, args );
+                       }
+               };
+       })( $.fn.hide ),
 
-       // jQuery core overloads toggle and creates _toggle
-       __toggle: $.fn.toggle,
-       toggle: function( speed ) {
-               if ( standardSpeed( speed ) || typeof speed === "boolean" || $.isFunction( speed ) ) {
-                       return this.__toggle.apply( this, arguments );
-               } else {
-                       var args = _normalizeArguments.apply( this, arguments );
-                       args.mode = "toggle";
-                       return this.effect.call( this, args );
-               }
-       },
+       toggle: (function( orig ) {
+               return function( option ) {
+                       if ( standardAnimationOption( option ) || typeof option === "boolean" ) {
+                               return orig.apply( this, arguments );
+                       } else {
+                               var args = _normalizeArguments.apply( this, arguments );
+                               args.mode = "toggle";
+                               return this.effect.call( this, args );
+                       }
+               };
+       })( $.fn.toggle ),
 
        // helper functions
        cssUnit: function(key) {