Skip to content

Commit

Permalink
feat: cleaner color API on SeriesSpec (opensearch-project#571)
Browse files Browse the repository at this point in the history
Refactor naming of `customSeriesColors` to `color` to be inline with new `name` prop on `SeriesSpec`. The `CustomSeriesColors` type  (which was `CustomSeriesColors`) now accepts a single color as string.

BREAKING CHANGE: `customSeriesColors` prop on `SeriesSpec` is now `color`. The `CustomSeriesColors` type is  replaced with `SeriesColorAccessor`.
  • Loading branch information
nickofthyme authored and markov00 committed Mar 2, 2020
1 parent f702e2c commit 6a78c4e
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 27 deletions.
25 changes: 18 additions & 7 deletions packages/osd-charts/src/chart_types/xy_chart/state/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ describe('Chart State utils', () => {
});

describe('series collection is not empty', () => {
it('it should return an empty map if no customSeriesColors', () => {
it('it should return an empty map if no color', () => {
const barSpec1 = MockSeriesSpec.bar({ id: specId1, data });
const barSpec2 = MockSeriesSpec.bar({ id: specId2, data });
const barSeriesSpecs = MockSeriesSpecs.fromSpecs([barSpec1, barSpec2]);
Expand All @@ -348,14 +348,25 @@ describe('Chart State utils', () => {
expect(actual.size).toBe(0);
});

it('it should return string color value', () => {
const color = 'green';
const barSpec1 = MockSeriesSpec.bar({ id: specId1, data, color });
const barSpec2 = MockSeriesSpec.bar({ id: specId2, data });
const barSeriesSpecs = MockSeriesSpecs.fromSpecs([barSpec1, barSpec2]);
const barSeriesCollection = MockSeriesCollection.fromSpecs(barSeriesSpecs);
const actual = getCustomSeriesColors(barSeriesSpecs, barSeriesCollection, new Map());

expect([...actual.values()]).toEqualArrayOf(color);
});

describe('with customSeriesColors array', () => {
const customSeriesColors = ['red', 'blue', 'green'];
const barSpec1 = MockSeriesSpec.bar({ id: specId1, data, customSeriesColors });
const barSpec1 = MockSeriesSpec.bar({ id: specId1, data, color: customSeriesColors });
const barSpec2 = MockSeriesSpec.bar({ id: specId2, data });
const barSeriesSpecs = MockSeriesSpecs.fromSpecs([barSpec1, barSpec2]);
const barSeriesCollection = MockSeriesCollection.fromSpecs(barSeriesSpecs);

it('it should return color from customSeriesColors array', () => {
it('it should return color from color array', () => {
const actual = getCustomSeriesColors(barSeriesSpecs, barSeriesCollection, new Map());

expect(actual.size).toBe(4);
Expand Down Expand Up @@ -386,20 +397,20 @@ describe('Chart State utils', () => {
});
});

describe('with customSeriesColors function', () => {
const customSeriesColors: SeriesColorAccessorFn = ({ yAccessor, splitAccessors }) => {
describe('with color function', () => {
const color: SeriesColorAccessorFn = ({ yAccessor, splitAccessors }) => {
if (yAccessor === 'y' && splitAccessors.get('g') === 'b') {
return 'aquamarine';
}

return null;
};
const barSpec1 = MockSeriesSpec.bar({ id: specId1, yAccessors: ['y'], data, customSeriesColors });
const barSpec1 = MockSeriesSpec.bar({ id: specId1, yAccessors: ['y'], data, color });
const barSpec2 = MockSeriesSpec.bar({ id: specId2, data });
const barSeriesSpecs = MockSeriesSpecs.fromSpecs([barSpec1, barSpec2]);
const barSeriesCollection = MockSeriesCollection.fromSpecs(barSeriesSpecs);

it('it should return color from customSeriesColors function', () => {
it('it should return color from color function', () => {
const actual = getCustomSeriesColors(barSeriesSpecs, barSeriesCollection, new Map());

expect(actual.size).toBe(1);
Expand Down
16 changes: 9 additions & 7 deletions packages/osd-charts/src/chart_types/xy_chart/state/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export function getCustomSeriesColors(
seriesCollection.forEach(({ seriesIdentifier }, seriesKey) => {
const spec = getSpecsById(seriesSpecs, seriesIdentifier.specId);

if (!spec || !(spec.customSeriesColors || seriesColorOverrides.size > 0)) {
if (!spec || !(spec.color || seriesColorOverrides.size > 0)) {
return;
}

Expand All @@ -141,12 +141,14 @@ export function getCustomSeriesColors(
color = seriesColorOverrides.get(seriesKey);
}

if (!color && spec.customSeriesColors) {
const counter = counters.get(seriesIdentifier.specId) || 0;
color = Array.isArray(spec.customSeriesColors)
? spec.customSeriesColors[counter % spec.customSeriesColors.length]
: spec.customSeriesColors(seriesIdentifier);
counters.set(seriesIdentifier.specId, counter + 1);
if (!color && spec.color) {
if (typeof spec.color === 'string') {
color = spec.color;
} else {
const counter = counters.get(seriesIdentifier.specId) || 0;
color = Array.isArray(spec.color) ? spec.color[counter % spec.color.length] : spec.color(seriesIdentifier);
counters.set(seriesIdentifier.specId, counter + 1);
}
}

if (color) {
Expand Down
4 changes: 2 additions & 2 deletions packages/osd-charts/src/chart_types/xy_chart/utils/specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ export interface SeriesSpec extends Spec {
/** The type of series you are looking to render */
seriesType: SeriesTypes;
/** Set colors for specific series */
customSeriesColors?: CustomSeriesColors;
color?: SeriesColorAccessor;
/** If the series should appear in the legend
* @default false
*/
Expand Down Expand Up @@ -307,7 +307,7 @@ export interface Postfixes {

export type SeriesColorsArray = string[];
export type SeriesColorAccessorFn = (seriesIdentifier: XYChartSeriesIdentifier) => string | null;
export type CustomSeriesColors = SeriesColorsArray | SeriesColorAccessorFn;
export type SeriesColorAccessor = string | SeriesColorsArray | SeriesColorAccessorFn;

export interface SeriesAccessors {
/** The field name of the x value on Datum object */
Expand Down
2 changes: 1 addition & 1 deletion packages/osd-charts/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export { SeriesIdentifier, XYChartSeriesIdentifier } from './chart_types/xy_char
export {
AnnotationDomainType,
AnnotationDomainTypes,
CustomSeriesColors,
SeriesColorAccessor,
SeriesColorsArray,
SeriesColorAccessorFn,
HistogramModeAlignment,
Expand Down
6 changes: 3 additions & 3 deletions packages/osd-charts/src/utils/themes/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,23 +131,23 @@ export interface Theme {
*
* __Note:__ This is not used to set the color of a specific series. As such, any changes to the styles will not be reflected in the tooltip, legend, etc..
*
* You may use `CustomSeriesColors` to assign colors to a given series or replace the `theme.colors.vizColors` colors to your desired colors.
* You may use `SeriesColorAccessor` to assign colors to a given series or replace the `theme.colors.vizColors` colors to your desired colors.
*/
lineSeriesStyle: LineSeriesStyle;
/**
* Global area styles.
*
* __Note:__ This is not used to set the color of a specific series. As such, any changes to the styles will not be reflected in the tooltip, legend, etc..
*
* You may use `CustomSeriesColors` to assign colors to a given series or replace the `theme.colors.vizColors` colors to your desired colors.
* You may use `SeriesColorAccessor` to assign colors to a given series or replace the `theme.colors.vizColors` colors to your desired colors.
*/
areaSeriesStyle: AreaSeriesStyle;
/**
* Global bar styles.
*
* __Note:__ This is not used to set the color of a specific series. As such, any changes to the styles will not be reflected in the tooltip, legend, etc..
*
* You may use `CustomSeriesColors` to assign colors to a given series or replace the `theme.colors.vizColors` colors to your desired colors.
* You may use `SeriesColorAccessor` to assign colors to a given series or replace the `theme.colors.vizColors` colors to your desired colors.
*/
barSeriesStyle: BarSeriesStyle;
arcSeriesStyle: ArcSeriesStyle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const example = () => {
xAccessor="x"
yAccessors={['y1']}
splitSeriesAccessors={['g1', 'g2']}
customSeriesColors={['red', 'orange', 'blue', 'green', 'black', 'lightgrey']}
color={['red', 'orange', 'blue', 'green', 'black', 'lightgrey']}
data={TestDatasets.BARCHART_2Y2G}
/>
</Chart>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { color } from '@storybook/addon-knobs';
import React from 'react';

import { Axis, BarSeries, Chart, CustomSeriesColors, LineSeries, Position, ScaleType, Settings } from '../../src';
import { Axis, BarSeries, Chart, SeriesColorAccessor, LineSeries, Position, ScaleType, Settings } from '../../src';
import * as TestDatasets from '../../src/utils/data_samples/test_dataset';

export const example = () => {
const barColor = color('barSeriesColor', '#000');
const barSeriesColorAccessor: CustomSeriesColors = ({ specId, yAccessor, splitAccessors }) => {
const barSeriesColorAccessor: SeriesColorAccessor = ({ specId, yAccessor, splitAccessors }) => {
if (
specId === 'bars' &&
yAccessor === 'y1' &&
Expand All @@ -21,7 +21,7 @@ export const example = () => {
};

const lineColor = color('linelineSeriesColor', '#ff0');
const lineSeriesColorAccessor: CustomSeriesColors = ({ specId, yAccessor, splitAccessors }) => {
const lineSeriesColorAccessor: SeriesColorAccessor = ({ specId, yAccessor, splitAccessors }) => {
// eslint-disable-next-line react/prop-types
if (specId === 'lines' && yAccessor === 'y1' && splitAccessors.size === 0) {
return lineColor;
Expand All @@ -42,7 +42,7 @@ export const example = () => {
xAccessor="x"
yAccessors={['y1', 'y2']}
splitSeriesAccessors={['g1', 'g2']}
customSeriesColors={barSeriesColorAccessor}
color={barSeriesColorAccessor}
data={TestDatasets.BARCHART_2Y2G}
/>
<LineSeries
Expand All @@ -51,7 +51,7 @@ export const example = () => {
yScaleType={ScaleType.Linear}
xAccessor="x"
yAccessors={['y']}
customSeriesColors={lineSeriesColorAccessor}
color={lineSeriesColorAccessor}
data={[
{ x: 0, y: 3 },
{ x: 1, y: 2 },
Expand Down
2 changes: 1 addition & 1 deletion packages/osd-charts/wiki/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export interface SeriesSpec {
/** The type of series you are looking to render */
seriesType: SeriesTypes;
/** Set colors for specific series */
customSeriesColors?: CustomSeriesColors;
color?: SeriesColorAccessor;
/** If the series should appear in the legend
* @default false
*/
Expand Down

0 comments on commit 6a78c4e

Please sign in to comment.