Skip to content

Commit

Permalink
Reimplement color processing
Browse files Browse the repository at this point in the history
Summary:
**Problem:**

As I was trying to document what color formats we supported, I realized that our current implementation based on the open source project tinycolor supported some crazy things. A few examples that were all valid:

```
tinycolor('abc')
tinycolor(' #abc ')
tinycolor('##abc')
tinycolor('rgb 255 0 0')
tinycolor('RGBA(0, 1, 2)')
tinycolor('rgb (0, 1, 2)')
tinycolor('hsv(0, 1, 2)')
tinycolor({r: 10, g: 10, b: 10})
tinycolor('hsl(1%, 2, 3)')
tinycolor('rgb(1.0, 2.0, 3.0)')
tinycolor('rgb(1%, 2%, 3%)')
```

The integrations of tinycolor were also really bad. processColor added "support" for pure numbers and an array of colors!?? ColorPropTypes did some crazy trim().toString() and repeated a bad error message twice.

**Solution:**

While iteratively cleaning the file, I eventually ended up reimplementing it entierly. Major changes are:
- The API is now dead simple: returns null if it doesn't parse or returns the int32 representation of the color
- Stricter parsing of at
Closes facebook#5529

Reviewed By: svcscm

Differential Revision: D2872015

Pulled By: nicklockwood

fb-gh-sync-id: df78244eefce6cf8e8ed2ea51f58d6b232de16f9
  • Loading branch information
vjeux authored and facebook-github-bot-8 committed Jan 29, 2016
1 parent 715081c commit c8a0a3e
Show file tree
Hide file tree
Showing 9 changed files with 529 additions and 580 deletions.
23 changes: 14 additions & 9 deletions Libraries/Animated/src/Interpolation.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
* @providesModule Interpolation
* @flow
*/
/* eslint no-bitwise: 0 */
'use strict';

var tinycolor = require('tinycolor');
var normalizeColor = require('normalizeColor');

// TODO(#7644673): fix this hack once github jest actually checks invariants
var invariant = function(condition, message) {
Expand Down Expand Up @@ -164,16 +165,20 @@ function interpolate(
return result;
}

function colorToRgba(
input: string
): string {
var color = tinycolor(input);
if (color.isValid()) {
var {r, g, b, a} = color.toRgb();
return `rgba(${r}, ${g}, ${b}, ${a === undefined ? 1 : a})`;
} else {
function colorToRgba(input: string): string {
var int32Color = normalizeColor(input);
if (int32Color === null) {
return input;
}

int32Color = int32Color || 0; // $FlowIssue

var a = ((int32Color & 0xff000000) >>> 24) / 255;
var r = (int32Color & 0x00ff0000) >>> 16;
var g = (int32Color & 0x0000ff00) >>> 8;
var b = int32Color & 0x000000ff;

return `rgba(${r}, ${g}, ${b}, ${a})`;
}

var stringShapeRegex = /[0-9\.-]+/g;
Expand Down
14 changes: 7 additions & 7 deletions Libraries/Animated/src/__tests__/Interpolation-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
jest
.dontMock('Interpolation')
.dontMock('Easing')
.dontMock('tinycolor');
.dontMock('normalizeColor');

var Interpolation = require('Interpolation');
var Easing = require('Easing');
Expand Down Expand Up @@ -216,12 +216,12 @@ describe('Interpolation', () => {
it('should work with output ranges as string', () => {
var interpolation = Interpolation.create({
inputRange: [0, 1],
outputRange: ['rgba(0, 100, 200, 0)', 'rgba(50, 150, 250, 0.5)'],
outputRange: ['rgba(0, 100, 200, 0)', 'rgba(50, 150, 250, 0.4)'],
});

expect(interpolation(0)).toBe('rgba(0, 100, 200, 0)');
expect(interpolation(0.5)).toBe('rgba(25, 125, 225, 0.25)');
expect(interpolation(1)).toBe('rgba(50, 150, 250, 0.5)');
expect(interpolation(0.5)).toBe('rgba(25, 125, 225, 0.2)');
expect(interpolation(1)).toBe('rgba(50, 150, 250, 0.4)');
});

it('should work with output ranges as short hex string', () => {
Expand Down Expand Up @@ -249,11 +249,11 @@ describe('Interpolation', () => {
it('should work with output ranges with mixed hex and rgba strings', () => {
var interpolation = Interpolation.create({
inputRange: [0, 1],
outputRange: ['rgba(100, 120, 140, .5)', '#87FC70'],
outputRange: ['rgba(100, 120, 140, .4)', '#87FC70'],
});

expect(interpolation(0)).toBe('rgba(100, 120, 140, 0.5)');
expect(interpolation(0.5)).toBe('rgba(117.5, 186, 126, 0.75)');
expect(interpolation(0)).toBe('rgba(100, 120, 140, 0.4)');
expect(interpolation(0.5)).toBe('rgba(117.5, 186, 126, 0.7)');
expect(interpolation(1)).toBe('rgba(135, 252, 112, 1)');
});

Expand Down
8 changes: 6 additions & 2 deletions Libraries/ReactIOS/requireNativeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,16 @@ var TypeToDifferMap = {
// (not yet implemented)
};

function processColorArray(colors: []): [] {
return colors && colors.map(processColor);
}

var TypeToProcessorMap = {
// iOS Types
CGColor: processColor,
CGColorArray: processColor,
CGColorArray: processColorArray,
UIColor: processColor,
UIColorArray: processColor,
UIColorArray: processColorArray,
CGImage: resolveAssetSource,
UIImage: resolveAssetSource,
RCTImageSource: resolveAssetSource,
Expand Down
40 changes: 26 additions & 14 deletions Libraries/StyleSheet/ColorPropType.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,38 @@
* @providesModule ColorPropType
*/
'use strict';

var ReactPropTypes = require('ReactPropTypes');
var tinycolor = require('tinycolor');

var colorValidator = function (props, propName) {
var selectedColor = props[propName];
if (selectedColor === null || selectedColor === undefined || selectedColor.toString().trim() === '') {
return new Error(
`Invalid argument supplied to ${propName}.Expected a string like #123ADF or 'red'.`
);
var normalizeColor = require('normalizeColor');

var ColorPropType = function(props, propName) {
var color = props[propName];
if (color === undefined || color === null) {
return;
}

if (tinycolor(selectedColor.toString().trim()).isValid()) {
return null;
if (typeof color === 'number') {
// Developers should not use a number, but we are using the prop type
// both for user provided colors and for transformed ones. This isn't ideal
// and should be fixed but will do for now...
return;
}

return new Error(
`Invalid argument supplied to ${propName}.Expected a string like #123ADF or 'red'.`
);
if (normalizeColor(color) === null) {
return new Error(
`Invalid color supplied to ${propName}: ${color}. Valid color formats are
- #f0f (#rgb)
- #f0fc (#rgba)
- #ff00ff (#rrggbb)
- #ff00ff00 (#rrggbbaa)
- rgb(255, 255, 255)
- rgba(255, 255, 255, 1.0)
- hsl(360, 100%, 100%)
- hsla(360, 100%, 100%, 1.0)
- transparent
- red`);
}
};

var ColorPropType = ReactPropTypes.oneOfType([colorValidator, ReactPropTypes.number]);

module.exports = ColorPropType;
112 changes: 112 additions & 0 deletions Libraries/StyleSheet/__tests__/normalizeColor-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/
'use strict';

jest.dontMock('normalizeColor');

var normalizeColor = require('normalizeColor');

describe('normalizeColor', function() {
it('should accept only spec compliant colors', function() {
expect(normalizeColor('#abc')).not.toBe(null);
expect(normalizeColor('#abcd')).not.toBe(null);
expect(normalizeColor('#abcdef')).not.toBe(null);
expect(normalizeColor('#abcdef01')).not.toBe(null);
expect(normalizeColor('rgb(1,2,3)')).not.toBe(null);
expect(normalizeColor('rgb(1, 2, 3)')).not.toBe(null);
expect(normalizeColor('rgb( 1 , 2 , 3 )')).not.toBe(null);
expect(normalizeColor('rgb(-1, -2, -3)')).not.toBe(null);
expect(normalizeColor('rgba(0, 0, 0, 1)')).not.toBe(null);
});

it('should refuse non spec compliant colors', function() {
expect(normalizeColor('#00gg00')).toBe(null);
expect(normalizeColor('rgb(1, 2, 3,)')).toBe(null);
expect(normalizeColor('rgb(1, 2, 3')).toBe(null);

// Used to be accepted by normalizeColor
expect(normalizeColor('abc')).toBe(null);
expect(normalizeColor(' #abc ')).toBe(null);
expect(normalizeColor('##abc')).toBe(null);
expect(normalizeColor('rgb 255 0 0')).toBe(null);
expect(normalizeColor('RGBA(0, 1, 2)')).toBe(null);
expect(normalizeColor('rgb (0, 1, 2)')).toBe(null);
expect(normalizeColor('hsv(0, 1, 2)')).toBe(null);
expect(normalizeColor({r: 10, g: 10, b: 10})).toBe(null);
expect(normalizeColor('hsl(1%, 2, 3)')).toBe(null);
expect(normalizeColor('rgb(1.0, 2.0, 3.0)')).toBe(null);
expect(normalizeColor('rgb(1%, 2%, 3%)')).toBe(null);
});

it('should handle hex6 properly', function() {
expect(normalizeColor('#000000')).toBe(0xff000000);
expect(normalizeColor('#ffffff')).toBe(0xffffffff);
expect(normalizeColor('#ff00ff')).toBe(0xffff00ff);
expect(normalizeColor('#abcdef')).toBe(0xffabcdef);
expect(normalizeColor('#012345')).toBe(0xff012345);
});

it('should handle hex3 properly', function() {
expect(normalizeColor('#000')).toBe(0xff000000);
expect(normalizeColor('#fff')).toBe(0xffffffff);
expect(normalizeColor('#f0f')).toBe(0xffff00ff);
});

it('should handle hex8 properly', function() {
expect(normalizeColor('#00000000')).toBe(0x00000000);
expect(normalizeColor('#ffffffff')).toBe(0xffffffff);
expect(normalizeColor('#ffff00ff')).toBe(0xffffff00);
expect(normalizeColor('#abcdef01')).toBe(0x01abcdef);
expect(normalizeColor('#01234567')).toBe(0x67012345);
});

it('should handle rgb properly', function() {
expect(normalizeColor('rgb(0, 0, 0)')).toBe(0xff000000);
expect(normalizeColor('rgb(-1, -2, -3)')).toBe(0xff000000);
expect(normalizeColor('rgb(0, 0, 255)')).toBe(0xff0000ff);
expect(normalizeColor('rgb(100, 15, 69)')).toBe(0xff640f45);
expect(normalizeColor('rgb(255, 255, 255)')).toBe(0xffffffff);
expect(normalizeColor('rgb(256, 256, 256)')).toBe(0xffffffff);
});

it('should handle rgba properly', function() {
expect(normalizeColor('rgba(0, 0, 0, 0.0)')).toBe(0x00000000);
expect(normalizeColor('rgba(0, 0, 0, 0)')).toBe(0x00000000);
expect(normalizeColor('rgba(0, 0, 0, -0.5)')).toBe(0x00000000);
expect(normalizeColor('rgba(0, 0, 0, 1.0)')).toBe(0xff000000);
expect(normalizeColor('rgba(0, 0, 0, 1)')).toBe(0xff000000);
expect(normalizeColor('rgba(0, 0, 0, 1.5)')).toBe(0xff000000);
expect(normalizeColor('rgba(100, 15, 69, 0.5)')).toBe(0x80640f45);
});

it('should handle hsl properly', function() {
expect(normalizeColor('hsl(0, 0%, 0%)')).toBe(0xff000000);
expect(normalizeColor('hsl(360, 100%, 100%)')).toBe(0xffffffff);
expect(normalizeColor('hsl(180, 50%, 50%)')).toBe(0xff40bfbf);
expect(normalizeColor('hsl(540, 50%, 50%)')).toBe(0xff40bfbf);
expect(normalizeColor('hsl(70, 25%, 75%)')).toBe(0xffcacfaf);
expect(normalizeColor('hsl(70, 100%, 75%)')).toBe(0xffeaff80);
expect(normalizeColor('hsl(70, 110%, 75%)')).toBe(0xffeaff80);
expect(normalizeColor('hsl(70, 0%, 75%)')).toBe(0xffbfbfbf);
expect(normalizeColor('hsl(70, -10%, 75%)')).toBe(0xffbfbfbf);
});

it('should handle hsla properly', function() {
expect(normalizeColor('hsla(0, 0%, 0%, 0)')).toBe(0x00000000);
expect(normalizeColor('hsla(360, 100%, 100%, 1)')).toBe(0xffffffff);
expect(normalizeColor('hsla(360, 100%, 100%, 0)')).toBe(0x00ffffff);
expect(normalizeColor('hsla(180, 50%, 50%, 0.2)')).toBe(0x3340bfbf);
});

it('should handle named colors properly', function() {
expect(normalizeColor('red')).toBe(0xffff0000);
expect(normalizeColor('transparent')).toBe(0x00000000);
expect(normalizeColor('peachpuff')).toBe(0xffffdab9);
});
});
24 changes: 0 additions & 24 deletions Libraries/StyleSheet/__tests__/processColor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,6 @@ describe('processColor', () => {
expect(colorFromString).toEqual(expectedInt);
});

it('should convert rgb x, y, z', () => {
var colorFromString = processColor('rgb 10, 20, 30');
var expectedInt = 0xFF0A141E;
expect(colorFromString).toEqual(expectedInt);
});

});

describe('RGBA strings', () => {
Expand All @@ -65,12 +59,6 @@ describe('processColor', () => {
expect(colorFromString).toEqual(expectedInt);
});

it('should convert rgba x, y, z, a', () => {
var colorFromString = processColor('rgba 10, 20, 30, 0.4');
var expectedInt = 0x660A141E;
expect(colorFromString).toEqual(expectedInt);
});

});

describe('HSL strings', () => {
Expand All @@ -81,12 +69,6 @@ describe('processColor', () => {
expect(colorFromString).toEqual(expectedInt);
});

it('should convert hsl x, y%, z%', () => {
var colorFromString = processColor('hsl 318, 69%, 55%');
var expectedInt = 0xFFDB3DAC;
expect(colorFromString).toEqual(expectedInt);
});

});

describe('HSL strings', () => {
Expand All @@ -97,12 +79,6 @@ describe('processColor', () => {
expect(colorFromString).toEqual(expectedInt);
});

it('should convert hsla x, y%, z%, a', () => {
var colorFromString = processColor('hsla 318, 69%, 55%, 0.25');
var expectedInt = 0x40DB3DAC;
expect(colorFromString).toEqual(expectedInt);
});

});

describe('hex strings', () => {
Expand Down
Loading

0 comments on commit c8a0a3e

Please sign in to comment.