From 1cd1bf9cbed8c47d6e12117c519b07653cf853c3 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Wed, 30 Sep 2020 11:51:14 +0200 Subject: [PATCH 1/4] Optimize charts plugin --- .../charts/public/services/colors/color_palette.ts | 6 +++--- .../charts/public/services/colors/colors_palette.test.ts | 6 ++++++ .../charts/public/services/colors/mapped_colors.test.ts | 8 ++++---- .../charts/public/services/colors/mapped_colors.ts | 4 ++-- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/plugins/charts/public/services/colors/color_palette.ts b/src/plugins/charts/public/services/colors/color_palette.ts index 464e9e3a66101..057e4cc45e7a2 100644 --- a/src/plugins/charts/public/services/colors/color_palette.ts +++ b/src/plugins/charts/public/services/colors/color_palette.ts @@ -17,8 +17,8 @@ * under the License. */ -import d3 from 'd3'; import _ from 'lodash'; +import { hsl } from 'color'; import { seedColors } from './seed_colors'; @@ -49,7 +49,7 @@ const fraction = function (goal: number) { * If the number is greater than the length of seed colors available, * new colors are generated up to the value of the input number. */ -export function createColorPalette(num?: any): string[] { +export function createColorPalette(num: number): string[] { if (!_.isNumber(num)) { throw new TypeError('ColorPaletteUtilService expects a number'); } @@ -58,7 +58,7 @@ export function createColorPalette(num?: any): string[] { const seedLength = seedColors.length; _.times(num - seedLength, function (i) { - colors.push(d3.hsl((fraction(i + seedLength + 1) * 360 + offset) % 360, 0.5, 0.5).toString()); + colors.push(hsl((fraction(i + seedLength + 1) * 360 + offset) % 360, 0.5, 0.5).toString()); }); return colors; diff --git a/src/plugins/charts/public/services/colors/colors_palette.test.ts b/src/plugins/charts/public/services/colors/colors_palette.test.ts index 6612447cefe9e..02ff5a6056d54 100644 --- a/src/plugins/charts/public/services/colors/colors_palette.test.ts +++ b/src/plugins/charts/public/services/colors/colors_palette.test.ts @@ -37,26 +37,32 @@ describe('Color Palette', () => { it('should throw an error if input is not a number', () => { expect(() => { + // @ts-expect-error createColorPalette(string); }).toThrowError(); expect(() => { + // @ts-expect-error createColorPalette(bool); }).toThrowError(); expect(() => { + // @ts-expect-error createColorPalette(nullValue); }).toThrowError(); expect(() => { + // @ts-expect-error createColorPalette(emptyArr); }).toThrowError(); expect(() => { + // @ts-expect-error createColorPalette(emptyObject); }).toThrowError(); expect(() => { + // @ts-expect-error createColorPalette(); }).toThrowError(); }); diff --git a/src/plugins/charts/public/services/colors/mapped_colors.test.ts b/src/plugins/charts/public/services/colors/mapped_colors.test.ts index e97ca8ac257b4..29e681109576e 100644 --- a/src/plugins/charts/public/services/colors/mapped_colors.test.ts +++ b/src/plugins/charts/public/services/colors/mapped_colors.test.ts @@ -18,7 +18,7 @@ */ import _ from 'lodash'; -import d3 from 'd3'; +import { rgb } from 'color'; import { coreMock } from '../../../../../core/public/mocks'; import { COLOR_MAPPING_SETTING } from '../../../common'; @@ -78,9 +78,9 @@ describe('Mapped Colors', () => { }); it('should treat different formats of colors as equal', () => { - const color = d3.rgb(seedColors[0]); - const rgb = `rgb(${color.r}, ${color.g}, ${color.b})`; - const newConfig = { bar: rgb }; + const color = rgb(seedColors[0]); + const rgbColorString = `rgbColorString(${color.red}, ${color.green}, ${color.blue})`; + const newConfig = { bar: rgbColorString }; config.set(COLOR_MAPPING_SETTING, newConfig); const arr = ['foo', 'bar', 'baz', 'qux']; diff --git a/src/plugins/charts/public/services/colors/mapped_colors.ts b/src/plugins/charts/public/services/colors/mapped_colors.ts index 3b9e1501d638d..e64a037596391 100644 --- a/src/plugins/charts/public/services/colors/mapped_colors.ts +++ b/src/plugins/charts/public/services/colors/mapped_colors.ts @@ -18,14 +18,14 @@ */ import _ from 'lodash'; -import d3 from 'd3'; +import { rgb } from 'color'; import { CoreSetup } from 'kibana/public'; import { COLOR_MAPPING_SETTING } from '../../../common'; import { createColorPalette } from './color_palette'; -const standardizeColor = (color: string) => d3.rgb(color).toString(); +const standardizeColor = (color: string) => rgb(color).toString(); /** * Maintains a lookup table that associates the value (key) with a hex color (value) From c2e98bc003c51f3fc174c8c335c4bce7201acec2 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Wed, 30 Sep 2020 14:03:07 +0200 Subject: [PATCH 2/4] Fix issues to keep same behavior --- .../charts/public/services/colors/color_palette.ts | 2 +- .../public/services/colors/mapped_colors.test.ts | 10 ++++++---- .../charts/public/services/colors/mapped_colors.ts | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/plugins/charts/public/services/colors/color_palette.ts b/src/plugins/charts/public/services/colors/color_palette.ts index 057e4cc45e7a2..e1c32fe68da12 100644 --- a/src/plugins/charts/public/services/colors/color_palette.ts +++ b/src/plugins/charts/public/services/colors/color_palette.ts @@ -58,7 +58,7 @@ export function createColorPalette(num: number): string[] { const seedLength = seedColors.length; _.times(num - seedLength, function (i) { - colors.push(hsl((fraction(i + seedLength + 1) * 360 + offset) % 360, 0.5, 0.5).toString()); + colors.push(hsl((fraction(i + seedLength + 1) * 360 + offset) % 360, 0.5, 0.5).hex()); }); return colors; diff --git a/src/plugins/charts/public/services/colors/mapped_colors.test.ts b/src/plugins/charts/public/services/colors/mapped_colors.test.ts index 29e681109576e..a86b9e74d986e 100644 --- a/src/plugins/charts/public/services/colors/mapped_colors.test.ts +++ b/src/plugins/charts/public/services/colors/mapped_colors.test.ts @@ -18,7 +18,9 @@ */ import _ from 'lodash'; -import { rgb } from 'color'; +import Color from 'color'; + +import d3 from 'd3'; import { coreMock } from '../../../../../core/public/mocks'; import { COLOR_MAPPING_SETTING } from '../../../common'; @@ -61,7 +63,7 @@ describe('Mapped Colors', () => { mappedColors.mapKeys(arr); const colorValues = _(mappedColors.mapping).values(); - expect(colorValues.includes(seedColors[0])).toBe(false); + expect(colorValues).not.toContain(seedColors[0]); expect(colorValues.uniq().size()).toBe(arr.length); }); @@ -78,8 +80,8 @@ describe('Mapped Colors', () => { }); it('should treat different formats of colors as equal', () => { - const color = rgb(seedColors[0]); - const rgbColorString = `rgbColorString(${color.red}, ${color.green}, ${color.blue})`; + const color = new Color(seedColors[0]); + const rgbColorString = `rgb(${color.red()}, ${color.green()}, ${color.blue()})`; const newConfig = { bar: rgbColorString }; config.set(COLOR_MAPPING_SETTING, newConfig); diff --git a/src/plugins/charts/public/services/colors/mapped_colors.ts b/src/plugins/charts/public/services/colors/mapped_colors.ts index e64a037596391..15f9be32b829c 100644 --- a/src/plugins/charts/public/services/colors/mapped_colors.ts +++ b/src/plugins/charts/public/services/colors/mapped_colors.ts @@ -18,14 +18,14 @@ */ import _ from 'lodash'; -import { rgb } from 'color'; +import Color from 'color'; import { CoreSetup } from 'kibana/public'; import { COLOR_MAPPING_SETTING } from '../../../common'; import { createColorPalette } from './color_palette'; -const standardizeColor = (color: string) => rgb(color).toString(); +const standardizeColor = (color: string) => new Color(color).hex().toLowerCase(); /** * Maintains a lookup table that associates the value (key) with a hex color (value) From fb58b89fa8d2146f416f6007f5ed7f28b20a40c2 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Wed, 30 Sep 2020 14:07:21 +0200 Subject: [PATCH 3/4] Remove dead import --- src/plugins/charts/public/services/colors/mapped_colors.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/plugins/charts/public/services/colors/mapped_colors.test.ts b/src/plugins/charts/public/services/colors/mapped_colors.test.ts index a86b9e74d986e..2f67fc5c8f6c3 100644 --- a/src/plugins/charts/public/services/colors/mapped_colors.test.ts +++ b/src/plugins/charts/public/services/colors/mapped_colors.test.ts @@ -20,8 +20,6 @@ import _ from 'lodash'; import Color from 'color'; -import d3 from 'd3'; - import { coreMock } from '../../../../../core/public/mocks'; import { COLOR_MAPPING_SETTING } from '../../../common'; import { seedColors } from './seed_colors'; From 85f50b7f32810bab2c5f25c4ea560606bf1eadf9 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Wed, 30 Sep 2020 14:09:07 +0200 Subject: [PATCH 4/4] Revert name change --- .../charts/public/services/colors/mapped_colors.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/charts/public/services/colors/mapped_colors.test.ts b/src/plugins/charts/public/services/colors/mapped_colors.test.ts index 2f67fc5c8f6c3..9d00bf098de4c 100644 --- a/src/plugins/charts/public/services/colors/mapped_colors.test.ts +++ b/src/plugins/charts/public/services/colors/mapped_colors.test.ts @@ -79,8 +79,8 @@ describe('Mapped Colors', () => { it('should treat different formats of colors as equal', () => { const color = new Color(seedColors[0]); - const rgbColorString = `rgb(${color.red()}, ${color.green()}, ${color.blue()})`; - const newConfig = { bar: rgbColorString }; + const rgb = `rgb(${color.red()}, ${color.green()}, ${color.blue()})`; + const newConfig = { bar: rgb }; config.set(COLOR_MAPPING_SETTING, newConfig); const arr = ['foo', 'bar', 'baz', 'qux'];