Skip to content

Commit

Permalink
add lineClamp, vendor prefixes to CSSProperty.isUnitlessNumber
Browse files Browse the repository at this point in the history
  • Loading branch information
jbonta authored and zpao committed Feb 13, 2014
1 parent 75b58d2 commit 5abcce5
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 19 deletions.
22 changes: 22 additions & 0 deletions src/browser/dom/CSSProperty.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var isUnitlessNumber = {
flexGrow: true,
flexShrink: true,
fontWeight: true,
lineClamp: true,
lineHeight: true,
opacity: true,
order: true,
Expand All @@ -41,6 +42,27 @@ var isUnitlessNumber = {
zoom: true
};

/**
* @param {string} prefix vendor-specific prefix, eg: Webkit
* @param {string} key style name, eg: transitionDuration
* @return {string} style name prefixed with `prefix`, properly camelCased, eg:
* WebkitTransitionDuration
*/
function prefixKey(prefix, key) {
return prefix + key.charAt(0).toUpperCase() + key.substring(1);
}

/**
* Support style names that may come passed in prefixed by adding permutations
* of vendor prefixes.
*/

This comment has been minimized.

Copy link
@sophiebits

sophiebits Feb 13, 2014

Collaborator

@jbonta I think the prefix for WebKit is webkit lowercase? Try typing document.body.style. and looking at the autocomplete suggestions.

This comment has been minimized.

Copy link
@plievone

plievone Feb 13, 2014

Contributor

@spicyj I'm not so familiar with React internals, but also docs for inline styles say "Vendor prefixes should begin with a capital letter."

This comment has been minimized.

Copy link
@jbonta

jbonta Feb 13, 2014

Author Contributor

yeah, it is. but this makes for easier hyphenation, ie: replacing Captials with hyphen and a lowercase variant. We still need to fix this for ms, but this actually works in webkit. Strangely, 'webkitTransitionDuration' in document.body.style and 'WebkitTransitionDuration' in document.body.style are both true, even though WebkitTransitionDuration does not belong to Object.keys(document.body.style). That's probably why, as @plievone mentions, we have recommended starting with capital letters. This is probably not ideal, and for ms it totally breaks down: #1063

This comment has been minimized.

Copy link
@plievone

plievone Feb 13, 2014

Contributor

@jbonta So why is ms lowercase if others are capitalized?

This comment has been minimized.

Copy link
@sophiebits

sophiebits Feb 13, 2014

Collaborator

@plievone When setting styles through JS, IE requires that you write ms not Ms; WebKit seems to work with either.

This comment has been minimized.

Copy link
@plievone

plievone Feb 13, 2014

Contributor

@spicyj Ok I got carried away by hyphenation and didn't think about setting via css vs js properly.

var prefixes = ['Webkit', 'ms', 'Moz', 'O'];
for (var k in isUnitlessNumber) {
prefixes.forEach(function(prefix) {
isUnitlessNumber[prefixKey(prefix, k)] = isUnitlessNumber[k];
});
}

/**
* Most style properties can be unset by doing .style[prop] = '' but IE8
* doesn't like doing that with shorthand properties so for the properties that
Expand Down
39 changes: 39 additions & 0 deletions src/browser/dom/__tests__/CSSProperty-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* Copyright 2014 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* @jsx React.DOM
* @emails react-core
*/

/*jslint evil: true */

"use strict";

describe('CSSProperty', function() {
var CSSProperty;

beforeEach(function() {
require('mock-modules').dumpCache();
CSSProperty = require('CSSProperty');
});

it('should generate browser prefixes for its `isUnitlessNumber`', function() {
expect(CSSProperty.isUnitlessNumber.lineClamp).toBeTruthy();
expect(CSSProperty.isUnitlessNumber.WebkitLineClamp).toBeTruthy();
expect(CSSProperty.isUnitlessNumber.msFlexGrow).toBeTruthy();
expect(CSSProperty.isUnitlessNumber.MozFlexGrow).toBeTruthy();
});

});
21 changes: 2 additions & 19 deletions src/browser/dom/__tests__/CSSPropertyOperations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,25 +69,8 @@ describe('CSSPropertyOperations', function() {
});

it('should not append `px` to styles that might need a number', function() {
var unitlessProperties = [
'columnCount',
'fillOpacity',
'flex',
'flexGrow',
'flexShrink',
'fontWeight',
'lineHeight',
'opacity',
'order',
'orphans',
'pitchRange',
'richness',
'stress',
'volume',
'widows',
'zIndex',
'zoom'
];
var CSSProperty = require('CSSProperty');
var unitlessProperties = Object.keys(CSSProperty.isUnitlessNumber);
unitlessProperties.forEach(function(property) {
var styles = {};
styles[property] = 1;
Expand Down

0 comments on commit 5abcce5

Please sign in to comment.