Skip to content

Commit

Permalink
CSS: Don't automatically add "px" to properties with a few exceptions
Browse files Browse the repository at this point in the history
  • Loading branch information
mgol committed Apr 8, 2019
1 parent c4f2fa2 commit 00a9c2e
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 37 deletions.
35 changes: 5 additions & 30 deletions src/css.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ define( [
"./var/rcssNum",
"./css/var/rnumnonpx",
"./css/var/cssExpand",
"./css/isAutoPx",
"./css/var/getStyles",
"./css/var/swap",
"./css/curCSS",
Expand All @@ -16,7 +17,7 @@ define( [
"./core/init",
"./core/ready",
"./selector" // contains
], function( jQuery, access, camelCase, rcssNum, rnumnonpx, cssExpand,
], function( jQuery, access, camelCase, rcssNum, rnumnonpx, cssExpand, isAutoPx,
getStyles, swap, curCSS, adjustCSS, addGetHookIf, support, finalPropName ) {

"use strict";
Expand Down Expand Up @@ -198,30 +199,6 @@ jQuery.extend( {
}
},

// Don't automatically add "px" to these possibly-unitless properties
cssNumber: {
"animationIterationCount": true,
"columnCount": true,
"fillOpacity": true,
"flexGrow": true,
"flexShrink": true,
"fontWeight": true,
"gridArea": true,
"gridColumn": true,
"gridColumnEnd": true,
"gridColumnStart": true,
"gridRow": true,
"gridRowEnd": true,
"gridRowStart": true,
"lineHeight": true,
"opacity": true,
"order": true,
"orphans": true,
"widows": true,
"zIndex": true,
"zoom": true
},

// Add in properties whose names you wish to fix before
// setting or getting the value
cssProps: {},
Expand Down Expand Up @@ -267,11 +244,9 @@ jQuery.extend( {
return;
}

// If a number was passed in, add the unit (except for certain CSS properties)
// The isCustomProp check can be removed in jQuery 4.0 when we only auto-append
// "px" to a few hardcoded values.
if ( type === "number" && !isCustomProp ) {
value += ret && ret[ 3 ] || ( jQuery.cssNumber[ origName ] ? "" : "px" );
// If the value is a number, add `px` for certain CSS properties
if ( type === "number" ) {
value += ret && ret[ 3 ] || ( isAutoPx( origName ) ? "px" : "" );
}

// background-* props affect original clone's values
Expand Down
7 changes: 4 additions & 3 deletions src/css/adjustCSS.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
define( [
"../core",
"./isAutoPx",
"../var/rcssNum"
], function( jQuery, rcssNum ) {
], function( jQuery, isAutoPx, rcssNum ) {

"use strict";

Expand All @@ -16,11 +17,11 @@ function adjustCSS( elem, prop, valueParts, tween ) {
return jQuery.css( elem, prop, "" );
},
initial = currentValue(),
unit = valueParts && valueParts[ 3 ] || ( jQuery.cssNumber[ prop ] ? "" : "px" ),
unit = valueParts && valueParts[ 3 ] || ( isAutoPx( prop ) ? "px" : "" ),

// Starting value computation is required for potential unit mismatches
initialInUnit = elem.nodeType &&
( jQuery.cssNumber[ prop ] || unit !== "px" && +initial ) &&
( !isAutoPx( prop ) || unit !== "px" && +initial ) &&
rcssNum.exec( jQuery.css( elem, prop ) );

if ( initialInUnit && initialInUnit[ 3 ] !== unit ) {
Expand Down
41 changes: 41 additions & 0 deletions src/css/isAutoPx.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
define( function() {

"use strict";

var ralphaStart = /^[a-z]/,

// The regex visualized:
//
// /----------\
// | | /-------\
// | / Top \ | | |
// /--- Border ---+-| Right |-+---+- Width -+---\
// | | Bottom | |
// | \ Left / |
// | |
// | /----------\ |
// | /-------------\ | | |- END
// | | | | / Top \ | |
// | | / Margin \ | | | Right | | |
// |---------+-| |-+---+-| Bottom |-+----|
// | \ Padding / \ Left / |
// BEGIN -| |
// | /---------\ |
// | | | |
// | | / Min \ | / Width \ |
// \--------------+-| |-+---| |---/
// \ Max / \ Height /
rautoPx = /^(?:Border(?:Top|Right|Bottom|Left)?(?:Width|)|(?:Margin|Padding)?(?:Top|Right|Bottom|Left)?|(?:Min|Max)?(?:Width|Height))$/;

function isAutoPx( prop ) {

// The first test is used to ensure that:
// 1. The prop starts with a lowercase letter (as we uppercase it for the second regex).
// 2. The prop is not empty.
return ralphaStart.test( prop ) &&
rautoPx.test( prop[ 0 ].toUpperCase() + prop.slice( 1 ) );
};

return isAutoPx;

} );
5 changes: 3 additions & 2 deletions src/effects/Tween.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
define( [
"../core",
"../css/isAutoPx",
"../css/finalPropName",

"../css"
], function( jQuery, finalPropName ) {
], function( jQuery, isAutoPx, finalPropName ) {

"use strict";

Expand All @@ -21,7 +22,7 @@ Tween.prototype = {
this.options = options;
this.start = this.now = this.cur();
this.end = end;
this.unit = unit || ( jQuery.cssNumber[ prop ] ? "" : "px" );
this.unit = unit || ( isAutoPx( prop ) ? "px" : "" );
},
cur: function() {
var hooks = Tween.propHooks[ this.prop ];
Expand Down
82 changes: 80 additions & 2 deletions test/unit/css.js
Original file line number Diff line number Diff line change
Expand Up @@ -1198,14 +1198,17 @@ if ( jQuery.fn.offset ) {
}

QUnit.test( "Do not append px (#9548, #12990, #2792)", function( assert ) {
assert.expect( 3 );
assert.expect( 4 );

var $div = jQuery( "<div>" ).appendTo( "#qunit-fixture" );

$div.css( "fill-opacity", 1 );

assert.equal( $div.css( "fill-opacity" ), 1, "Do not append px to 'fill-opacity'" );

$div.css( "font-size", "27px" );
$div.css( "line-height", 2 );
assert.equal( $div.css( "line-height" ), "54px", "Do not append px to 'line-height'" );

$div.css( "column-count", 1 );
if ( $div.css( "column-count" ) !== undefined ) {
assert.equal( $div.css( "column-count" ), 1, "Do not append px to 'column-count'" );
Expand Down Expand Up @@ -1273,6 +1276,81 @@ QUnit[
}
} );

QUnit.test( "Do not append px to most properties not accepting integer values", function( assert ) {
assert.expect( 3 );

var $div = jQuery( "<div>" ).appendTo( "#qunit-fixture" );

$div.css( "font-size", "27px" );

$div.css( "font-size", 2 );
assert.equal( $div.css( "font-size" ), "27px", "Do not append px to 'font-size'" );

$div.css( "fontSize", 2 );
assert.equal( $div.css( "fontSize" ), "27px", "Do not append px to 'fontSize'" );

$div.css( "letter-spacing", "2px" );
$div.css( "letter-spacing", 3 );
assert.equal( $div.css( "letter-spacing" ), "2px", "Do not append px to 'letter-spacing'" );
} );

QUnit.test( "Append px to whitelisted properties", function( assert ) {
var prop,
$div = jQuery( "<div>" ).appendTo( "#qunit-fixture" ),
whitelist = {
margin: "marginTop",
marginTop: undefined,
marginRight: undefined,
marginBottom: undefined,
marginLeft: undefined,
padding: "paddingTop",
paddingTop: undefined,
paddingRight: undefined,
paddingBottom: undefined,
paddingLeft: undefined,
top: undefined,
right: undefined,
bottom: undefined,
left: undefined,
width: undefined,
height: undefined,
minWidth: undefined,
minHeight: undefined,
maxWidth: undefined,
maxHeight: undefined,
border: "borderTopWidth",
borderWidth: "borderTopWidth",
borderTop: "borderTopWidth",
borderTopWidth: undefined,
borderRight: "borderRightWidth",
borderRightWidth: undefined,
borderBottom: "borderBottomWidth",
borderBottomWidth: undefined,
borderLeft: "borderLeftWidth",
borderLeftWidth: undefined
};

assert.expect( ( Object.keys( whitelist ).length ) * 2 );

for ( prop in whitelist ) {
var propToCheck = whitelist[ prop ] || prop,
kebabProp = prop.replace( /[A-Z]/g, function( match ) {
return "-" + match.toLowerCase();
} ),
kebabPropToCheck = propToCheck.replace( /[A-Z]/g, function( match ) {
return "-" + match.toLowerCase();
} );
$div.css( prop, 3 )
.css( "position", "absolute" )
.css( "border-style", "solid" );
assert.equal( $div.css( propToCheck ), "3px", "Append px to '" + prop + "'" );
$div.css( kebabProp, 3 )
.css( "position", "absolute" )
.css( "border-style", "solid" );
assert.equal( $div.css( kebabPropToCheck ), "3px", "Append px to '" + kebabProp + "'" );
}
} );

QUnit.test( "css('width') and css('height') should respect box-sizing, see #11004", function( assert ) {
assert.expect( 4 );

Expand Down

0 comments on commit 00a9c2e

Please sign in to comment.