From d726d69172f39e1941815f3e50fc29ce5e927e0f Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Thu, 7 Mar 2019 07:51:16 -0800 Subject: [PATCH 1/5] feat(series): set custom series colors through spec prop --- src/lib/series/series.test.ts | 4 ++-- src/lib/series/series.ts | 13 ++++------- src/lib/series/specs.ts | 5 +++++ src/state/chart_state.test.ts | 2 +- src/state/chart_state.ts | 20 +++++++++++++++-- stories/styling.tsx | 42 +++++++++++++++++++++++++++++++++++ 6 files changed, 72 insertions(+), 14 deletions(-) diff --git a/src/lib/series/series.test.ts b/src/lib/series/series.test.ts index 961a1be4b0..a2788bb632 100644 --- a/src/lib/series/series.test.ts +++ b/src/lib/series/series.test.ts @@ -265,7 +265,7 @@ describe('Series', () => { const emptyCustomColors = new Map(); - const defaultColorMap = getSeriesColorMap(seriesColors, chartColors, emptyCustomColors, specs); + const defaultColorMap = getSeriesColorMap(seriesColors, chartColors, emptyCustomColors); const expectedDefaultColorMap = new Map(); expectedDefaultColorMap.set('spec1', 'elastic_charts_c1'); expect(defaultColorMap).toEqual(expectedDefaultColorMap); @@ -273,7 +273,7 @@ describe('Series', () => { const customColors: Map = new Map(); customColors.set('spec1', 'custom_color'); - const customizedColorMap = getSeriesColorMap(seriesColors, chartColors, customColors, specs); + const customizedColorMap = getSeriesColorMap(seriesColors, chartColors, customColors); const expectedCustomizedColorMap = new Map(); expectedCustomizedColorMap.set('spec1', 'custom_color'); expect(customizedColorMap).toEqual(expectedCustomizedColorMap); diff --git a/src/lib/series/series.ts b/src/lib/series/series.ts index 6baf74f7b1..c448523730 100644 --- a/src/lib/series/series.ts +++ b/src/lib/series/series.ts @@ -3,7 +3,6 @@ import { ColorConfig } from '../themes/theme'; import { Accessor } from '../utils/accessor'; import { GroupId, SpecId } from '../utils/ids'; import { splitSpecsByGroupId, YBasicSeriesSpec } from './domains/y_domain'; -import { getSeriesColorLabel } from './legend'; import { BasicSeriesSpec, Datum, SeriesAccessors } from './specs'; export interface RawDataSeriesDatum { @@ -148,7 +147,7 @@ function getColorValues( /** * Get the array of values that forms a series key */ -function getColorValuesAsString(colorValues: any[], specId: SpecId): string { +export function getColorValuesAsString(colorValues: any[], specId: SpecId): string { return `specId:{${specId}},colors:{${colorValues}}`; } @@ -392,17 +391,13 @@ export function getSeriesColorMap( seriesColors: Map, chartColors: ColorConfig, customColors: Map, - specs: Map, ): Map { const seriesColorMap = new Map(); let counter = 0; - seriesColors.forEach((value, seriesColorKey) => { - const spec = specs.get(value.specId); - const hasSingleSeries = seriesColors.size === 1; - const seriesLabel = getSeriesColorLabel(value, hasSingleSeries, spec); - - const color = (seriesLabel && customColors.get(seriesLabel)) || + seriesColors.forEach((value: DataSeriesColorsValues, seriesColorKey: string) => { + const customSeriesColor: string | undefined = customColors.get(seriesColorKey); + const color = customSeriesColor || chartColors.vizColors[counter % chartColors.vizColors.length]; seriesColorMap.set( diff --git a/src/lib/series/specs.ts b/src/lib/series/specs.ts index 1d062c50ef..34d281b5c8 100644 --- a/src/lib/series/specs.ts +++ b/src/lib/series/specs.ts @@ -4,6 +4,7 @@ import { Domain } from '../utils/domain'; import { AxisId, GroupId, SpecId } from '../utils/ids'; import { ScaleContinuousType, ScaleType } from '../utils/scales/scales'; import { CurveType } from './curves'; +import { DataSeriesColorsValues } from './series'; import { TooltipPosition } from './tooltip'; export type Datum = any; @@ -37,8 +38,12 @@ export interface SeriesSpec { yDomain?: Domain; /** The type of series you are looking to render */ seriesType: 'bar' | 'line' | 'area' | 'basic'; + /** Custom colors for series */ + customSeriesColors?: CustomSeriesColorsMap; } +export type CustomSeriesColorsMap = Map; + export interface SeriesAccessors { /** The field name of the x value on Datum object */ xAccessor: Accessor; diff --git a/src/state/chart_state.test.ts b/src/state/chart_state.test.ts index c224f1d18f..b671cee61e 100644 --- a/src/state/chart_state.test.ts +++ b/src/state/chart_state.test.ts @@ -484,7 +484,7 @@ describe('Chart Store', () => { store.legendItems = [firstLegendItem, secondLegendItem]; const expectedCustomColors = new Map(); - expectedCustomColors.set(firstLegendItem.label, 'foo'); + expectedCustomColors.set('specId:{spec_1},colors:{}', 'foo'); store.setSeriesColor(-1, 'foo'); expect(computeChart).not.toBeCalled(); diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index 40cd6f0c91..f179acc6ef 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -21,6 +21,7 @@ import { countClusteredSeries } from '../lib/series/scales'; import { DataSeriesColorsValues, FormattedDataSeries, + getColorValuesAsString, getSeriesColorMap, RawDataSeries, } from '../lib/series/series'; @@ -293,7 +294,8 @@ export class ChartStore { const legendItem = getLegendItemByIndex(this.legendItems, legendItemIndex); if (legendItem) { - const key = legendItem.label; + const { colorValues, specId } = legendItem.value; + const key = getColorValuesAsString(colorValues, specId); this.customSeriesColors.set(key, color); this.computeChart(); } @@ -438,6 +440,21 @@ export class ChartStore { this.selectedDataSeries = getAllDataSeriesColorValues(seriesDomains.seriesColors); } + // Merge all series spec custom colors with state custom colors map + this.seriesSpecs.forEach((spec: BasicSeriesSpec, id: SpecId) => { + if (spec.customSeriesColors) { + spec.customSeriesColors.forEach((color: string, seriesColorValues: DataSeriesColorsValues) => { + const seriesLabel = getColorValuesAsString(seriesColorValues.colorValues, id); + if (seriesLabel) { + this.customSeriesColors.set(seriesLabel, color); + } else { + // tslint:disable-next-line:no-console + console.warn('Cannot assign custom color to series: ', seriesColorValues); + } + }); + } + }); + // tslint:disable-next-line:no-console // console.log({colors: seriesDomains.seriesColors}); @@ -447,7 +464,6 @@ export class ChartStore { seriesDomains.seriesColors, this.chartTheme.colors, this.customSeriesColors, - this.seriesSpecs, ); this.legendItems = computeLegend( diff --git a/stories/styling.tsx b/stories/styling.tsx index 17e8cdad82..674ca89b29 100644 --- a/stories/styling.tsx +++ b/stories/styling.tsx @@ -21,6 +21,9 @@ import { ScaleType, Settings, } from '../src/'; +import { DataSeriesColorsValues } from '../src/lib/series/series'; +import { CustomSeriesColorsMap } from '../src/lib/series/specs'; +import * as TestDatasets from '../src/lib/series/utils/test_dataset'; import { DEFAULT_MISSING_COLOR } from '../src/lib/themes/theme_commons'; function range( @@ -348,4 +351,43 @@ storiesOf('Stylings', module) /> ); + }) + .add('series colors', () => { + // const customSeriesColors: SpecsCustomSeriesColorsMap = new Map(); + const customSeriesColors: CustomSeriesColorsMap = new Map(); + const dataSeriesColorValues: DataSeriesColorsValues = { + colorValues: ['cloudflare.com', 'direct-cdn', 'y2'], + specId: getSpecId('bars'), + }; + customSeriesColors.set(dataSeriesColorValues, '#000'); + + return ( + + + + Number(d).toFixed(2)} + /> + + + + ); }); From 4ca027265c872b820df6f47c9645ed70e19a00de Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Fri, 8 Mar 2019 09:19:30 -0800 Subject: [PATCH 2/5] refactor(chart_state): set spec customSeriesColor on setSeriesColor --- src/state/chart_state.test.ts | 11 +++++++---- src/state/chart_state.ts | 16 +++++++++++++--- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/state/chart_state.test.ts b/src/state/chart_state.test.ts index b671cee61e..7366438975 100644 --- a/src/state/chart_state.test.ts +++ b/src/state/chart_state.test.ts @@ -483,16 +483,19 @@ describe('Chart Store', () => { store.computeChart = computeChart; store.legendItems = [firstLegendItem, secondLegendItem]; - const expectedCustomColors = new Map(); - expectedCustomColors.set('specId:{spec_1},colors:{}', 'foo'); - store.setSeriesColor(-1, 'foo'); expect(computeChart).not.toBeCalled(); expect(store.customSeriesColors).toEqual(new Map()); store.setSeriesColor(0, 'foo'); expect(computeChart).toBeCalled(); - expect(store.customSeriesColors).toEqual(expectedCustomColors); + expect(store.seriesSpecs.get(firstLegendItem.value.specId)).toBeUndefined(); + + store.addSeriesSpec(spec); + store.setSeriesColor(0, 'foo'); + const expectedSpecCustomColorSeries = new Map(); + expectedSpecCustomColorSeries.set(firstLegendItem.value, 'foo'); + expect(spec.customSeriesColors).toEqual(expectedSpecCustomColorSeries); }); test('can reset selectedDataSeries', () => { diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index f179acc6ef..fac285a3b4 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -294,9 +294,19 @@ export class ChartStore { const legendItem = getLegendItemByIndex(this.legendItems, legendItemIndex); if (legendItem) { - const { colorValues, specId } = legendItem.value; - const key = getColorValuesAsString(colorValues, specId); - this.customSeriesColors.set(key, color); + const { specId } = legendItem.value; + + const spec = this.seriesSpecs.get(specId); + if (spec) { + if (spec.customSeriesColors) { + spec.customSeriesColors.set(legendItem.value, color); + } else { + const specCustomSeriesColors = new Map(); + spec.customSeriesColors = specCustomSeriesColors; + spec.customSeriesColors.set(legendItem.value, color); + } + } + this.computeChart(); } }); From bc89b356c38282b2f924c1146b89203d74de1c00 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Fri, 8 Mar 2019 09:22:17 -0800 Subject: [PATCH 3/5] test(chart_state): add test for branch where spec has customSeriesColor --- src/state/chart_state.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/state/chart_state.test.ts b/src/state/chart_state.test.ts index 7366438975..34ae13ebd4 100644 --- a/src/state/chart_state.test.ts +++ b/src/state/chart_state.test.ts @@ -496,6 +496,10 @@ describe('Chart Store', () => { const expectedSpecCustomColorSeries = new Map(); expectedSpecCustomColorSeries.set(firstLegendItem.value, 'foo'); expect(spec.customSeriesColors).toEqual(expectedSpecCustomColorSeries); + + store.setSeriesColor(1, 'bar'); + expectedSpecCustomColorSeries.set(secondLegendItem.value, 'bar'); + expect(spec.customSeriesColors).toEqual(expectedSpecCustomColorSeries); }); test('can reset selectedDataSeries', () => { From dcc1ae7f4dc43c70b45839d40433d193673dec8e Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Fri, 8 Mar 2019 10:21:18 -0800 Subject: [PATCH 4/5] refactor(chart_state): move customSeriesColor update to utils & add test --- src/state/chart_state.ts | 17 +++-------------- src/state/utils.test.ts | 33 +++++++++++++++++++++++++++++++++ src/state/utils.ts | 15 +++++++++++++++ 3 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index fac285a3b4..5ba087f6f7 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -21,7 +21,6 @@ import { countClusteredSeries } from '../lib/series/scales'; import { DataSeriesColorsValues, FormattedDataSeries, - getColorValuesAsString, getSeriesColorMap, RawDataSeries, } from '../lib/series/series'; @@ -51,6 +50,7 @@ import { getAllDataSeriesColorValues, getAxesSpecForSpecId, getLegendItemByIndex, + getUpdatedCustomSeriesColors, Transform, updateSelectedDataSeries, } from './utils'; @@ -451,19 +451,8 @@ export class ChartStore { } // Merge all series spec custom colors with state custom colors map - this.seriesSpecs.forEach((spec: BasicSeriesSpec, id: SpecId) => { - if (spec.customSeriesColors) { - spec.customSeriesColors.forEach((color: string, seriesColorValues: DataSeriesColorsValues) => { - const seriesLabel = getColorValuesAsString(seriesColorValues.colorValues, id); - if (seriesLabel) { - this.customSeriesColors.set(seriesLabel, color); - } else { - // tslint:disable-next-line:no-console - console.warn('Cannot assign custom color to series: ', seriesColorValues); - } - }); - } - }); + const updatedCustomSeriesColors = getUpdatedCustomSeriesColors(this.seriesSpecs); + this.customSeriesColors = new Map([...this.customSeriesColors, ...updatedCustomSeriesColors]); // tslint:disable-next-line:no-console // console.log({colors: seriesDomains.seriesColors}); diff --git a/src/state/utils.test.ts b/src/state/utils.test.ts index d7259bb62d..b253316319 100644 --- a/src/state/utils.test.ts +++ b/src/state/utils.test.ts @@ -11,6 +11,7 @@ import { findSelectedDataSeries, getAllDataSeriesColorValues, getLegendItemByIndex, + getUpdatedCustomSeriesColors, updateSelectedDataSeries, } from './utils'; @@ -214,4 +215,36 @@ describe('Chart State utils', () => { expect(getAllDataSeriesColorValues(colorMap)).toEqual(expected); }); + it('should get an updated customSeriesColor based on specs', () => { + const spec1: BasicSeriesSpec = { + id: getSpecId('spec1'), + groupId: getGroupId('group1'), + seriesType: 'line', + yScaleType: ScaleType.Log, + xScaleType: ScaleType.Linear, + xAccessor: 'x', + yAccessors: ['y'], + yScaleToDataExtent: false, + data: BARCHART_1Y0G, + }; + + const specs = new Map(); + specs.set(spec1.id, spec1); + + const emptyCustomSeriesColors = getUpdatedCustomSeriesColors(specs); + expect(emptyCustomSeriesColors).toEqual(new Map()); + + const dataSeriesColorValues = { + specId: spec1.id, + colorValues: ['bar'], + }; + spec1.customSeriesColors = new Map(); + spec1.customSeriesColors.set(dataSeriesColorValues, 'custom_color'); + + const updatedCustomSeriesColors = getUpdatedCustomSeriesColors(specs); + const expectedCustomSeriesColors = new Map(); + expectedCustomSeriesColors.set('specId:{spec1},colors:{bar}', 'custom_color'); + + expect(updatedCustomSeriesColors).toEqual(expectedCustomSeriesColors); + }); }); diff --git a/src/state/utils.ts b/src/state/utils.ts index 997ae8863c..761e9bb1d7 100644 --- a/src/state/utils.ts +++ b/src/state/utils.ts @@ -18,6 +18,7 @@ import { DataSeries, DataSeriesColorsValues, FormattedDataSeries, + getColorValuesAsString, getFormattedDataseries, getSplittedSeries, RawDataSeries, @@ -89,6 +90,20 @@ export function updateSelectedDataSeries( return updatedSeries; } +export function getUpdatedCustomSeriesColors(seriesSpecs: Map): Map { + const updatedCustomSeriesColors = new Map(); + seriesSpecs.forEach((spec: BasicSeriesSpec, id: SpecId) => { + if (spec.customSeriesColors) { + spec.customSeriesColors.forEach((color: string, seriesColorValues: DataSeriesColorsValues) => { + const { colorValues, specId } = seriesColorValues; + const seriesLabel = getColorValuesAsString(colorValues, specId); + updatedCustomSeriesColors.set(seriesLabel, color); + }); + } + }); + return updatedCustomSeriesColors; +} + export function computeSeriesDomains( seriesSpecs: Map, selectedDataSeries?: DataSeriesColorsValues[] | null, From 2ab4a667c75d4cb52fb5185fd906a44b4b13565a Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Fri, 8 Mar 2019 10:54:12 -0800 Subject: [PATCH 5/5] docs(series/styling): add know for customSeriesColor props --- stories/styling.tsx | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/stories/styling.tsx b/stories/styling.tsx index 674ca89b29..183165f1b8 100644 --- a/stories/styling.tsx +++ b/stories/styling.tsx @@ -352,14 +352,23 @@ storiesOf('Stylings', module) ); }) - .add('series colors', () => { - // const customSeriesColors: SpecsCustomSeriesColorsMap = new Map(); - const customSeriesColors: CustomSeriesColorsMap = new Map(); - const dataSeriesColorValues: DataSeriesColorsValues = { + .add('custom series colors through spec props', () => { + const barCustomSeriesColors: CustomSeriesColorsMap = new Map(); + const barDataSeriesColorValues: DataSeriesColorsValues = { colorValues: ['cloudflare.com', 'direct-cdn', 'y2'], specId: getSpecId('bars'), }; - customSeriesColors.set(dataSeriesColorValues, '#000'); + + const lineCustomSeriesColors: CustomSeriesColorsMap = new Map(); + const lineDataSeriesColorValues: DataSeriesColorsValues = { + colorValues: [], + specId: getSpecId('lines'), + }; + + const customBarColorKnob = color('barDataSeriesColor', '#000'); + const customLineColorKnob = color('lineDataSeriesColor', '#ff0'); + barCustomSeriesColors.set(barDataSeriesColorValues, customBarColorKnob); + lineCustomSeriesColors.set(lineDataSeriesColorValues, customLineColorKnob); return ( @@ -384,10 +393,20 @@ storiesOf('Stylings', module) xAccessor="x" yAccessors={['y1', 'y2']} splitSeriesAccessors={['g1', 'g2']} - customSeriesColors={customSeriesColors} + customSeriesColors={barCustomSeriesColors} data={TestDatasets.BARCHART_2Y2G} yScaleToDataExtent={false} /> + ); });