Skip to content

refactor(Charts - New): memoize chartConfig objects #480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 5, 2020
Merged

refactor(Charts - New): memoize chartConfig objects #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f9cf1c0
memoize bar chartConfig object
MarcusNotheis May 5, 2020
c312225
memoize donut chart props
MarcusNotheis May 5, 2020
751da8b
use static prop for tooltip
MarcusNotheis May 5, 2020
c8412d4
memoize column chart chartConfig
MarcusNotheis May 5, 2020
0445659
memoize composed chart chartConfig
MarcusNotheis May 5, 2020
9508087
memoize line chart chartConfig
MarcusNotheis May 5, 2020
3654f45
Update MicroBarChart.tsx
MarcusNotheis May 5, 2020
ca10630
memoize pie charts chartConfig
MarcusNotheis May 5, 2020
ca38b6e
Update RadarChart.tsx
MarcusNotheis May 5, 2020
f5c419f
Update RadialChart.tsx
MarcusNotheis May 5, 2020
5b60031
Update RadialChart.tsx
MarcusNotheis May 5, 2020
25197fe
Update PieChart.tsx
MarcusNotheis May 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 23 additions & 24 deletions packages/charts/src/components/BarChart/BarChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ import { ThemingParameters } from '@ui5/webcomponents-react-base/lib/ThemingPara
import { useConsolidatedRef } from '@ui5/webcomponents-react-base/lib/useConsolidatedRef';
import { enrichEventWithDetails } from '@ui5/webcomponents-react-base/lib/Utils';
import { BarChartPlaceholder } from '@ui5/webcomponents-react-charts/lib/BarChartPlaceholder';
import { ChartDataLabel } from '@ui5/webcomponents-react-charts/lib/components/ChartDataLabel';
import { XAxisTicks } from '@ui5/webcomponents-react-charts/lib/components/XAxisTicks';
import { YAxisTicks } from '@ui5/webcomponents-react-charts/lib/components/YAxisTicks';
import { ChartContainer } from '@ui5/webcomponents-react-charts/lib/next/ChartContainer';
import { useLegendItemClick } from '@ui5/webcomponents-react-charts/lib/useLegendItemClick';
import React, { FC, forwardRef, Ref, useCallback } from 'react';
import React, { FC, forwardRef, Ref, useCallback, useMemo } from 'react';
import {
Bar,
BarChart as BarChartLib,
Expand All @@ -22,10 +25,7 @@ import { useTooltipFormatter } from '../../hooks/useTooltipFormatter';
import { IChartDimension } from '../../interfaces/IChartDimension';
import { IChartMeasure } from '../../interfaces/IChartMeasure';
import { RechartBaseProps } from '../../interfaces/RechartBaseProps';
import { ChartDataLabel } from '@ui5/webcomponents-react-charts/lib/components/ChartDataLabel';
import { XAxisTicks } from '@ui5/webcomponents-react-charts/lib/components/XAxisTicks';
import { YAxisTicks } from '@ui5/webcomponents-react-charts/lib/components/YAxisTicks';
import { tickLineConfig, tooltipContentStyle } from '../../internal/staticProps';
import { tickLineConfig, tooltipContentStyle, tooltipFillOpacity } from '../../internal/staticProps';

const formatYAxisTicks = (tick = '') => {
const splitTick = tick.split(' ');
Expand Down Expand Up @@ -106,7 +106,14 @@ const BarChart: FC<BarChartProps> = forwardRef((props: BarChartProps, ref: Ref<a
noLegend = false,
onDataPointClick,
onLegendClick,
chartConfig = {
style,
className,
tooltip,
slot
} = props;

const chartConfig = useMemo(() => {
return {
margin: {},
yAxisVisible: true,
xAxisVisible: true,
Expand All @@ -116,17 +123,9 @@ const BarChart: FC<BarChartProps> = forwardRef((props: BarChartProps, ref: Ref<a
legendPosition: 'top',
barGap: 3,
zoomingTool: false,
referenceLine: {
label: undefined,
value: undefined,
color: undefined
}
},
style,
className,
tooltip,
slot
} = props;
...props.chartConfig
};
}, [props.chartConfig]);

const { dimensions, measures } = usePrepareDimensionsAndMeasures(
props.dimensions,
Expand Down Expand Up @@ -187,21 +186,21 @@ const BarChart: FC<BarChartProps> = forwardRef((props: BarChartProps, ref: Ref<a
>
<BarChartLib margin={marginChart} layout="vertical" data={dataset} barGap={chartConfig.barGap}>
<CartesianGrid
vertical={chartConfig.gridVertical ?? false}
vertical={chartConfig.gridVertical}
horizontal={chartConfig.gridHorizontal}
stroke={chartConfig.gridStroke ?? ThemingParameters.sapList_BorderColor}
stroke={chartConfig.gridStroke}
/>
{(chartConfig.xAxisVisible ?? true) && (
{chartConfig.xAxisVisible && (
<XAxis
interval={0}
type="number"
tick={<XAxisTicks config={primaryMeasure} chartRef={chartRef} />}
axisLine={chartConfig.xAxisVisible ?? false}
axisLine={chartConfig.xAxisVisible}
tickLine={tickLineConfig}
tickFormatter={primaryMeasure?.formatter}
/>
)}
{(chartConfig.yAxisVisible ?? true) &&
{(chartConfig.yAxisVisible) &&
dimensions.map((dimension, index) => {
return (
<YAxis
Expand Down Expand Up @@ -235,15 +234,15 @@ const BarChart: FC<BarChartProps> = forwardRef((props: BarChartProps, ref: Ref<a
/>
);
})}
{!noLegend && <Legend verticalAlign={chartConfig.legendPosition ?? 'top'} onClick={onItemLegendClick} />}
{!noLegend && <Legend verticalAlign={chartConfig.legendPosition} onClick={onItemLegendClick} />}
{chartConfig.referenceLine && (
<ReferenceLine
stroke={chartConfig.referenceLine.color}
x={chartConfig.referenceLine.value}
label={chartConfig.referenceLine.label}
/>
)}
<Tooltip cursor={{ fillOpacity: 0.3 }} formatter={tooltipValueFormatter} contentStyle={tooltipContentStyle} />
<Tooltip cursor={tooltipFillOpacity} formatter={tooltipValueFormatter} contentStyle={tooltipContentStyle} />
{chartConfig.zoomingTool && (
<Brush
y={10}
Expand Down
52 changes: 22 additions & 30 deletions packages/charts/src/components/ColumnChart/ColumnChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { XAxisTicks } from '@ui5/webcomponents-react-charts/lib/components/XAxis
import { YAxisTicks } from '@ui5/webcomponents-react-charts/lib/components/YAxisTicks';
import { ChartContainer } from '@ui5/webcomponents-react-charts/lib/next/ChartContainer';
import { useLegendItemClick } from '@ui5/webcomponents-react-charts/lib/useLegendItemClick';
import React, { FC, forwardRef, Ref, useCallback } from 'react';
import React, { FC, forwardRef, Ref, useCallback, useMemo } from 'react';
import {
Bar as Column,
BarChart as ColumnChartLib,
Expand All @@ -25,7 +25,7 @@ import { useTooltipFormatter } from '../../hooks/useTooltipFormatter';
import { IChartDimension } from '../../interfaces/IChartDimension';
import { IChartMeasure } from '../../interfaces/IChartMeasure';
import { RechartBaseProps } from '../../interfaces/RechartBaseProps';
import { tickLineConfig, tooltipContentStyle } from '../../internal/staticProps';
import { tickLineConfig, tooltipContentStyle, tooltipFillOpacity } from '../../internal/staticProps';

interface MeasureConfig extends IChartMeasure {
/**
Expand Down Expand Up @@ -95,33 +95,25 @@ const ColumnChart: FC<ColumnChartProps> = forwardRef((props: ColumnChartProps, r
noLegend = false,
onDataPointClick,
onLegendClick,
chartConfig = {
margin: {},
style,
className,
tooltip,
slot
} = props;

const chartConfig = useMemo(() => {
return {
yAxisVisible: false,
xAxisVisible: true,
gridStroke: ThemingParameters.sapList_BorderColor,
gridHorizontal: true,
gridVertical: false,
yAxisColor: ThemingParameters.sapList_BorderColor,
legendPosition: 'top',
barGap: 3,
zoomingTool: false,
secondYAxis: {
dataKey: undefined,
name: undefined,
color: undefined
},
referenceLine: {
label: undefined,
value: undefined,
color: undefined
}
},
style,
className,
tooltip,
slot
} = props;
...props.chartConfig
};
}, [props.chartConfig]);

const { dimensions, measures } = usePrepareDimensionsAndMeasures(
props.dimensions,
Expand All @@ -139,7 +131,7 @@ const ColumnChart: FC<ColumnChartProps> = forwardRef((props: ColumnChartProps, r

const dataKeys = measures.map(({ accessor }) => accessor);
const colorSecondY = chartConfig.secondYAxis
? dataKeys.findIndex((key) => key === chartConfig.secondYAxis.dataKey)
? dataKeys.findIndex((key) => key === chartConfig.secondYAxis?.dataKey)
: 0;

const onItemLegendClick = useLegendItemClick(onLegendClick);
Expand Down Expand Up @@ -189,11 +181,11 @@ const ColumnChart: FC<ColumnChartProps> = forwardRef((props: ColumnChartProps, r
>
<ColumnChartLib margin={marginChart} data={dataset} barGap={chartConfig.barGap}>
<CartesianGrid
vertical={chartConfig.gridVertical ?? false}
vertical={chartConfig.gridVertical}
horizontal={chartConfig.gridHorizontal}
stroke={chartConfig.gridStroke ?? ThemingParameters.sapList_BorderColor}
stroke={chartConfig.gridStroke}
/>
{(chartConfig.xAxisVisible ?? true) &&
{chartConfig.xAxisVisible &&
dimensions.map((dimension, index) => {
return (
<XAxis
Expand All @@ -208,13 +200,13 @@ const ColumnChart: FC<ColumnChartProps> = forwardRef((props: ColumnChartProps, r
);
})}
<YAxis
axisLine={chartConfig.yAxisVisible ?? false}
axisLine={chartConfig.yAxisVisible}
tickLine={tickLineConfig}
yAxisId="left"
interval={0}
tick={<YAxisTicks config={primaryMeasure} />}
/>
{chartConfig.secondYAxis && chartConfig.secondYAxis.dataKey && (
{chartConfig.secondYAxis?.dataKey && (
<YAxis
dataKey={chartConfig.secondYAxis.dataKey}
stroke={chartConfig.secondYAxis.color ?? `var(--sapChart_OrderedColor_${(colorSecondY % 11) + 1})`}
Expand All @@ -227,7 +219,7 @@ const ColumnChart: FC<ColumnChartProps> = forwardRef((props: ColumnChartProps, r
{measures.map((element, index) => {
return (
<Column
yAxisId={chartConfig?.secondYAxis?.dataKey === element.accessor ? 'right' : 'left'}
yAxisId={chartConfig.secondYAxis?.dataKey === element.accessor ? 'right' : 'left'}
stackId={element.stackId}
fillOpacity={element.opacity}
key={element.accessor}
Expand All @@ -243,7 +235,7 @@ const ColumnChart: FC<ColumnChartProps> = forwardRef((props: ColumnChartProps, r
/>
);
})}
{!noLegend && <Legend verticalAlign={chartConfig.legendPosition ?? 'top'} onClick={onItemLegendClick} />}
{!noLegend && <Legend verticalAlign={chartConfig.legendPosition} onClick={onItemLegendClick} />}
{chartConfig.referenceLine && (
<ReferenceLine
stroke={chartConfig.referenceLine.color}
Expand All @@ -252,7 +244,7 @@ const ColumnChart: FC<ColumnChartProps> = forwardRef((props: ColumnChartProps, r
yAxisId={'left'}
/>
)}
<Tooltip cursor={{ fillOpacity: 0.3 }} formatter={tooltipValueFormatter} contentStyle={tooltipContentStyle} />
<Tooltip cursor={tooltipFillOpacity} formatter={tooltipValueFormatter} contentStyle={tooltipContentStyle} />
{chartConfig.zoomingTool && (
<Brush
y={10}
Expand Down
64 changes: 26 additions & 38 deletions packages/charts/src/components/ComposedChart/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { XAxisTicks } from '@ui5/webcomponents-react-charts/lib/components/XAxis
import { YAxisTicks } from '@ui5/webcomponents-react-charts/lib/components/YAxisTicks';
import { ChartContainer } from '@ui5/webcomponents-react-charts/lib/next/ChartContainer';
import { useLegendItemClick } from '@ui5/webcomponents-react-charts/lib/useLegendItemClick';
import React, { FC, forwardRef, Ref, useCallback, useState } from 'react';
import React, { FC, forwardRef, Ref, useCallback, useState, useMemo } from 'react';
import {
Area,
Bar,
Expand All @@ -28,7 +28,7 @@ import { useTooltipFormatter } from '../../hooks/useTooltipFormatter';
import { IChartDimension } from '../../interfaces/IChartDimension';
import { IChartMeasure } from '../../interfaces/IChartMeasure';
import { RechartBaseProps } from '../../interfaces/RechartBaseProps';
import { tickLineConfig, tooltipContentStyle } from '../../internal/staticProps';
import { tickLineConfig, tooltipContentStyle, tooltipFillOpacity } from '../../internal/staticProps';

const dimensionDefaults = {
formatter: (d) => d
Expand Down Expand Up @@ -116,29 +116,6 @@ const ComposedChart: FC<ComposedChartProps> = forwardRef((props: ComposedChartPr
noLegend = false,
onLegendClick,
layout = 'horizontal',
chartConfig = {
margin: {},
yAxisVisible: false,
xAxisVisible: true,
gridStroke: ThemingParameters.sapList_BorderColor,
gridHorizontal: true,
gridVertical: false,
yAxisId: '',
yAxisColor: ThemingParameters.sapList_BorderColor,
legendPosition: 'top',
zoomingTool: false,
barGap: undefined,
referenceLine: {
label: undefined,
value: undefined,
color: undefined
},
secondYAxis: {
name: undefined,
dataKey: undefined,
color: undefined
}
},
style,
className,
tooltip,
Expand All @@ -150,6 +127,19 @@ const ComposedChart: FC<ComposedChartProps> = forwardRef((props: ComposedChartPr
dataset.some(({ type }) => type === 'bar') ? BAR_DEFAULT_PADDING : 0
);

const chartConfig = useMemo(() => {
return {
yAxisVisible: false,
xAxisVisible: true,
gridStroke: ThemingParameters.sapList_BorderColor,
gridHorizontal: true,
gridVertical: false,
legendPosition: 'top',
zoomingTool: false,
...props.chartConfig
};
}, [props.chartConfig]);

const { dimensions, measures } = usePrepareDimensionsAndMeasures(
props.dimensions,
props.measures,
Expand All @@ -164,7 +154,7 @@ const ComposedChart: FC<ComposedChartProps> = forwardRef((props: ComposedChartPr

const dataKeys = measures.map(({ accessor }) => accessor);
const colorSecondY = chartConfig.secondYAxis
? dataKeys.findIndex((key) => key === chartConfig.secondYAxis.dataKey)
? dataKeys.findIndex((key) => key === chartConfig.secondYAxis?.dataKey)
: 0;

const onDataPointClickInternal = useCallback(
Expand Down Expand Up @@ -217,7 +207,7 @@ const ComposedChart: FC<ComposedChartProps> = forwardRef((props: ComposedChartPr
);

const measureAxisProps = {
axisLine: chartConfig.yAxisVisible ?? false,
axisLine: chartConfig.yAxisVisible,
tickLine: tickLineConfig,
tickFormatter: primaryMeasure?.formatter,
interval: 0
Expand Down Expand Up @@ -259,11 +249,11 @@ const ComposedChart: FC<ComposedChartProps> = forwardRef((props: ComposedChartPr
>
<ComposedChartLib margin={marginChart} data={dataset} layout={layout}>
<CartesianGrid
vertical={chartConfig.gridVertical ?? false}
vertical={chartConfig.gridVertical}
horizontal={chartConfig.gridHorizontal}
stroke={chartConfig.gridStroke ?? ThemingParameters.sapList_BorderColor}
stroke={chartConfig.gridStroke}
/>
{(chartConfig.xAxisVisible ?? true) &&
{chartConfig.xAxisVisible &&
dimensions.map((dimension, index) => {
let AxisComponent;
const axisProps = {
Expand Down Expand Up @@ -302,7 +292,7 @@ const ComposedChart: FC<ComposedChartProps> = forwardRef((props: ComposedChartPr
/>
)}

{chartConfig.secondYAxis && chartConfig.secondYAxis.dataKey && layout === 'horizontal' && (
{chartConfig.secondYAxis?.dataKey && layout === 'horizontal' && (
<YAxis
dataKey={chartConfig.secondYAxis.dataKey}
stroke={chartConfig.secondYAxis.color ?? `var(--sapChart_OrderedColor_${(colorSecondY % 11) + 1})`}
Expand All @@ -312,7 +302,7 @@ const ComposedChart: FC<ComposedChartProps> = forwardRef((props: ComposedChartPr
yAxisId="secondary"
/>
)}
{chartConfig.secondYAxis && chartConfig.secondYAxis.dataKey && layout === 'vertical' && (
{chartConfig.secondYAxis?.dataKey && layout === 'vertical' && (
<XAxis
dataKey={chartConfig.secondYAxis.dataKey}
stroke={chartConfig.secondYAxis.color ?? `var(--sapChart_OrderedColor_${(colorSecondY % 11) + 1})`}
Expand All @@ -333,8 +323,8 @@ const ComposedChart: FC<ComposedChartProps> = forwardRef((props: ComposedChartPr
xAxisId={layout === 'vertical' ? 'primary' : undefined}
/>
)}
<Tooltip cursor={{ fillOpacity: 0.3 }} formatter={tooltipValueFormatter} contentStyle={tooltipContentStyle} />
{!noLegend && <Legend verticalAlign={chartConfig.legendPosition ?? 'top'} onClick={onItemLegendClick} />}
<Tooltip cursor={tooltipFillOpacity} formatter={tooltipValueFormatter} contentStyle={tooltipContentStyle} />
{!noLegend && <Legend verticalAlign={chartConfig.legendPosition} onClick={onItemLegendClick} />}
{measures?.map((element, index) => {
const ChartElement = (ChartTypes[element.type] as any) as FC<any>;

Expand Down Expand Up @@ -375,11 +365,9 @@ const ComposedChart: FC<ComposedChartProps> = forwardRef((props: ComposedChartPr
}

if (layout === 'vertical') {
chartElementProps.xAxisId =
chartConfig?.secondYAxis?.dataKey === element.accessor ? 'secondary' : 'primary';
chartElementProps.xAxisId = chartConfig.secondYAxis?.dataKey === element.accessor ? 'secondary' : 'primary';
} else {
chartElementProps.yAxisId =
chartConfig?.secondYAxis?.dataKey === element.accessor ? 'secondary' : 'primary';
chartElementProps.yAxisId = chartConfig.secondYAxis?.dataKey === element.accessor ? 'secondary' : 'primary';
}
return (
<ChartElement
Expand Down
Loading