Skip to content

Commit e5c719d

Browse files
bert-ebert-e
committed
Merge branch 'improvement/chart-legend-click-behaviour' into q/1.0
2 parents 0e615ab + 9efc6c4 commit e5c719d

File tree

3 files changed

+330
-35
lines changed

3 files changed

+330
-35
lines changed

‎src/lib/components/chartlegend/ChartLegend.test.tsx‎

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
import { render, screen } from '@testing-library/react';
2+
import { ChartLegend } from './ChartLegend';
3+
import React from 'react';
4+
import { ChartLegendWrapper } from './ChartLegendWrapper';
5+
import userEvent from '@testing-library/user-event';
6+
7+
const colorSet = {
8+
CPU: 'red',
9+
Memory: 'blue',
10+
Disk: 'green',
11+
};
12+
13+
describe('ChartLegend', () => {
14+
beforeEach(() => {
15+
jest.clearAllMocks();
16+
});
17+
it('should render all legend items', () => {
18+
render(
19+
<ChartLegendWrapper colorSet={colorSet}>
20+
<ChartLegend shape="line" />
21+
</ChartLegendWrapper>,
22+
);
23+
expect(screen.getByText('CPU')).toBeInTheDocument();
24+
expect(screen.getByText('Memory')).toBeInTheDocument();
25+
expect(screen.getByText('Disk')).toBeInTheDocument();
26+
});
27+
28+
it('should have all items selected by default', () => {
29+
render(
30+
<ChartLegendWrapper colorSet={colorSet}>
31+
<ChartLegend shape="line" />
32+
</ChartLegendWrapper>,
33+
);
34+
35+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
36+
expect(screen.getByLabelText('Memory selected')).toBeInTheDocument();
37+
expect(screen.getByLabelText('Disk selected')).toBeInTheDocument();
38+
});
39+
it('should not respond to clicks when disabled', () => {
40+
render(
41+
<ChartLegendWrapper colorSet={colorSet}>
42+
<ChartLegend shape="line" disabled />
43+
</ChartLegendWrapper>,
44+
);
45+
46+
userEvent.click(screen.getByText('CPU'));
47+
// If disabled, should not select any items
48+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
49+
expect(screen.getByLabelText('Memory selected')).toBeInTheDocument();
50+
expect(screen.getByLabelText('Disk selected')).toBeInTheDocument();
51+
52+
userEvent.click(screen.getByText('Memory'), { metaKey: true });
53+
// If disabled, should not select any items
54+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
55+
expect(screen.getByLabelText('Memory selected')).toBeInTheDocument();
56+
expect(screen.getByLabelText('Disk selected')).toBeInTheDocument();
57+
});
58+
describe('Normal Click Behavior', () => {
59+
it('should select only clicked item when all are selected', () => {
60+
render(
61+
<ChartLegendWrapper colorSet={colorSet}>
62+
<ChartLegend shape="line" />
63+
</ChartLegendWrapper>,
64+
);
65+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
66+
expect(screen.getByLabelText('Memory selected')).toBeInTheDocument();
67+
expect(screen.getByLabelText('Disk selected')).toBeInTheDocument();
68+
69+
// Click on CPU should select only CPU
70+
userEvent.click(screen.getByLabelText('CPU selected'));
71+
72+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
73+
expect(screen.getByLabelText('Memory not selected')).toBeInTheDocument();
74+
expect(screen.getByLabelText('Disk not selected')).toBeInTheDocument();
75+
});
76+
77+
it('should select all items when clicking on the only selected item', () => {
78+
render(
79+
<ChartLegendWrapper colorSet={colorSet}>
80+
<ChartLegend shape="line" />
81+
</ChartLegendWrapper>,
82+
);
83+
84+
// First click to select only CPU
85+
userEvent.click(screen.getByLabelText('CPU selected'));
86+
87+
// Second click should select all
88+
userEvent.click(screen.getByLabelText('CPU selected'));
89+
90+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
91+
expect(screen.getByLabelText('Memory selected')).toBeInTheDocument();
92+
expect(screen.getByLabelText('Disk selected')).toBeInTheDocument();
93+
});
94+
95+
it('should select only clicked item when clicking unselected item', () => {
96+
render(
97+
<ChartLegendWrapper colorSet={colorSet}>
98+
<ChartLegend shape="line" />
99+
</ChartLegendWrapper>,
100+
);
101+
102+
// Select only CPU first
103+
userEvent.click(screen.getByLabelText('CPU selected'));
104+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
105+
expect(screen.getByLabelText('Memory not selected')).toBeInTheDocument();
106+
107+
// Click on unselected Memory should select only Memory
108+
userEvent.click(screen.getByText('Memory'));
109+
expect(screen.getByLabelText('CPU not selected')).toBeInTheDocument();
110+
expect(screen.getByLabelText('Memory selected')).toBeInTheDocument();
111+
expect(screen.getByLabelText('Disk not selected')).toBeInTheDocument();
112+
});
113+
});
114+
115+
describe('Cmd+Click Behavior', () => {
116+
it('should add unselected item to selection when cmd+clicking', () => {
117+
render(
118+
<ChartLegendWrapper colorSet={colorSet}>
119+
<ChartLegend shape="line" />
120+
</ChartLegendWrapper>,
121+
);
122+
123+
// Select only CPU first
124+
userEvent.click(screen.getByText('CPU'));
125+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
126+
expect(screen.getByLabelText('Memory not selected')).toBeInTheDocument();
127+
128+
// Cmd+click Memory should add it to selection
129+
userEvent.click(screen.getByText('Memory'), { metaKey: true });
130+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
131+
expect(screen.getByLabelText('Memory selected')).toBeInTheDocument();
132+
expect(screen.getByLabelText('Disk not selected')).toBeInTheDocument();
133+
});
134+
135+
it('should remove selected item from selection when cmd+clicking with multiple selected', () => {
136+
render(
137+
<ChartLegendWrapper colorSet={colorSet}>
138+
<ChartLegend shape="line" />
139+
</ChartLegendWrapper>,
140+
);
141+
142+
// Start with all selected, select only CPU, then add Memory
143+
userEvent.click(screen.getByText('CPU'));
144+
userEvent.click(screen.getByText('Memory'), { metaKey: true });
145+
146+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
147+
expect(screen.getByLabelText('Memory selected')).toBeInTheDocument();
148+
expect(screen.getByLabelText('Disk not selected')).toBeInTheDocument();
149+
150+
// Cmd+click CPU should remove it
151+
userEvent.click(screen.getByText('CPU'), { metaKey: true });
152+
expect(screen.getByLabelText('CPU not selected')).toBeInTheDocument();
153+
expect(screen.getByLabelText('Memory selected')).toBeInTheDocument();
154+
expect(screen.getByLabelText('Disk not selected')).toBeInTheDocument();
155+
});
156+
157+
it('should select all when cmd+clicking the only selected item', () => {
158+
render(
159+
<ChartLegendWrapper colorSet={colorSet}>
160+
<ChartLegend shape="line" />
161+
</ChartLegendWrapper>,
162+
);
163+
164+
// Select only CPU
165+
userEvent.click(screen.getByText('CPU'));
166+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
167+
expect(screen.getByLabelText('Memory not selected')).toBeInTheDocument();
168+
expect(screen.getByLabelText('Disk not selected')).toBeInTheDocument();
169+
170+
// Cmd+click the only selected item should select all
171+
userEvent.click(screen.getByText('CPU'), { metaKey: true });
172+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
173+
expect(screen.getByLabelText('Memory selected')).toBeInTheDocument();
174+
expect(screen.getByLabelText('Disk selected')).toBeInTheDocument();
175+
});
176+
177+
it('should work with ctrl+click on Windows/Linux', () => {
178+
render(
179+
<ChartLegendWrapper colorSet={colorSet}>
180+
<ChartLegend shape="line" />
181+
</ChartLegendWrapper>,
182+
);
183+
184+
// Select only CPU first
185+
userEvent.click(screen.getByText('CPU'));
186+
187+
// Ctrl+click Memory should add it to selection
188+
userEvent.click(screen.getByText('Memory'), { ctrlKey: true });
189+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
190+
expect(screen.getByLabelText('Memory selected')).toBeInTheDocument();
191+
expect(screen.getByLabelText('Disk not selected')).toBeInTheDocument();
192+
});
193+
194+
it('should handle mixed normal and cmd+clicks correctly', () => {
195+
render(
196+
<ChartLegendWrapper colorSet={colorSet}>
197+
<ChartLegend shape="line" />
198+
</ChartLegendWrapper>,
199+
);
200+
201+
// Normal click to select CPU only
202+
userEvent.click(screen.getByText('CPU'));
203+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
204+
expect(screen.getByLabelText('Memory not selected')).toBeInTheDocument();
205+
206+
// Cmd+click to add Memory
207+
userEvent.click(screen.getByText('Memory'), { metaKey: true });
208+
expect(screen.getByLabelText('CPU selected')).toBeInTheDocument();
209+
expect(screen.getByLabelText('Memory selected')).toBeInTheDocument();
210+
211+
// Normal click on Disk should select only Disk
212+
userEvent.click(screen.getByText('Disk'));
213+
expect(screen.getByLabelText('CPU not selected')).toBeInTheDocument();
214+
expect(screen.getByLabelText('Memory not selected')).toBeInTheDocument();
215+
expect(screen.getByLabelText('Disk selected')).toBeInTheDocument();
216+
});
217+
});
218+
});

‎src/lib/components/chartlegend/ChartLegend.tsx‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,21 +72,51 @@ export const ChartLegend = ({
7272
isSelected,
7373
addSelectedResource,
7474
removeSelectedResource,
75+
selectAllResources,
76+
getAllResourcesCount,
77+
getSelectedCount,
78+
selectOnlyResource,
7579
} = useChartLegend();
7680

7781
const resources = listResources();
7882

7983
const handleLegendClick = useCallback(
80-
(resource: string) => {
84+
(resource: string, event: React.MouseEvent) => {
8185
if (disabled) return;
8286

83-
if (isSelected(resource)) {
84-
removeSelectedResource(resource);
87+
const isModifierClick = event.metaKey || event.ctrlKey;
88+
const itemIsSelected = isSelected(resource);
89+
90+
if (isModifierClick) {
91+
if (itemIsSelected) {
92+
if (getSelectedCount() === 1) {
93+
selectAllResources();
94+
} else {
95+
removeSelectedResource(resource);
96+
}
97+
} else {
98+
addSelectedResource(resource);
99+
}
85100
} else {
86-
addSelectedResource(resource);
101+
if (getSelectedCount() === getAllResourcesCount()) {
102+
selectOnlyResource(resource);
103+
} else if (itemIsSelected) {
104+
selectAllResources();
105+
} else {
106+
selectOnlyResource(resource);
107+
}
87108
}
88109
},
89-
[disabled, isSelected, addSelectedResource, removeSelectedResource],
110+
[
111+
disabled,
112+
isSelected,
113+
addSelectedResource,
114+
removeSelectedResource,
115+
selectAllResources,
116+
selectOnlyResource,
117+
getAllResourcesCount,
118+
getSelectedCount,
119+
],
90120
);
91121

92122
return (
@@ -100,7 +130,8 @@ export const ChartLegend = ({
100130
key={resource}
101131
disabled={disabled}
102132
selected={selected}
103-
onClick={() => handleLegendClick(resource)}
133+
aria-label={`${resource} ${selected ? 'selected' : 'not selected'}`}
134+
onClick={(event) => handleLegendClick(resource, event)}
104135
>
105136
<LegendShape
106137
color={color}

0 commit comments

Comments
 (0)