Skip to content

Commit 84f7776

Browse files
Lukas742Lukas742
committed
fix onClick event properties of BarChart, ColumnChart, ComposedChart
1 parent 9b2e929 commit 84f7776

File tree

11 files changed

+159
-42
lines changed

11 files changed

+159
-42
lines changed

‎packages/charts/src/components/BarChart/BarChart.tsx‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { enrichEventWithDetails, ThemingParameters, useIsRTL, useSyncRef } from '@ui5/webcomponents-react-base';
44
import type { CSSProperties } from 'react';
5-
import { forwardRef, useCallback } from 'react';
5+
import { useRef, forwardRef, useCallback } from 'react';
66
import {
77
Bar,
88
BarChart as BarChartLib,
@@ -26,7 +26,7 @@ import { useObserveXAxisHeights } from '../../hooks/useObserveXAxisHeights.js';
2626
import { useOnClickInternal } from '../../hooks/useOnClickInternal.js';
2727
import { usePrepareDimensionsAndMeasures } from '../../hooks/usePrepareDimensionsAndMeasures.js';
2828
import { useTooltipFormatter } from '../../hooks/useTooltipFormatter.js';
29-
import type { IChartBaseProps } from '../../interfaces/IChartBaseProps.js';
29+
import type { ActivePayload, IChartBaseProps } from '../../interfaces/IChartBaseProps.js';
3030
import type { IChartDimension } from '../../interfaces/IChartDimension.js';
3131
import type { IChartMeasure } from '../../interfaces/IChartMeasure.js';
3232
import { ChartContainer } from '../../internal/ChartContainer.js';
@@ -158,7 +158,6 @@ const BarChart = forwardRef<HTMLDivElement, BarChartProps>((props, ref) => {
158158
...props.chartConfig,
159159
};
160160
const referenceLine = chartConfig.referenceLine;
161-
162161
const { dimensions, measures } = usePrepareDimensionsAndMeasures(
163162
props.dimensions,
164163
props.measures,
@@ -180,7 +179,7 @@ const BarChart = forwardRef<HTMLDivElement, BarChartProps>((props, ref) => {
180179
: 0;
181180

182181
const [componentRef, chartRef] = useSyncRef<any>(ref);
183-
182+
const activePayloadsRef = useRef<ActivePayload[]>(measures);
184183
const onItemLegendClick = useLegendItemClick(onLegendClick);
185184
const labelFormatter = useLabelFormatter(primaryDimension);
186185

@@ -203,7 +202,7 @@ const BarChart = forwardRef<HTMLDivElement, BarChartProps>((props, ref) => {
203202
[onDataPointClick],
204203
);
205204

206-
const onClickInternal = useOnClickInternal(onClick);
205+
const onClickInternal = useOnClickInternal(onClick, dataset, activePayloadsRef);
207206

208207
const isBigDataSet = dataset?.length > 30;
209208
const primaryDimensionAccessor = primaryDimension?.accessor;
@@ -307,17 +306,28 @@ const BarChart = forwardRef<HTMLDivElement, BarChartProps>((props, ref) => {
307306
})}
308307
{isMounted &&
309308
measures.map((element, index) => {
309+
const color = element.color ?? `var(--sapChart_OrderedColor_${(index % 12) + 1})`;
310+
const dataKey = element.accessor;
311+
const name = element.label ?? element.accessor;
312+
const opacity = element.opacity ?? 1;
313+
activePayloadsRef.current[index].color = color;
314+
activePayloadsRef.current[index].stroke = color;
315+
activePayloadsRef.current[index].dataKey = dataKey;
316+
activePayloadsRef.current[index].hide = element.hide;
317+
activePayloadsRef.current[index].name = name;
318+
activePayloadsRef.current[index].fillOpacity = opacity;
319+
activePayloadsRef.current[index].strokeOpacity = opacity;
310320
return (
311321
<Bar
322+
key={element.reactKey}
323+
fill={color}
324+
stroke={color}
312325
stackId={element.stackId}
326+
name={name}
313327
fillOpacity={element.opacity}
314-
key={element.reactKey}
315-
name={element.label ?? element.accessor}
316328
strokeOpacity={element.opacity}
317329
type="monotone"
318-
dataKey={element.accessor}
319-
fill={element.color ?? `var(--sapChart_OrderedColor_${(index % 12) + 1})`}
320-
stroke={element.color ?? `var(--sapChart_OrderedColor_${(index % 12) + 1})`}
330+
dataKey={dataKey}
321331
barSize={element.width}
322332
onClick={onDataPointClickInternal}
323333
isAnimationActive={!noAnimation}

‎packages/charts/src/components/BulletChart/BulletChart.tsx‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { enrichEventWithDetails, ThemingParameters, useIsRTL, useSyncRef } from '@ui5/webcomponents-react-base';
44
import type { CSSProperties } from 'react';
5-
import { forwardRef, useCallback, useMemo } from 'react';
5+
import { useRef, forwardRef, useCallback, useMemo } from 'react';
66
import {
77
Bar,
88
Brush,
@@ -24,7 +24,7 @@ import { useObserveXAxisHeights } from '../../hooks/useObserveXAxisHeights.js';
2424
import { useOnClickInternal } from '../../hooks/useOnClickInternal.js';
2525
import { usePrepareDimensionsAndMeasures } from '../../hooks/usePrepareDimensionsAndMeasures.js';
2626
import { useTooltipFormatter } from '../../hooks/useTooltipFormatter.js';
27-
import type { IChartBaseProps } from '../../interfaces/IChartBaseProps.js';
27+
import type { ActivePayload, IChartBaseProps } from '../../interfaces/IChartBaseProps.js';
2828
import type { IChartDimension } from '../../interfaces/IChartDimension.js';
2929
import type { IChartMeasure } from '../../interfaces/IChartMeasure.js';
3030
import { ChartContainer } from '../../internal/ChartContainer.js';
@@ -172,6 +172,7 @@ const BulletChart = forwardRef<HTMLDivElement, BulletChartProps>((props, ref) =>
172172
dimensionDefaults,
173173
measureDefaults,
174174
);
175+
const activePayloadsRef = useRef<ActivePayload[]>(measures);
175176

176177
const sortedMeasures = useMemo(() => {
177178
return measures.sort((measure) => {
@@ -239,7 +240,8 @@ const BulletChart = forwardRef<HTMLDivElement, BulletChartProps>((props, ref) =>
239240
);
240241

241242
const onItemLegendClick = useLegendItemClick(onLegendClick);
242-
const onClickInternal = useOnClickInternal(onClick);
243+
//todo: implement activePayloadsRef
244+
const onClickInternal = useOnClickInternal(onClick, dataset, activePayloadsRef);
243245

244246
const isBigDataSet = dataset?.length > 30;
245247
const primaryDimensionAccessor = primaryDimension?.accessor;

‎packages/charts/src/components/ColumnChart/ColumnChart.tsx‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { enrichEventWithDetails, ThemingParameters, useIsRTL, useSyncRef } from '@ui5/webcomponents-react-base';
44
import type { CSSProperties } from 'react';
5-
import { forwardRef, useCallback } from 'react';
5+
import { useRef, forwardRef, useCallback } from 'react';
66
import {
77
Bar as Column,
88
BarChart as ColumnChartLib,
@@ -26,7 +26,7 @@ import { useObserveXAxisHeights } from '../../hooks/useObserveXAxisHeights.js';
2626
import { useOnClickInternal } from '../../hooks/useOnClickInternal.js';
2727
import { usePrepareDimensionsAndMeasures } from '../../hooks/usePrepareDimensionsAndMeasures.js';
2828
import { useTooltipFormatter } from '../../hooks/useTooltipFormatter.js';
29-
import type { IChartBaseProps } from '../../interfaces/IChartBaseProps.js';
29+
import type { ActivePayload, IChartBaseProps } from '../../interfaces/IChartBaseProps.js';
3030
import type { IChartDimension } from '../../interfaces/IChartDimension.js';
3131
import type { IChartMeasure } from '../../interfaces/IChartMeasure.js';
3232
import { ChartContainer } from '../../internal/ChartContainer.js';
@@ -175,6 +175,7 @@ const ColumnChart = forwardRef<HTMLDivElement, ColumnChartProps>((props, ref) =>
175175

176176
const labelFormatter = useLabelFormatter(primaryDimension);
177177
const [componentRef, chartRef] = useSyncRef<any>(ref);
178+
const activePayloadsRef = useRef<ActivePayload[]>(measures);
178179

179180
const dataKeys = measures.map(({ accessor }) => accessor);
180181
const colorSecondY = chartConfig.secondYAxis
@@ -203,7 +204,7 @@ const ColumnChart = forwardRef<HTMLDivElement, ColumnChartProps>((props, ref) =>
203204
[onDataPointClick],
204205
);
205206

206-
const onClickInternal = useOnClickInternal(onClick);
207+
const onClickInternal = useOnClickInternal(onClick, dataset, activePayloadsRef);
207208

208209
const isBigDataSet = dataset?.length > 30;
209210
const primaryDimensionAccessor = primaryDimension?.accessor;
@@ -301,19 +302,30 @@ const ColumnChart = forwardRef<HTMLDivElement, ColumnChartProps>((props, ref) =>
301302
)}
302303
{isMounted &&
303304
measures.map((element, index) => {
305+
const color = element.color ?? `var(--sapChart_OrderedColor_${(index % 12) + 1})`;
306+
const dataKey = element.accessor;
307+
const name = element.label ?? element.accessor;
308+
const opacity = element.opacity ?? 1;
309+
activePayloadsRef.current[index].color = color;
310+
activePayloadsRef.current[index].stroke = color;
311+
activePayloadsRef.current[index].dataKey = dataKey;
312+
activePayloadsRef.current[index].hide = element.hide;
313+
activePayloadsRef.current[index].name = name;
314+
activePayloadsRef.current[index].fillOpacity = opacity;
315+
activePayloadsRef.current[index].strokeOpacity = opacity;
304316
return (
305317
<Column
306-
// todo: multiple `yAxisId`s cause the Cartesian Grid to break
307-
yAxisId={chartConfig.secondYAxis?.dataKey === element.accessor ? 'right' : 'left'}
318+
key={element.reactKey}
319+
fill={color}
320+
stroke={color}
308321
stackId={element.stackId}
322+
name={name}
309323
fillOpacity={element.opacity}
310-
key={element.reactKey}
311-
name={element.label ?? element.accessor}
312324
strokeOpacity={element.opacity}
313325
type="monotone"
314-
dataKey={element.accessor}
315-
fill={element.color ?? `var(--sapChart_OrderedColor_${(index % 12) + 1})`}
316-
stroke={element.color ?? `var(--sapChart_OrderedColor_${(index % 12) + 1})`}
326+
dataKey={dataKey}
327+
// todo: multiple `yAxisId`s cause the Cartesian Grid to break
328+
yAxisId={chartConfig.secondYAxis?.dataKey === element.accessor ? 'right' : 'left'}
317329
barSize={element.width}
318330
onClick={onDataPointClickInternal}
319331
isAnimationActive={!noAnimation}

‎packages/charts/src/components/ComposedChart/ComposedChart.cy.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,13 @@ describe('ComposedChart', () => {
5151
measures={measures}
5252
onClick={onClick}
5353
onLegendClick={onLegendClick}
54+
noAnimation
5455
/>,
5556
);
5657

5758
cy.findByText('January').click();
5859
cy.get('@onClick').should('have.been.called');
59-
cy.get('[name="January"]').eq(0).click();
60+
cy.get('[name="January"]').eq(0).realClick({ position: 'topLeft' });
6061
cy.get('@onClick')
6162
.should('have.been.calledTwice')
6263
.and(
@@ -68,6 +69,7 @@ describe('ComposedChart', () => {
6869
}),
6970
);
7071

72+
//todo: onLegendClick is never fired, check why
7173
cy.contains('Users').click();
7274
cy.get('@onLegendClick').should(
7375
'have.been.calledWith',

‎packages/charts/src/components/ComposedChart/index.tsx‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { enrichEventWithDetails, ThemingParameters, useIsRTL, useSyncRef } from '@ui5/webcomponents-react-base';
44
import type { CSSProperties, FC } from 'react';
5-
import { forwardRef, useCallback } from 'react';
5+
import { useRef, forwardRef, useCallback } from 'react';
66
import {
77
Area,
88
Bar,
@@ -27,7 +27,7 @@ import { useObserveXAxisHeights } from '../../hooks/useObserveXAxisHeights.js';
2727
import { useOnClickInternal } from '../../hooks/useOnClickInternal.js';
2828
import { usePrepareDimensionsAndMeasures } from '../../hooks/usePrepareDimensionsAndMeasures.js';
2929
import { useTooltipFormatter } from '../../hooks/useTooltipFormatter.js';
30-
import type { IChartBaseProps } from '../../interfaces/IChartBaseProps.js';
30+
import type { ActivePayload, IChartBaseProps } from '../../interfaces/IChartBaseProps.js';
3131
import type { IChartDimension } from '../../interfaces/IChartDimension.js';
3232
import type { IChartMeasure } from '../../interfaces/IChartMeasure.js';
3333
import { ChartContainer } from '../../internal/ChartContainer.js';
@@ -188,6 +188,7 @@ const ComposedChart = forwardRef<HTMLDivElement, ComposedChartProps>((props, ref
188188
measureDefaults,
189189
);
190190

191+
const activePayloadsRef = useRef<ActivePayload[]>(measures);
191192
const tooltipValueFormatter = useTooltipFormatter(measures);
192193

193194
const primaryDimension = dimensions[0];
@@ -238,7 +239,7 @@ const ComposedChart = forwardRef<HTMLDivElement, ComposedChartProps>((props, ref
238239
};
239240

240241
const onItemLegendClick = useLegendItemClick(onLegendClick);
241-
const onClickInternal = useOnClickInternal(onClick);
242+
const onClickInternal = useOnClickInternal(onClick, dataset, activePayloadsRef);
242243

243244
const isBigDataSet = dataset?.length > 30;
244245
const primaryDimensionAccessor = primaryDimension?.accessor;
@@ -438,11 +439,22 @@ const ComposedChart = forwardRef<HTMLDivElement, ComposedChartProps>((props, ref
438439
)}
439440
{measures?.map((element, index) => {
440441
const ChartElement = ChartTypes[element.type] as any as FC<any>;
441-
442442
const chartElementProps: any = {
443443
isAnimationActive: !noAnimation,
444444
};
445445
let labelPosition = 'top';
446+
const color = element.color ?? `var(--sapChart_OrderedColor_${(index % 12) + 1})`;
447+
const dataKey = element.accessor;
448+
const name = element.label ?? element.accessor;
449+
const opacity = element.opacity ?? 1;
450+
const hide = element.hide;
451+
activePayloadsRef.current[index].color = color;
452+
activePayloadsRef.current[index].stroke = color;
453+
activePayloadsRef.current[index].dataKey = dataKey;
454+
activePayloadsRef.current[index].hide = hide;
455+
activePayloadsRef.current[index].name = name;
456+
activePayloadsRef.current[index].fillOpacity = opacity;
457+
activePayloadsRef.current[index].strokeOpacity = opacity;
446458

447459
switch (element.type) {
448460
case 'line':
@@ -452,9 +464,12 @@ const ComposedChart = forwardRef<HTMLDivElement, ComposedChartProps>((props, ref
452464
chartElementProps.strokeWidth = element.width;
453465
chartElementProps.strokeOpacity = element.opacity;
454466
chartElementProps.dot = element.showDot ?? !isBigDataSet;
467+
468+
activePayloadsRef.current[index].fillOpacity = opacity;
469+
activePayloadsRef.current[index].strokeOpacity = opacity;
455470
break;
456471
case 'bar':
457-
chartElementProps.hide = element.hide;
472+
chartElementProps.hide = hide;
458473
chartElementProps.fillOpacity = element.opacity;
459474
chartElementProps.strokeOpacity = element.opacity;
460475
chartElementProps.barSize = element.width;
@@ -475,6 +490,9 @@ const ComposedChart = forwardRef<HTMLDivElement, ComposedChartProps>((props, ref
475490
chartElementProps.activeDot = {
476491
onClick: onDataPointClickInternal,
477492
};
493+
494+
activePayloadsRef.current[index].fillOpacity = 0.3;
495+
activePayloadsRef.current[index].strokeOpacity = opacity;
478496
break;
479497
}
480498

@@ -486,22 +504,22 @@ const ComposedChart = forwardRef<HTMLDivElement, ComposedChartProps>((props, ref
486504
return (
487505
<ChartElement
488506
key={element.reactKey}
489-
name={element.label ?? element.accessor}
507+
name={name}
490508
label={
491509
element.type === 'bar' || isBigDataSet ? undefined : (
492510
<ChartDataLabel config={element} chartType={element.type} position={labelPosition} />
493511
)
494512
}
495-
stroke={element.color ?? `var(--sapChart_OrderedColor_${(index % 12) + 1})`}
496-
fill={element.color ?? `var(--sapChart_OrderedColor_${(index % 12) + 1})`}
513+
stroke={color}
514+
fill={color}
497515
type="monotone"
498-
dataKey={element.accessor}
516+
dataKey={dataKey}
499517
{...chartElementProps}
500518
>
501519
{element.type === 'bar' && (
502520
<>
503521
<LabelList
504-
dataKey={element.accessor}
522+
dataKey={dataKey}
505523
content={<ChartDataLabel config={element} chartType="column" position={'insideTop'} />}
506524
/>
507525
{dataset.map((data, i) => {

‎packages/charts/src/components/LineChart/LineChart.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,8 @@ const LineChart = forwardRef<HTMLDivElement, LineChartProps>((props, ref) => {
180180

181181
const onItemLegendClick = useLegendItemClick(onLegendClick);
182182
const preventOnClickCall = useRef(0);
183+
184+
//todo: check this
183185
const onDataPointClickInternal = useCallback(
184186
(payload, eventOrIndex) => {
185187
if (eventOrIndex.dataKey && typeof onDataPointClick === 'function') {

‎packages/charts/src/components/RadarChart/RadarChart.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,10 @@ const RadarChart = forwardRef<HTMLDivElement, RadarChartProps>((props, ref) => {
125125

126126
const onItemLegendClick = useLegendItemClick(onLegendClick);
127127
const preventOnClickCall = useRef(false);
128+
//todo: check this
128129
const onClickInternal = useCallback(
129130
(payload, event) => {
131+
console.log(payload);
130132
if (typeof onClick === 'function' && !preventOnClickCall.current) {
131133
onClick(
132134
enrichEventWithDetails(event, {

‎packages/charts/src/components/RadialChart/RadialChart.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export interface RadialChartProps
5757
*/
5858
onClick?: (
5959
event: CustomEvent<{
60+
// todo: map new values to old ones to prevent breaking changes
6061
payload: unknown;
6162
activePayloads: Record<string, unknown>[];
6263
dataIndex: number;

‎packages/charts/src/components/ScatterChart/ScatterChart.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,10 @@ const ScatterChart = forwardRef<HTMLDivElement, ScatterChartProps>((props, ref)
168168
const [componentRef, chartRef] = useSyncRef<any>(ref);
169169
const preventOnClickCall = useRef(false);
170170
const onItemLegendClick = useLegendItemClick(onLegendClick);
171+
//todo: error on click
171172
const onClickInternal = useCallback(
172173
(payload, event) => {
174+
console.log(payload);
173175
if (typeof onClick === 'function' && !preventOnClickCall.current) {
174176
onClick(
175177
enrichEventWithDetails(event, {

0 commit comments

Comments
 (0)