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: point style accessor for isolated points #2464

Merged
merged 8 commits into from
Jun 12, 2024
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 20 additions & 0 deletions e2e/tests/test_cases_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,24 @@ test.describe('Test cases stories', () => {
);
});
});

test.describe('Isolated point styles', () => {
test('should override theme point and iso point styles with series styles', async ({ page }) => {
await common.expectChartAtUrlToMatchScreenshot(page)(
'http://localhost:9001/?path=/story/test-cases--point-style-overrides&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-isolatedPoint.stroke - series level=orange&knob-isolatedPoint.stroke - theme level=green&knob-point.stroke - series level=blue&knob-point.stroke - theme level=red&knob-series type=line&knob-stroke - pointStyleAccessor=black&knob-use point style overrides=false&knob-use series iso overrides=false&knob-use series overrides=true',
);
});

test('should override theme and series point styles with iso point styles', async ({ page }) => {
await common.expectChartAtUrlToMatchScreenshot(page)(
'http://localhost:9001/?path=/story/test-cases--point-style-overrides&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-isolatedPoint.stroke - series level=orange&knob-isolatedPoint.stroke - theme level=green&knob-point.stroke - series level=blue&knob-point.stroke - theme level=red&knob-series type=line&knob-stroke - pointStyleAccessor=black&knob-use point style overrides=false&knob-use series iso overrides=true&knob-use series overrides=true',
);
});

test('should override all point styles with point accessor styles', async ({ page }) => {
await common.expectChartAtUrlToMatchScreenshot(page)(
'http://localhost:9001/?path=/story/test-cases--point-style-overrides&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-isolatedPoint.stroke - series level=orange&knob-isolatedPoint.stroke - theme level=green&knob-point.stroke - series level=blue&knob-point.stroke - theme level=red&knob-series type=line&knob-stroke - pointStyleAccessor=black&knob-use point style overrides=true&knob-use series iso overrides=true&knob-use series overrides=true',
);
});
});
});
2 changes: 1 addition & 1 deletion packages/charts/api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2446,7 +2446,7 @@ export interface PointStyle {
}

// @public
export type PointStyleAccessor = (datum: DataSeriesDatum, seriesIdentifier: XYChartSeriesIdentifier) => PointStyleOverride;
export type PointStyleAccessor = (datum: DataSeriesDatum, seriesIdentifier: XYChartSeriesIdentifier, isolatedPoint: boolean) => PointStyleOverride;

// @public (undocumented)
export type PointStyleOverride = RecursivePartial<PointStyle> | Color | null;
Expand Down
9 changes: 5 additions & 4 deletions packages/charts/src/chart_types/xy_chart/rendering/points.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ export function renderPoints(
const y = yDatumKeyName === 'y1' ? y1Fn(datum) : y0Fn(datum);
const originalY = getDatumYValue(datum, keyIndex === 0, isBandedSpec, dataSeries.stackMode);
const seriesIdentifier: XYChartSeriesIdentifier = getSeriesIdentifierFromDataSeries(dataSeries);
const styleOverrides = getPointStyleOverrides(datum, seriesIdentifier, styleAccessor);
const style = buildPointGeometryStyles(color, pointStyle, styleOverrides);
const isPointIsolated = allowIsolated && isIsolatedPoint(dataIndex, dataSeries.data.length, yDefined, prev, next);
const isolatedPointStyle = buildPointGeometryStyles(color, isolatedPointThemeStyle);
const styleOverrides = getPointStyleOverrides(datum, seriesIdentifier, isPointIsolated, styleAccessor);
const style = buildPointGeometryStyles(color, pointStyle, styleOverrides);
const isolatedPointStyle = buildPointGeometryStyles(color, isolatedPointThemeStyle, styleOverrides);
// if radius is defined with the mark, limit the minimum radius to the theme radius value
const radius = isPointIsolated
? isolatedPointThemeStyle.radius
Expand Down Expand Up @@ -134,9 +134,10 @@ export function renderPoints(
export function getPointStyleOverrides(
datum: DataSeriesDatum,
seriesIdentifier: XYChartSeriesIdentifier,
isolatedPoint: boolean,
pointStyleAccessor?: PointStyleAccessor,
): Partial<PointStyle> | undefined {
const styleOverride = pointStyleAccessor && pointStyleAccessor(datum, seriesIdentifier);
const styleOverride = pointStyleAccessor && pointStyleAccessor(datum, seriesIdentifier, isolatedPoint);

if (!styleOverride) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,28 +323,28 @@ describe('Rendering utils', () => {
});

it('should return undefined if no pointStyleAccessor is passed', () => {
const styleOverrides = getPointStyleOverrides(datum, seriesIdentifier);
const styleOverrides = getPointStyleOverrides(datum, seriesIdentifier, false);

expect(styleOverrides).toBeUndefined();
});

it('should return undefined if pointStyleAccessor returns null', () => {
mockAccessor.mockReturnValue(null);
const styleOverrides = getPointStyleOverrides(datum, seriesIdentifier, mockAccessor);
const styleOverrides = getPointStyleOverrides(datum, seriesIdentifier, false, mockAccessor);

expect(styleOverrides).toBeUndefined();
});

it('should call pointStyleAccessor with datum and seriesIdentifier', () => {
getPointStyleOverrides(datum, seriesIdentifier, mockAccessor);
it('should call pointStyleAccessor with datum, seriesIdentifier and isolatedPoint', () => {
getPointStyleOverrides(datum, seriesIdentifier, true, mockAccessor);

expect(mockAccessor).toHaveBeenCalledWith(datum, seriesIdentifier);
expect(mockAccessor).toHaveBeenCalledWith(datum, seriesIdentifier, true);
});

it('should return seriesStyle with updated stroke color', () => {
const stroke = 'blue';
mockAccessor.mockReturnValue(stroke);
const styleOverrides = getPointStyleOverrides(datum, seriesIdentifier, mockAccessor);
const styleOverrides = getPointStyleOverrides(datum, seriesIdentifier, false, mockAccessor);
const expectedStyles: Partial<PointStyle> = {
stroke,
};
Expand Down
1 change: 1 addition & 0 deletions packages/charts/src/chart_types/xy_chart/utils/specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export type BarStyleAccessor = (datum: DataSeriesDatum, seriesIdentifier: XYChar
export type PointStyleAccessor = (
datum: DataSeriesDatum,
seriesIdentifier: XYChartSeriesIdentifier,
isolatedPoint: boolean,
) => PointStyleOverride;

/**
Expand Down
8 changes: 4 additions & 4 deletions storybook/stories/line/12_isolated_data_points.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,19 @@ export const Example: ChartsStory = (_, { title, description }) => {
max: 10,
step: 1,
});
const radius = number('isolated point radius', 2, {
const overrideRadius = number('override point radius', 0, {
range: true,
min: 0,
max: 10,
step: 1,
});
const overrideRadius = number('override point radius', 0, {
const overridePointRadius = boolean('override radius for isolated points', false);
const radius = number('isolated point radius', 2, {
range: true,
min: 0,
max: 10,
step: 1,
});
const overridePointRadius = boolean('override radius for isolated points', false);

return (
<Chart title={title} description={description}>
Expand Down Expand Up @@ -77,7 +77,7 @@ export const Example: ChartsStory = (_, { title, description }) => {
id="x"
position={Position.Bottom}
style={{
tickLine: { size: 0.0001, padding: 4 },
tickLine: { size: 0, padding: 4 },
tickLabel: {
alignment: { horizontal: Position.Left, vertical: Position.Bottom },
padding: 0,
Expand Down
138 changes: 138 additions & 0 deletions storybook/stories/test_cases/13_point_style_overrides.story.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { boolean, color } from '@storybook/addon-knobs';
import React from 'react';

import { Axis, Chart, Position, ScaleType, Settings } from '@elastic/charts';
import { KIBANA_METRICS } from '@elastic/charts/src/utils/data_samples/test_dataset_kibana';

import { ChartsStory } from '../../types';
import { useBaseTheme } from '../../use_base_theme';
import { customKnobs } from '../utils/knobs';

const defaultPointStyles = {
radius: 4,
strokeWidth: 2,
};

const defaultlineStyles = {
stroke: 'black',
};

export const Example: ChartsStory = (_, { title, description }) => {
const [Series] = customKnobs.enum.xySeries('series type', 'line', { exclude: ['bar', 'bubble'] });
const themeLevelStroke = color('point.stroke - theme level', 'red');
const themeLevelStrokeIso = color('isolatedPoint.stroke - theme level', 'green');
const useSeriesOverrides = boolean('use series overrides', false);
const seriesLevelStroke = color('point.stroke - series level', 'blue');
const useSeriesIsoOverrides = boolean('use series iso overrides', false);
const seriesLevelStrokeIso = color('isolatedPoint.stroke - series level', 'orange');
const usePointStyleOverrides = boolean('use point style overrides', false);
const pointStyleStroke = color('stroke - pointStyleAccessor', 'black');

return (
<Chart title={title} description={description}>
<Settings
legendPosition={Position.Right}
theme={{
areaSeriesStyle: {
line: defaultlineStyles,
point: {
visible: true,
stroke: themeLevelStroke,
...defaultPointStyles,
},
isolatedPoint: {
stroke: themeLevelStrokeIso,
...defaultPointStyles,
},
},
lineSeriesStyle: {
line: defaultlineStyles,
point: {
visible: true,
stroke: themeLevelStroke,
...defaultPointStyles,
},
isolatedPoint: {
stroke: themeLevelStrokeIso,
...defaultPointStyles,
},
},
}}
baseTheme={useBaseTheme()}
/>
<Axis
id="x"
position={Position.Bottom}
style={{
tickLine: { size: 0, padding: 4 },
tickLabel: {
alignment: { horizontal: Position.Left, vertical: Position.Bottom },
padding: 0,
offset: { x: 0, y: 0 },
},
}}
timeAxisLayerCount={2}
gridLine={{
visible: true,
}}
/>
<Axis id="y" position={Position.Left} ticks={5} />

<Series
id="Count of records"
xScaleType={ScaleType.Time}
yScaleType={ScaleType.Linear}
xAccessor={0}
yAccessors={[1]}
splitSeriesAccessors={[2]}
pointStyleAccessor={
!usePointStyleOverrides
? undefined
: () => ({
stroke: pointStyleStroke,
})
}
areaSeriesStyle={{
...(useSeriesOverrides && {
point: {
stroke: seriesLevelStroke,
},
}),
...(useSeriesIsoOverrides && {
isolatedPoint: {
stroke: seriesLevelStrokeIso,
},
}),
}}
lineSeriesStyle={{
...(useSeriesOverrides && {
point: {
stroke: seriesLevelStroke,
},
}),
...(useSeriesIsoOverrides && {
isolatedPoint: {
stroke: seriesLevelStrokeIso,
},
}),
}}
data={[
...KIBANA_METRICS.metrics.kibana_os_load.v1.data.slice(0, 40).map((d, i) => {
if ([1, 10, 12, 20, 22, 24, 28].includes(i)) {
return [d[0], null, 'A'];
}
return [...d, 'A'];
}),
]}
/>
</Chart>
);
};
1 change: 1 addition & 0 deletions storybook/stories/test_cases/test_cases.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ export { Example as domainEdges } from './21_domain_edges.story';
export { Example as metricTrendLength } from './22_metric_trend_length.story';
export { Example as startDayOfWeek } from './11_start_day_of_week.story';
export { Example as logWithNegativeValues } from './12_log_with_negative_values.story';
export { Example as pointStyleOverrides } from './13_point_style_overrides.story';