Skip to content

Commit 62ddea8

Browse files
LG-5588 Refactor chart by adding Series abstraction to Line data series (#3205)
* conf: add jetbrains project defs to gitignore * refactor: introduce Series as superclass of Line this will create room for the follow PR that introduces Bar series # Conflicts: # charts/core/src/Chart.stories.tsx * doc: add changeset * refactor: use better typing and interface for Series * refactor: rename stylingOptions to options to encompass all series options
1 parent 772c55d commit 62ddea8

File tree

18 files changed

+183
-92
lines changed

18 files changed

+183
-92
lines changed

.changeset/quick-needles-burn.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@lg-charts/core': patch
3+
'@lg-charts/drag-provider': patch
4+
---
5+
6+
introduce `Series` abstraction as a superclass of `Line` this allows supporting more diverse series
7+
types such as `Bar` in a follow up PR

.gitignore

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
# IDE files
2+
.idea
3+
*.iml
4+
15
# Locally-installed dependencies
26
node_modules/
37

charts/core/src/Chart.stories.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { StoryObj } from '@storybook/react';
55
import { ChartProps } from './Chart/Chart.types';
66
import { ChartHeaderProps } from './ChartHeader/ChartHeader.types';
77
import { ChartTooltipProps } from './ChartTooltip/ChartTooltip.types';
8-
import { LineProps } from './Line';
8+
import { LineProps } from './Series';
99
import { makeLineData } from './testUtils';
1010
import { ThresholdLineProps } from './ThresholdLine';
1111
import {

charts/core/src/Echart/Echart.types.ts

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import type { XAXisComponentOption, YAXisComponentOption } from 'echarts';
2-
import type { LineSeriesOption } from 'echarts/charts';
32
import type {
43
DatasetComponentOption,
54
GridComponentOption,
@@ -9,16 +8,45 @@ import type {
98
TooltipComponentOption,
109
} from 'echarts/components';
1110
import type { ComposeOption, EChartsType } from 'echarts/core';
11+
import {
12+
BarSeriesOption,
13+
LineSeriesOption,
14+
SeriesOption,
15+
} from 'echarts/types/dist/shared';
1216

13-
import { Theme } from '@leafygreen-ui/lib';
17+
import { Theme, ValuesOf } from '@leafygreen-ui/lib';
1418

1519
// Type not exported by echarts.
1620
// reference: https://github.com/apache/echarts/blob/master/src/coord/axisCommonTypes.ts#L193
1721
export type AxisLabelValueFormatter = (value: number, index?: number) => string;
1822

19-
type RequiredSeriesProps = 'type' | 'name' | 'data';
20-
export type EChartSeriesOption = Pick<LineSeriesOption, RequiredSeriesProps> &
21-
Partial<Omit<LineSeriesOption, RequiredSeriesProps>>;
23+
export interface StylingContext {
24+
seriesColor?: string;
25+
}
26+
27+
// to convert an SeriesOption type of echarts into a structured form more aligned with LeafyGreen design standards,
28+
// where the 'type', 'name', and 'data' fields are explicitly required and typed,
29+
// and all additional series properties encapsulated within the 'options' object
30+
interface DisciplinedSeriesOption<EChartType extends SeriesOption> {
31+
type: NonNullable<EChartType['type']>;
32+
name: string;
33+
data: NonNullable<EChartType['data']>;
34+
options: Omit<EChartType, 'type' | 'name' | 'data'>;
35+
}
36+
37+
// all supported series options types disciplined and grouped into a single interface
38+
export interface EChartSeriesOptions {
39+
line: DisciplinedSeriesOption<LineSeriesOption>;
40+
// TODO: to be leveraged in a follow-up PR to add Bar chart support
41+
bar: DisciplinedSeriesOption<BarSeriesOption>;
42+
}
43+
44+
// a disciplined substitute for SeriesOption type of echarts limited to the ones supported here
45+
export type EChartSeriesOption = Omit<
46+
ValuesOf<EChartSeriesOptions>,
47+
'options'
48+
> &
49+
ValuesOf<EChartSeriesOptions>['options'];
2250

2351
/**
2452
* TODO: This might need to be improved. `ComposeOption` appears to make most base option
@@ -108,6 +136,7 @@ interface EChartsEventHandlerType {
108136
callback: (params: any) => void,
109137
options?: Partial<{ useCanvasAsTrigger: boolean }>,
110138
): void;
139+
111140
(
112141
event: 'zoomselect',
113142
callback: (params: EChartZoomSelectionEvent) => void,

charts/core/src/Echart/utils/updateUtils.spec.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,44 @@
1-
import { EChartOptions } from '../Echart.types';
1+
import { EChartOptions, EChartSeriesOption } from '../Echart.types';
22

33
import { addSeries, removeSeries, updateOptions } from './updateUtils';
44

55
describe('@lg-charts/core/Chart/hooks/updateUtils', () => {
66
test('addSeries should add a series to the chart options', () => {
77
const currentOptions: Partial<EChartOptions> = {
8-
series: [{ name: 'series1' }],
8+
series: [{ type: 'line', name: 'series1', data: [] }],
99
};
1010
const newSeriesName = 'series2';
11-
const data = { name: newSeriesName };
11+
const data: EChartSeriesOption = {
12+
type: 'line',
13+
name: newSeriesName,
14+
data: [],
15+
};
1216
const updatedOptions = addSeries(currentOptions, data);
1317
expect(updatedOptions.series).toHaveLength(2);
1418
expect(updatedOptions.series?.[1].name).toBe(newSeriesName);
1519
});
1620

1721
test('addSeries should not add a series if a chart with the same name exists', () => {
1822
const currentOptions: Partial<EChartOptions> = {
19-
series: [{ name: 'series1' }],
23+
series: [{ type: 'line', name: 'series1', data: [] }],
2024
};
2125
const newSeriesName = 'series1';
22-
const data = { name: newSeriesName };
26+
const data: EChartSeriesOption = {
27+
type: 'line',
28+
name: newSeriesName,
29+
data: [],
30+
};
2331
const updatedOptions = addSeries(currentOptions, data);
2432
expect(updatedOptions.series).toHaveLength(1);
2533
expect(updatedOptions.series?.[0].name).toBe(newSeriesName);
2634
});
2735

2836
test('removeSeries should remove a series from the chart options', () => {
2937
const currentOptions: Partial<EChartOptions> = {
30-
series: [{ name: 'series1' }, { name: 'series2' }],
38+
series: [
39+
{ type: 'line', name: 'series1', data: [] },
40+
{ type: 'line', name: 'series2', data: [] },
41+
],
3142
};
3243
const seriesName1 = 'series1';
3344
const seriesName2 = 'series2';

charts/core/src/EventMarkers/BaseEventMarker/utils.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ export function getMarkConfig({
109109
symbolSize: [16, 16],
110110
symbolRotate: 360, // Icon shows upside down without this
111111
},
112+
data: [],
112113
} as SeriesOption;
113114
} else {
114115
return {
@@ -130,6 +131,7 @@ export function getMarkConfig({
130131
symbolSize: [16, 16],
131132
symbol: generateSymbolDataUri(level, theme),
132133
},
134+
data: [],
133135
} as SeriesOption;
134136
}
135137
}

charts/core/src/Line/Line.tsx

Lines changed: 0 additions & 54 deletions
This file was deleted.

charts/core/src/Line/config/defaultLineOptions.ts

Lines changed: 0 additions & 20 deletions
This file was deleted.

charts/core/src/Line/config/index.ts

Lines changed: 0 additions & 1 deletion
This file was deleted.

charts/core/src/Line/index.ts

Lines changed: 0 additions & 2 deletions
This file was deleted.

0 commit comments

Comments
 (0)