Skip to content

Commit 86ac72b

Browse files
authored
LG-4770: Fix duplicate tooltip in remounted chart groups (#2635)
* Cleanup group on unmount * Changeset * Fix and add to tests
1 parent 8d924f6 commit 86ac72b

File tree

4 files changed

+59
-2
lines changed

4 files changed

+59
-2
lines changed

Diff for: .changeset/fuzzy-moons-sneeze.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@lg-charts/core': patch
3+
---
4+
5+
Fixes duplicate tooltip bug in remounted chart groups

Diff for: charts/core/src/Chart/hooks/useChart.spec.ts

+46
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ jest.mock('../../Echart', () => ({
1313
useEchart: jest.fn(() => ({
1414
ready: false,
1515
addToGroup: jest.fn(),
16+
removeFromGroup: jest.fn(),
1617
setupZoomSelect: jest.fn(),
1718
on: jest.fn(),
1819
})),
@@ -32,6 +33,7 @@ describe('@lg-echarts/core/hooks/useChart', () => {
3233
(useEchart as jest.Mock).mockReturnValue({
3334
ready: false,
3435
addToGroup: jest.fn(),
36+
removeFromGroup: jest.fn(),
3537
setupZoomSelect: jest.fn(),
3638
on: jest.fn(),
3739
});
@@ -46,6 +48,7 @@ describe('@lg-echarts/core/hooks/useChart', () => {
4648
(useEchart as jest.Mock).mockReturnValue({
4749
ready: true,
4850
addToGroup: jest.fn(),
51+
removeFromGroup: jest.fn(),
4952
setupZoomSelect: jest.fn(),
5053
on: jest.fn(),
5154
});
@@ -62,6 +65,7 @@ describe('@lg-echarts/core/hooks/useChart', () => {
6265
(useEchart as jest.Mock).mockReturnValue({
6366
ready: true,
6467
addToGroup: jest.fn(),
68+
removeFromGroup: jest.fn(),
6569
setupZoomSelect,
6670
on: jest.fn(),
6771
});
@@ -87,6 +91,7 @@ describe('@lg-echarts/core/hooks/useChart', () => {
8791
(useEchart as jest.Mock).mockReturnValue({
8892
ready: true,
8993
addToGroup: jest.fn(),
94+
removeFromGroup: jest.fn(),
9095
setupZoomSelect: jest.fn(),
9196
on,
9297
});
@@ -113,6 +118,7 @@ describe('@lg-echarts/core/hooks/useChart', () => {
113118
const mockEchartInstance = {
114119
ready: true,
115120
addToGroup: jest.fn(),
121+
removeFromGroup: jest.fn(),
116122
setupZoomSelect: jest.fn(),
117123
on: jest.fn(),
118124
};
@@ -126,4 +132,44 @@ describe('@lg-echarts/core/hooks/useChart', () => {
126132
ref: expect.any(Function),
127133
});
128134
});
135+
136+
test('should call `addToGroup` when `groupId` is present', async () => {
137+
const { useEchart } = require('../../Echart');
138+
const addToGroup = jest.fn();
139+
140+
(useEchart as jest.Mock).mockReturnValue({
141+
ready: true,
142+
addToGroup,
143+
removeFromGroup: jest.fn(),
144+
setupZoomSelect: jest.fn(),
145+
on: jest.fn(),
146+
});
147+
148+
const groupId = 'test-group';
149+
150+
renderHook(() => useChart({ theme: 'dark', groupId }));
151+
152+
expect(addToGroup).toHaveBeenCalledWith(groupId);
153+
});
154+
155+
test('should call `removeFromGroup` on unmount if `groupId` is present', async () => {
156+
const { useEchart } = require('../../Echart');
157+
const removeFromGroup = jest.fn();
158+
159+
(useEchart as jest.Mock).mockReturnValue({
160+
ready: true,
161+
addToGroup: jest.fn(),
162+
removeFromGroup,
163+
setupZoomSelect: jest.fn(),
164+
on: jest.fn(),
165+
});
166+
167+
const groupId = 'test-group';
168+
169+
const { unmount } = renderHook(() => useChart({ theme: 'dark', groupId }));
170+
171+
unmount();
172+
173+
expect(removeFromGroup).toHaveBeenCalled();
174+
});
129175
});

Diff for: charts/core/src/Chart/hooks/useChart.ts

+4
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ export function useChart({
4242
if (groupId) {
4343
echart.addToGroup(groupId);
4444
}
45+
46+
return () => {
47+
echart.removeFromGroup();
48+
};
4549
}
4650
}, [echart.ready, groupId]);
4751

Diff for: charts/core/src/Echart/useEchart.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,10 @@ export function useEchart({
104104
withInstanceCheck((groupId: string) => {
105105
// echartsCoreRef.current should exist if instance does, but checking for extra safety
106106
if (echartsCoreRef.current) {
107-
(echartsInstance as EChartsType).group = groupId;
108-
echartsCoreRef.current.connect(groupId);
107+
if ((echartsInstance as EChartsType).group !== groupId) {
108+
(echartsInstance as EChartsType).group = groupId;
109+
echartsCoreRef.current.connect(groupId);
110+
}
109111
}
110112
}),
111113
[echartsCoreRef.current, echartsInstance],

0 commit comments

Comments
 (0)