Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash when invalid args for library polished's functions #6939

Merged
merged 14 commits into from
Jun 13, 2019
23 changes: 11 additions & 12 deletions lib/components/src/tooltip/Tooltip.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import React, { FunctionComponent } from 'react';
import memoize from 'memoizerific';

import { styled, Color } from '@storybook/theming';
import { rgba, lighten, darken } from 'polished';
import { styled, Color, lighten, darken } from '@storybook/theming';

const match = memoize(1000)((requestes, actual, value, fallback = 0) =>
actual.split('-')[0] === requestes ? value : fallback
Expand Down Expand Up @@ -40,32 +39,32 @@ const Arrow = styled.div<ArrowProps>(
'top',
placement,
theme.color[color] || color || theme.base === 'light'
? rgba(`${lighten(1, theme.background.app)}`, 0.95)
: rgba(`${darken(1, theme.background.app)}`, 0.95),
? lighten(theme.background.app)
: darken(theme.background.app),
'transparent'
),
borderBottomColor: match(
'bottom',
placement,
theme.color[color] || color || theme.base === 'light'
? rgba(`${lighten(1, theme.background.app)}`, 0.95)
: rgba(`${darken(1, theme.background.app)}`, 0.95),
? lighten(theme.background.app)
: darken(theme.background.app),
'transparent'
),
borderLeftColor: match(
'left',
placement,
theme.color[color] || color || theme.base === 'light'
? rgba(`${lighten(1, theme.background.app)}`, 0.95)
: rgba(`${darken(1, theme.background.app)}`, 0.95),
? lighten(theme.background.app)
: darken(theme.background.app),
'transparent'
),
borderRightColor: match(
'right',
placement,
theme.color[color] || color || theme.base === 'light'
? rgba(`${lighten(1, theme.background.app)}`, 0.95)
: rgba(`${darken(1, theme.background.app)}`, 0.95),
? lighten(theme.background.app)
: darken(theme.background.app),
'transparent'
),
})
Expand Down Expand Up @@ -93,8 +92,8 @@ const Wrapper = styled.div<WrapperProps>(

background:
theme.color[color] || color || theme.base === 'light'
? rgba(`${lighten(1, theme.background.app)}`, 0.95)
: rgba(`${darken(1, theme.background.app)}`, 0.95),
? lighten(theme.background.app)
: darken(theme.background.app),
filter: `
drop-shadow(0px 5px 5px rgba(0,0,0,0.05))
drop-shadow(0 1px 3px rgba(0,0,0,0.1))
Expand Down
2 changes: 2 additions & 0 deletions lib/theming/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,5 @@ export { createGlobal, createReset } from './global';
export * from './create';
export * from './convert';
export * from './ensure';

export { lightenColor as lighten, darkenColor as darken } from './utils';
77 changes: 77 additions & 0 deletions lib/theming/src/tests/util.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { lightenColor as lighten, darkenColor as darken } from '../utils';

describe('utils', () => {
it('should apply polished when valid arguments are passed', () => {
const lightColor = '#F6F9FC';
const darkColor = '#2f2f2f';
const darkenedColor = darken(lightColor);
const lightenedColor = lighten(darkColor);

expect(darkenedColor).toEqual('rgba(0,0,0,0.95)');
expect(lightenedColor).toEqual('rgba(255,255,255,0.95)');
});

it('should guard non-string value is being passed to color of theme object', () => {
const result = () => {
return lighten(1234);
};

expect(result).toThrow();
});

it('should guard anything that is not working with polished', () => {
const color = '1234';

const result = lighten(color);

expect(result).toEqual(color);
});

it('should guard css variables is being passed to polished functions', () => {
const color = 'var(--my-var, blue)';

const result = lighten(color);

expect(result).toEqual(color);
});

it('should guard css calc is being passed to polished functions', () => {
const color = 'rgb(calc(0 + 100), calc(0 + 100), calc(0 + 100))';

const result = lighten(color);

expect(result).toEqual(color);
});

it('should guard linear-gradient is being passed to polished functions', () => {
const color = `linear-gradient(to bottom, white, blue)`;

const result = lighten(color);

expect(result).toEqual(color);
});

it('should guard radial-gradient is being passed to polished functions', () => {
const color = `radial-gradient(red, green, blue)`;

const result = lighten(color);

expect(result).toEqual(color);
});

it('should guard repeating-linear-gradient is being passed to polished functions', () => {
const color = `repeating-linear-gradient(red, yellow 10%, green 20%)`;

const result = lighten(color);

expect(result).toEqual(color);
});

it('should guard repeating-radial-gradient is being passed to polished functions', () => {
const color = `repeating-radial-gradient(red, yellow 10%, green 20%)`;

const result = lighten(color);

expect(result).toEqual(color);
});
});
58 changes: 58 additions & 0 deletions lib/theming/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1 +1,59 @@
import { rgba, lighten, darken } from 'polished';

import { logger } from '@storybook/client-logger';

export const mkColor = (color: string) => ({ color });

// Check if it is a string. This is for the sake of warning users
// and the successive guarding logics that use String methods.
const isColorString = (color: string) => {
if (typeof color !== 'string') {
logger.warn(
`Color passed to theme object should be a string. Instead ` +
`${color}(${typeof color}) was passed.`
);
return false;
}

return true;
};

// Passing arguments that can't be converted to RGB such as linear-gradient
// to library polished's functions such as lighten or darken throws the error
// that crashes the entire storybook. It needs to be guarded when arguments
// of those functions are from user input.
const isValidColorForPolished = (color: string) => {
return !/(gradient|var|calc)/.test(color);
};

const applyPolished = (type: string, color: string) => {
if (type === 'darken') {
return rgba(`${darken(1, color)}`, 0.95);
}

if (type === 'lighten') {
return rgba(`${lighten(1, color)}`, 0.95);
}

return color;
};

const colorFactory = (type: string) => (color: string) => {
if (!isColorString(color)) {
return color;
}

if (!isValidColorForPolished(color)) {
return color;
}

// Guard anything that is not working with polished.
try {
return applyPolished(type, color);
} catch (error) {
return color;
lonyele marked this conversation as resolved.
Show resolved Hide resolved
}
};

export const lightenColor = colorFactory('lighten');
export const darkenColor = colorFactory('darken');
7 changes: 2 additions & 5 deletions lib/ui/src/components/notifications/item.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import { styled } from '@storybook/theming';
import { styled, lighten, darken } from '@storybook/theming';
import { Link } from '@storybook/router';
import { rgba, lighten, darken } from 'polished';

const baseStyle = ({ theme }) => ({
display: 'block',
Expand All @@ -14,9 +13,7 @@ const baseStyle = ({ theme }) => ({
boxShadow: '0 5px 15px 0 rgba(0, 0, 0, 0.1), 0 2px 5px 0 rgba(0, 0, 0, 0.05)',
color: theme.color.inverseText,
backgroundColor:
theme.base === 'light'
? rgba(`${darken(1, theme.background.app)}`, 0.95)
: rgba(`${lighten(1, theme.background.app)}`, 0.95),
theme.base === 'light' ? darken(theme.background.app) : lighten(theme.background.app),
textDecoration: 'none',
});

Expand Down