From dcac70b4ca6997a44e7aad77e7b4eebcfc7364d2 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Thu, 13 Dec 2018 09:39:47 -0700 Subject: [PATCH 1/3] Allow any value in EuiIcon --- src-docs/src/views/icon/icon_example.js | 7 +- .../__snapshots__/accordion.test.js.snap | 24 +-- .../in_memory_table.test.js.snap | 36 +--- .../__snapshots__/super_select.test.js.snap | 36 +--- .../icon/__snapshots__/icon.test.tsx.snap | 158 ++++++++++++++++++ src/components/icon/icon.test.tsx | 13 ++ src/components/icon/icon.tsx | 14 -- 7 files changed, 191 insertions(+), 97 deletions(-) diff --git a/src-docs/src/views/icon/icon_example.js b/src-docs/src/views/icon/icon_example.js index 7b035eace14..382c5957f0f 100644 --- a/src-docs/src/views/icon/icon_example.js +++ b/src-docs/src/views/icon/icon_example.js @@ -204,9 +204,10 @@ export const IconExample = { text: (

Use the color prop to assign a color for your icons. It - can accept named colors from our pallete or a three or six color hex code. - The default behavior is to inherit the text color as the SVG - color fill property via currentColor in CSS. + can accept named colors from our palette, a three or six color hex code, + and rgb(r, g, b) or rgba(r, g, b, a) formats. The default behavior is to + inherit the text color as the SVG color fill property + via currentColor in CSS.

), demo: , diff --git a/src/components/accordion/__snapshots__/accordion.test.js.snap b/src/components/accordion/__snapshots__/accordion.test.js.snap index 0b2e9e332c3..5f2ff86f0ac 100644 --- a/src/components/accordion/__snapshots__/accordion.test.js.snap +++ b/src/components/accordion/__snapshots__/accordion.test.js.snap @@ -62,11 +62,7 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = ` className="euiIcon euiIcon--medium" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -76,11 +72,7 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = ` className="euiIcon euiIcon--medium" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -204,11 +196,7 @@ exports[`EuiAccordion behavior opens when clicked once 1`] = ` className="euiIcon euiIcon--medium" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -218,11 +206,7 @@ exports[`EuiAccordion behavior opens when clicked once 1`] = ` className="euiIcon euiIcon--medium" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" diff --git a/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap b/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap index e4fa86af344..0c606f001b6 100644 --- a/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap +++ b/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap @@ -331,11 +331,7 @@ exports[`EuiInMemoryTable behavior pagination 1`] = ` className="euiIcon euiIcon--medium euiButtonEmpty__icon" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -346,11 +342,7 @@ exports[`EuiInMemoryTable behavior pagination 1`] = ` className="euiIcon euiIcon--medium euiButtonEmpty__icon" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -428,11 +420,7 @@ exports[`EuiInMemoryTable behavior pagination 1`] = ` className="euiIcon euiIcon--medium euiButtonIcon__icon" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -443,11 +431,7 @@ exports[`EuiInMemoryTable behavior pagination 1`] = ` className="euiIcon euiIcon--medium euiButtonIcon__icon" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -572,11 +556,7 @@ exports[`EuiInMemoryTable behavior pagination 1`] = ` className="euiIcon euiIcon--medium euiButtonIcon__icon" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -587,11 +567,7 @@ exports[`EuiInMemoryTable behavior pagination 1`] = ` className="euiIcon euiIcon--medium euiButtonIcon__icon" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" diff --git a/src/components/form/super_select/__snapshots__/super_select.test.js.snap b/src/components/form/super_select/__snapshots__/super_select.test.js.snap index 1841e21edcb..7d57846836d 100644 --- a/src/components/form/super_select/__snapshots__/super_select.test.js.snap +++ b/src/components/form/super_select/__snapshots__/super_select.test.js.snap @@ -588,11 +588,7 @@ exports[`EuiSuperSelect props more props are propogated to each option 2`] = ` className="euiIcon euiIcon--medium euiFormControlLayoutCustomIcon__icon" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -603,11 +599,7 @@ exports[`EuiSuperSelect props more props are propogated to each option 2`] = ` className="euiIcon euiIcon--medium euiFormControlLayoutCustomIcon__icon" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -749,11 +741,7 @@ exports[`EuiSuperSelect props more props are propogated to each option 2`] = ` className="euiIcon euiIcon--medium euiContextMenu__icon" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -762,11 +750,7 @@ exports[`EuiSuperSelect props more props are propogated to each option 2`] = ` className="euiIcon euiIcon--medium euiContextMenu__icon" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -829,11 +813,7 @@ exports[`EuiSuperSelect props more props are propogated to each option 2`] = ` className="euiIcon euiIcon--medium euiContextMenu__icon" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" @@ -842,11 +822,7 @@ exports[`EuiSuperSelect props more props are propogated to each option 2`] = ` className="euiIcon euiIcon--medium euiContextMenu__icon" focusable="false" height="16" - style={ - Object { - "fill": undefined, - } - } + style={null} viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" diff --git a/src/components/icon/__snapshots__/icon.test.tsx.snap b/src/components/icon/__snapshots__/icon.test.tsx.snap index 9c6e1aabfcd..9a43ed394a9 100644 --- a/src/components/icon/__snapshots__/icon.test.tsx.snap +++ b/src/components/icon/__snapshots__/icon.test.tsx.snap @@ -24,6 +24,164 @@ exports[`EuiIcon is rendered 1`] = ` `; +exports[`EuiIcon props color #885522 is rendered 1`] = ` + +`; + +exports[`EuiIcon props color #fde is rendered 1`] = ` + +`; + +exports[`EuiIcon props color accent is rendered 1`] = ` + +`; + +exports[`EuiIcon props color danger is rendered 1`] = ` + +`; + +exports[`EuiIcon props color default is rendered 1`] = ` + +`; + +exports[`EuiIcon props color ghost is rendered 1`] = ` + +`; + +exports[`EuiIcon props color hsla(270, 60%, 70%, 0.9) is rendered 1`] = ` + +`; + +exports[`EuiIcon props color primary is rendered 1`] = ` + +`; + +exports[`EuiIcon props color rgb(100, 150, 200) is rendered 1`] = ` + +`; + +exports[`EuiIcon props color secondary is rendered 1`] = ` + +`; + +exports[`EuiIcon props color subdued is rendered 1`] = ` + +`; + +exports[`EuiIcon props color success is rendered 1`] = ` + +`; + +exports[`EuiIcon props color text is rendered 1`] = ` + +`; + +exports[`EuiIcon props color warning is rendered 1`] = ` + +`; + exports[`EuiIcon props other props are passed through to the icon 1`] = ` { @@ -52,6 +53,18 @@ describe('EuiIcon', () => { }); }); + describe('color', () => { + COLORS.concat(['#fde', '#885522', 'rgb(100, 150, 200)', 'hsla(270, 60%, 70%, 0.9)']).forEach(color => { + test(`${color} is rendered`, () => { + const component = render( + + ); + + expect(component).toMatchSnapshot(); + }); + }); + }); + describe('tabIndex', () => { test('renders focusable="false" when not provided', () => { const component = render(); diff --git a/src/components/icon/icon.tsx b/src/components/icon/icon.tsx index d1011fd2877..6a4178232d2 100644 --- a/src/components/icon/icon.tsx +++ b/src/components/icon/icon.tsx @@ -604,15 +604,11 @@ export const EuiIcon: SFC = ({ let optionalCustomStyles = null; if (color) { - checkValidColor(color, type || 'empty'); - if (COLORS.indexOf(color) > -1) { optionalColorClass = colorToClassMap[color]; } else { optionalCustomStyles = { fill: color }; } - } else { - optionalCustomStyles = { fill: undefined }; } // These icons are a little special and get some extra CSS flexibility @@ -649,16 +645,6 @@ export const EuiIcon: SFC = ({ ); }; -function checkValidColor(color: string, type: string) { - const validHex = /(^#[0-9A-F]{6}$)|(^#[0-9A-F]{3}$)/i.test(color); - if (color && !validHex && !COLORS.includes(color)) { - throw new Error( - `EuiIcon[${type}} needs to pass a valid color. This can either be a three ` + - `or six character hex value or one of the following: ${COLORS}` - ); - } -} - EuiIcon.defaultProps = { size: 'm', }; From 14cf5c372f2461c79ac25abe8fe6c15e5f74eaad Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Thu, 13 Dec 2018 09:44:36 -0700 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8877a2e9bf..b00d5e58c8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ ## [`master`](https://github.com/elastic/eui/tree/master) - Reinstate ([#1353](https://github.com/elastic/eui/pull/1353)) `onBlur` action on `EuiComboBox` ([#1364](https://github.com/elastic/eui/pull/1364)) -- Convert roughly half of the services to TypeScript ([#1400](https://github.com/elastic/eui/pull/1400)) +- Convert roughly half of the services to TypeScript ([#1360](https://github.com/elastic/eui/pull/1360)) **Bug fixes** @@ -9,6 +9,7 @@ - Added `anchorClassName` prop to `EuiPopover` ([#1367](https://github.com/elastic/eui/pull/1367)) - Added support for `fullWidth` on `EuiSuperSelect` ([#1367](https://github.com/elastic/eui/pull/1367)) - Applied new scrollbar customization for Firefox ([#1367](https://github.com/elastic/eui/pull/1367)) +- Allow any color value to be passed to `EuiIcon` ([#1370](https://github.com/elastic/eui/pull/1370)) ## [`5.7.0`](https://github.com/elastic/eui/tree/v5.7.0) From c7685f2c3a79f95a81ab33a2f906de8a882534b9 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Thu, 13 Dec 2018 10:25:35 -0700 Subject: [PATCH 3/3] feedback --- src-docs/src/views/icon/icon_example.js | 13 ++++++++----- src/components/icon/icon.tsx | 3 +++ 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src-docs/src/views/icon/icon_example.js b/src-docs/src/views/icon/icon_example.js index 382c5957f0f..4b96a21299f 100644 --- a/src-docs/src/views/icon/icon_example.js +++ b/src-docs/src/views/icon/icon_example.js @@ -10,6 +10,7 @@ import { EuiCode, EuiIcon, EuiToken, + EuiLink, } from '../../../../src/components'; const iconHtmlWarning = () => ( @@ -203,11 +204,13 @@ export const IconExample = { }], text: (

- Use the color prop to assign a color for your icons. It - can accept named colors from our palette, a three or six color hex code, - and rgb(r, g, b) or rgba(r, g, b, a) formats. The default behavior is to - inherit the text color as the SVG color fill property - via currentColor in CSS. + The default behavior of icons is to inherit from the text color. You can + use the color prop to assign a custom color which + accepts a named color from our palette or a valid  + CSS color data type +  which will be passed down through the inline-style fill  + property. We recommend relying on the EUI named color palette  + unless the custom color is initiated by the user (like as a graph color).

), demo: , diff --git a/src/components/icon/icon.tsx b/src/components/icon/icon.tsx index 6a4178232d2..78884a619a2 100644 --- a/src/components/icon/icon.tsx +++ b/src/components/icon/icon.tsx @@ -586,6 +586,9 @@ export type IconSize = keyof typeof sizeToClassNameMap; export interface EuiIconProps { type?: IconType; + /** + * One of EUI's color palette or a valid CSS color value https://developer.mozilla.org/en-US/docs/Web/CSS/color_value + */ color?: IconColor; size?: IconSize; }