Split apart jQuery.css into jQuery.css (computed values) and jQuery.s…
…tyle (currently set values).
- Loading branch information
- +56 −28 src/css.js
- +2 −2 src/dimensions.js
- +11 −11 src/effects.js
- +9 −9 src/offset.js
- +2 −10 test/unit/css.js
- +5 −5 test/unit/effects.js
| @@ -29,7 +29,7 @@ jQuery.fn.extend({ | ||
|
|
||
| this[i].style.display = old || ""; | ||
|
|
||
| if ( jQuery.css(this[i], "display") === "none" ) { | ||
| if ( jQuery.css( this[i], "display" ) === "none" ) { | ||
| var nodeName = this[i].nodeName, display; | ||
|
|
||
| if ( elemdisplay[ nodeName ] ) { | ||
| @@ -71,7 +71,7 @@ jQuery.fn.extend({ | ||
| for ( var i = 0, l = this.length; i < l; i++ ) { | ||
| var old = jQuery.data(this[i], "olddisplay"); | ||
| if ( !old && old !== "none" ) { | ||
| jQuery.data( this[i], "olddisplay", jQuery.css(this[i], "display") ); | ||
| jQuery.data( this[i], "olddisplay", jQuery.css( this[i], "display" ) ); | ||
| } | ||
| } | ||
|
|
||
| @@ -139,7 +139,7 @@ jQuery.fn.extend({ | ||
|
|
||
| if ( ( p === "height" || p === "width" ) && this.style ) { | ||
| // Store display property | ||
| opt.display = jQuery.css(this, "display"); | ||
| opt.display = this.style.display; | ||
|
|
||
| // Make sure that nothing sneaks out | ||
| opt.overflow = this.style.overflow; | ||
| @@ -316,13 +316,13 @@ jQuery.fx.prototype = { | ||
| }, | ||
|
|
||
| // Get the current size | ||
| cur: function( force ) { | ||
| cur: function() { | ||
| if ( this.elem[this.prop] != null && (!this.elem.style || this.elem.style[this.prop] == null) ) { | ||
| return this.elem[ this.prop ]; | ||
| } | ||
|
|
||
| var r = parseFloat(jQuery.css(this.elem, this.prop, undefined, force)); | ||
| return r && r > -10000 ? r : parseFloat(jQuery.css(this.elem, this.prop)) || 0; | ||
This comment has been minimized.
This comment has been minimized.
DanielBaulig
|
||
| var r = jQuery.css( this.elem, this.prop ); | ||
| return r && r > -10000 ? r : 0; | ||
| }, | ||
|
|
||
| // Start an animation from one number to another | ||
| @@ -349,7 +349,7 @@ jQuery.fx.prototype = { | ||
| // Simple 'show' function | ||
| show: function() { | ||
| // Remember where we started, so that we can go back to it later | ||
| this.options.orig[this.prop] = jQuery.css( this.elem, this.prop, undefined, false ); | ||
| this.options.orig[this.prop] = jQuery.style( this.elem, this.prop ); | ||
| this.options.show = true; | ||
|
|
||
| // Begin the animation | ||
| @@ -364,7 +364,7 @@ jQuery.fx.prototype = { | ||
| // Simple 'hide' function | ||
| hide: function() { | ||
| // Remember where we started, so that we can go back to it later | ||
| this.options.orig[this.prop] = jQuery.css( this.elem, this.prop, undefined, false ); | ||
| this.options.orig[this.prop] = jQuery.style( this.elem, this.prop ); | ||
| this.options.hide = true; | ||
|
|
||
| // Begin the animation | ||
| @@ -397,7 +397,7 @@ jQuery.fx.prototype = { | ||
| var old = jQuery.data(this.elem, "olddisplay"); | ||
| this.elem.style.display = old ? old : this.options.display; | ||
|
|
||
| if ( jQuery.css(this.elem, "display") === "none" ) { | ||
| if ( jQuery.css( this.elem, "display" ) === "none" ) { | ||
| this.elem.style.display = "block"; | ||
| } | ||
| } | ||
| @@ -410,7 +410,7 @@ jQuery.fx.prototype = { | ||
| // Reset the properties, if the item has been hidden or shown | ||
| if ( this.options.hide || this.options.show ) { | ||
| for ( var p in this.options.curAnim ) { | ||
| jQuery.css( this.elem, p, this.options.orig[p] ); | ||
| jQuery.style( this.elem, p, this.options.orig[p] ); | ||
| } | ||
| } | ||
|
|
||
| @@ -467,7 +467,7 @@ jQuery.extend( jQuery.fx, { | ||
|
|
||
| step: { | ||
| opacity: function( fx ) { | ||
| jQuery.css( fx.elem, "opacity", fx.now ); | ||
| jQuery.style( fx.elem, "opacity", fx.now ); | ||
| }, | ||
|
|
||
| _default: function( fx ) { | ||
12 comments
on commit 37b607d
This comment has been minimized.
This comment has been minimized.
kangax
replied
Sep 16, 2010
|
Is it just me or do "css" and "style" names sound rather non-descriptive? Is there anything about "css" name suggesting that method should return computed style values? Why not have "style" and "computedStyle" (or "compStyle")? |
This comment has been minimized.
This comment has been minimized.
|
Because that's the API that we have - and have had for years? The methods aren't typically user-facing but they are used by plugin authors so they're likely going to stay as-is for quite some time. |
This comment has been minimized.
This comment has been minimized.
kangax
replied
Sep 16, 2010
|
I see. I was just surprised by how unclear those 2 method names are, that you had to specify how they differ in parens. I thought — why not just make names more descriptive? Have you considered aliasing methods like these to more explanatory names? And then discourage their usage in favor of new ones? Or are method names like these not considered a problem? |
This comment has been minimized.
This comment has been minimized.
|
Not really considered a problem - they're almost entirely used internally. As far as the naming goes it seems to work fairly well - our main .css() methods (used by the end user and provides the computed styling of an element) maps directly to our internal jQuery.css(). jQuery.style() is another method that we use internally to manage the setting and getting of the style property. |
This comment has been minimized.
This comment has been minimized.
GarrettS
replied
Sep 16, 2010
|
Aliasing seems like a good start. But if the new method wants to have different behavior (bug fix), then aliasing wouldn't work. With method delegation, the new [and] old methods share scope with a new private method, introduced to perform common tasks. {
...
css: deprecatedCSS,
computedStyle: computedStyle,
...
}
function deprecatedCSS(a, b, c) {
/* legacy-related code here */
var style = _readComputedtStyle(a, b, c);
/* more legacy-related code */
return style;
}
function _readComputedStyle(a, b, c) {
var style;
/*...*/
return style;
}
function computedStyle(a, b, c) {
return _readComputedStyle(a, b, c);
}
|
This comment has been minimized.
This comment has been minimized.
CaViCcHi
replied
Oct 18, 2010
|
Hi, I would suggest that the row 325 effects.js return r && r > -10000 ? r : 0; would be changed back to return r; It works correctly in opera, otherwise anything moved with animate with a position > 10000 will get back to 0 and animate from there. thank you |
This comment has been minimized.
This comment has been minimized.
Flcn
replied
Nov 17, 2010
|
return r && r > -10000 ? r : 0; |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
CaViCcHi
replied
Nov 17, 2010
|
@Flcn an easy way around it according to your needs is to add this to your code. So you can keep on going :) /*
* Overwrites the Method created, because of a jQuery bug
* http://github.com/jquery/jquery/commit/37b607d2815b893d13de4ac3461090d0dd46535e#commitcomment-170919
*/
jQuery.fx.prototype.cur = function() {
if ( this.elem[this.prop] != null && (!this.elem.style || this.elem.style[this.prop] == null) ) {
return this.elem[ this.prop ];
}
var r = parseFloat( jQuery.css( this.elem, this.prop ) );
return r /*&& r > -10000 ? r : 0*/;
};
|
This comment has been minimized.
This comment has been minimized.
igorw
replied
Nov 17, 2010
|
I came across this myself just today. Hadn't seen this discussion yet, however. #101 |
This comment has been minimized.
This comment has been minimized.
CaViCcHi
replied
Nov 29, 2010
|
use my workaround :) |
This comment has been minimized.
This comment has been minimized.
DanielBaulig
replied
Nov 29, 2010
|
Actually I propose to rather use |
whe you remove this? parseFloat(jQuery.css(this.elem, this.prop)) ?
it breaks some applications where values can be less then -10000.
and i wonder, why exactly -10000 ? not -100000 for example