From e7dbb1a0ca1d9404ffb1cacdf6d7fe0168e28b70 Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Fri, 21 Jun 2019 17:31:12 +0200 Subject: [PATCH] Use getDerivedState for Chart (and kick the can...) (#1320) * Use getDerivedState for Chart (and kick the can...) * Update Doughnut.tsx * Update HorizBar.tsx --- packages/ui-app/src/Chart/Doughnut.tsx | 5 ++ packages/ui-app/src/Chart/HorizBar.tsx | 87 +++++++++++++++----------- 2 files changed, 57 insertions(+), 35 deletions(-) diff --git a/packages/ui-app/src/Chart/Doughnut.tsx b/packages/ui-app/src/Chart/Doughnut.tsx index 153b3d51ee..1ebdb01910 100644 --- a/packages/ui-app/src/Chart/Doughnut.tsx +++ b/packages/ui-app/src/Chart/Doughnut.tsx @@ -33,6 +33,11 @@ export default class ChartDoughnut extends React.PureComponent { render () { const { className, size = 100, style, values } = this.props; + // FIXME Classic case of kicking the can down the road, i.e. don't expend energy + // when stuff are not used. This was replaced by the HorizBar as the only Chart + // in actual use (by Referendum). However the below is not optimal, and gets re- + // calculated on each render. If this component is put back in use, look at + // getDerivedStateFromProps in HorizBar (the logic is the same for chartData) const options = values.reduce((options, { colors: [normalColor = '#00f', hoverColor], label, value }) => { options.colorNormal.push(normalColor); options.colorHover.push(hoverColor || normalColor); diff --git a/packages/ui-app/src/Chart/HorizBar.tsx b/packages/ui-app/src/Chart/HorizBar.tsx index 46f632667c..4fb54a41e6 100644 --- a/packages/ui-app/src/Chart/HorizBar.tsx +++ b/packages/ui-app/src/Chart/HorizBar.tsx @@ -21,33 +21,64 @@ type Props = BareProps & { values: Array }; -type Options = { - colorNormal: Array, - colorHover: Array, - data: Array, - labels: Array +type State = { + chartData?: ChartJs.ChartData, + chartOptions?: ChartJs.ChartOptions, + valuesStr?: string }; const alphaColor = (hexColor: string): string => ChartJs.helpers.color(hexColor).alpha(0.65).rgbString(); -export default class ChartHorizBar extends React.PureComponent { - render () { - const { aspectRatio = 4, className, style, values } = this.props; +export default class ChartHorizBar extends React.PureComponent { + state: State = {}; - const options = values.reduce((options, { colors: [normalColor = '#00f', hoverColor], label, value }) => { - options.colorNormal.push(alphaColor(normalColor)); - options.colorHover.push(alphaColor(hoverColor || normalColor)); - options.data.push(bnToBn(value).toNumber()); - options.labels.push(label); + static getDerivedStateFromProps ({ aspectRatio = 4, values }: Props, prevState: State) { + const valuesStr = JSON.stringify(values); - return options; + if (valuesStr === prevState.valuesStr) { + return null; + } + + const chartData = values.reduce((data, { colors: [normalColor = '#00f', hoverColor], label, value }) => { + const dataset = data.datasets[0]; + + dataset.backgroundColor.push(alphaColor(normalColor)); + dataset.hoverBackgroundColor.push(alphaColor(hoverColor || normalColor)); + dataset.data.push(bnToBn(value).toNumber()); + data.labels.push(label); + + return data; }, { - colorNormal: [], - colorHover: [], - data: [], - labels: [] - } as Options); + labels: [] as Array, + datasets: [{ + data: [] as Array, + backgroundColor: [] as Array, + hoverBackgroundColor: [] as Array + }] + }); + + return { + chartData, + chartOptions: { + // width/height by default this is "1", i.e. a square box + aspectRatio, + // no need for the legend, expect the labels contain everything + legend: { + display: false + } + }, + valuesStr + }; + } + + render () { + const { className, style } = this.props; + const { chartData, chartOptions } = this.state; + + if (!chartData) { + return null; + } return (
{ style={style} >
);