diff --git a/.changeset/nice-radios-move.md b/.changeset/nice-radios-move.md new file mode 100644 index 0000000000..eeab95cbf6 --- /dev/null +++ b/.changeset/nice-radios-move.md @@ -0,0 +1,6 @@ +--- +'@lg-charts/core': patch +--- + +- Fixes hiding chart elements when components are removed. +- Fixes adding of duplicate series. diff --git a/charts/core/README.md b/charts/core/README.md index c7940d1925..b518e0d4ec 100644 --- a/charts/core/README.md +++ b/charts/core/README.md @@ -70,10 +70,10 @@ Component that takes in data points and renders a single line on the chart. #### Props -| Name | Description | Type | Default | -| ------ | ---------------------------------------------------------------------- | ----------------------------------------------------------- | ------- | -| `name` | Name used to identify the series. | string | | -| `data` | Data array of tuples that represent x and y coordinates in the series. | Array<[string \| number \| Date, string \| number \| Date]> | | +| Name | Description | Type | Default | +| ------ | ------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------- | ------- | +| `name` | Unique name used to identify the series. **Important note:** If two lines have the same name, only one will be rendered. | string | | +| `data` | Data array of tuples that represent x and y coordinates in the series. | Array<[string \| number \| Date, string \| number \| Date]> | | ### `Grid` diff --git a/charts/core/src/Chart/hooks/updateUtils.spec.ts b/charts/core/src/Chart/hooks/updateUtils.spec.ts index 710bd4c893..e87a86141e 100644 --- a/charts/core/src/Chart/hooks/updateUtils.spec.ts +++ b/charts/core/src/Chart/hooks/updateUtils.spec.ts @@ -3,7 +3,7 @@ import { ChartOptions } from '../Chart.types'; import { addSeries, removeSeries, updateOptions } from './updateUtils'; describe('@lg-charts/core/Chart/hooks/updateUtils', () => { - test('should add a series to the chart options', () => { + test('addSeries should add a series to the chart options', () => { const currentOptions: Partial = { series: [{ name: 'series1' }], }; @@ -14,7 +14,18 @@ describe('@lg-charts/core/Chart/hooks/updateUtils', () => { expect(updatedOptions.series?.[1].name).toBe(newSeriesName); }); - test('should remove a series from the chart options', () => { + test('addSeries should not add a series if a chart with the same name exists', () => { + const currentOptions: Partial = { + series: [{ name: 'series1' }], + }; + const newSeriesName = 'series1'; + const data = { name: newSeriesName }; + const updatedOptions = addSeries(currentOptions, data); + expect(updatedOptions.series).toHaveLength(1); + expect(updatedOptions.series?.[0].name).toBe(newSeriesName); + }); + + test('removeSeries should remove a series from the chart options', () => { const currentOptions: Partial = { series: [{ name: 'series1' }, { name: 'series2' }], }; @@ -28,7 +39,7 @@ describe('@lg-charts/core/Chart/hooks/updateUtils', () => { /** * Tests that option updates don't overwrite the entire chart options object. */ - test('should merge chart options non-destructively', () => { + test('updateOptions should merge chart options non-destructively', () => { const currentOptions: Partial = { xAxis: { show: true, diff --git a/charts/core/src/Chart/hooks/updateUtils.ts b/charts/core/src/Chart/hooks/updateUtils.ts index 829535ddcf..a41dd13d78 100644 --- a/charts/core/src/Chart/hooks/updateUtils.ts +++ b/charts/core/src/Chart/hooks/updateUtils.ts @@ -9,7 +9,9 @@ export function addSeries( if (!updatedOptions.series) { updatedOptions.series = [data]; } else { - updatedOptions.series.push(data); + if (!updatedOptions.series.some(series => series.name === data.name)) { + updatedOptions.series.push(data); + } } return updatedOptions; @@ -22,9 +24,9 @@ export function removeSeries( const updatedOptions = { ...currentOptions }; if (updatedOptions.series) { - updatedOptions.series = updatedOptions.series.filter( - series => series.name !== name, - ); + updatedOptions.series = [ + ...updatedOptions.series.filter(series => series.name !== name), + ]; } return updatedOptions; diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index 5417c60531..c62ab73e8f 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -74,7 +74,12 @@ export function useChart({ theme, onChartReady }: ChartHookProps) { const updateChartRef = useMemo( () => debounce((chartOptions: Partial) => { - chartInstanceRef.current?.setOption(chartOptions); + /** + * The second argument is `true` to merge the new options with the existing ones. + * This is needed to ensure that series get removed properly. + * See issue: https://github.com/apache/echarts/issues/6202 + * */ + chartInstanceRef.current?.setOption(chartOptions, true); }, 50), [], ); diff --git a/charts/core/src/Grid/Grid.tsx b/charts/core/src/Grid/Grid.tsx index 01f80e605a..4dfbcd8de1 100644 --- a/charts/core/src/Grid/Grid.tsx +++ b/charts/core/src/Grid/Grid.tsx @@ -8,6 +8,12 @@ import { useChartContext } from '../ChartContext'; import { GridProps } from './Grid.types'; +const unsetGridOptions = { + splitLine: { + show: false, + }, +}; + export function Grid({ horizontal = true, vertical = true }: GridProps) { const { updateChartOptions } = useChartContext(); const { theme } = useDarkMode(); @@ -26,6 +32,16 @@ export function Grid({ horizontal = true, vertical = true }: GridProps) { updatedOptions.xAxis = getUpdatedLineOptions(!!vertical); updatedOptions.yAxis = getUpdatedLineOptions(!!horizontal); updateChartOptions(updatedOptions); + + return () => { + /** + * Hides the grid lines when the component is unmounted. + */ + updateChartOptions({ + xAxis: unsetGridOptions, + yAxis: unsetGridOptions, + }); + }; }, [horizontal, vertical, theme]); return null; diff --git a/charts/core/src/Line/Line.tsx b/charts/core/src/Line/Line.tsx index 694fb94b0a..91e9d62702 100644 --- a/charts/core/src/Line/Line.tsx +++ b/charts/core/src/Line/Line.tsx @@ -6,7 +6,7 @@ import { defaultLineOptions } from './config'; import { LineProps } from './Line.types'; export function Line({ name, data }: LineProps) { - const { addChartSeries } = useChartContext(); + const { addChartSeries, removeChartSeries } = useChartContext(); useEffect(() => { addChartSeries({ @@ -14,6 +14,14 @@ export function Line({ name, data }: LineProps) { name, data, }); + + return () => { + /** + * Remove the series when the component unmounts to make sure the series + * is removed when a `Line` is hidden. + */ + removeChartSeries(name); + }; }, [name, data]); return null; diff --git a/charts/core/src/XAxis/XAxis.tsx b/charts/core/src/XAxis/XAxis.tsx index ebca501ad3..1ff98d044e 100644 --- a/charts/core/src/XAxis/XAxis.tsx +++ b/charts/core/src/XAxis/XAxis.tsx @@ -77,6 +77,15 @@ const getOptions = ({ return options; }; +const unsetAxisOptions = { + axisLine: { + show: false, + }, + axisLabel: { + show: false, + }, +}; + /** * React component that can render an x-axis on a parent chart. * @@ -97,6 +106,15 @@ export function XAxis({ type, label, unit }: XAxisProps) { useEffect(() => { updateChartOptions(getOptions({ type, label, unit, theme })); + + return () => { + /** + * Hides the axis when the component is unmounted. + */ + updateChartOptions({ + xAxis: unsetAxisOptions, + }); + }; }, [type, label, unit, theme, updateChartOptions]); return null; diff --git a/charts/core/src/YAxis/YAxis.tsx b/charts/core/src/YAxis/YAxis.tsx index dea1e73319..a4e7d629e4 100644 --- a/charts/core/src/YAxis/YAxis.tsx +++ b/charts/core/src/YAxis/YAxis.tsx @@ -76,6 +76,15 @@ const getOptions = ({ return options; }; +const unsetAxisOptions = { + axisLine: { + show: false, + }, + axisLabel: { + show: false, + }, +}; + /** * React component that can render an y-axis on a parent chart. * @@ -96,6 +105,15 @@ export function YAxis({ type, label, unit }: YAxisProps) { useEffect(() => { updateChartOptions(getOptions({ type, label, unit, theme })); + + return () => { + /** + * Hides the axis when the component is unmounted. + */ + updateChartOptions({ + yAxis: unsetAxisOptions, + }); + }; }, [type, label, unit, theme, updateChartOptions]); return null;