sp-dev-fx-controls-react icon indicating copy to clipboard operation
sp-dev-fx-controls-react copied to clipboard

Chart not updating on re-render

Open Holden15 opened this issue 5 years ago • 4 comments

Thank you for reporting an issue, suggesting an enhancement, or asking a question. We appreciate your feedback - to help the team understand your needs please complete the below template to ensure we have the details to help. Thanks!

Please check out the documentation to see if your question is already addressed there. This will help us ensure our documentation is up to date.

Category

[ ] Enhancement

[X] Bug

[ ] Question

Version

Please specify what version of the library you are using: [1.15.0]

` public render(): React.ReactElement<ISafetyChartsProps> { let regionDropdownOptions: IDropdownOption[] = [{key: kAll, text: kAll}];

	for (const regionLabel of kRegionLabels) {
		regionDropdownOptions.push({key: regionLabel, text: regionLabel});
	}

	console.log("SafetyCharts | render | this.state.region: ", this.state.region);

	return (
		<div>
			<Stack styles={{ root: {width: 500 } }}>
				{/* <Stack.Item >
					{this._createIncidentReportChart(this.state.incidentReports, this.state.workHours, this.state.year)}
				</Stack.Item> */}
				<Stack.Item>
					<Dropdown
						label="Region"
						options={regionDropdownOptions}
						selectedKey={this.state.region}
						onChange={this._onChangeRegion}
					/>
				</Stack.Item>
				<Stack.Item>
					{console.log("chart being created")}
					{this._createRegionOSHACountChartForRegion(this.state.region, this.state.incidentReports)}
					{console.log("chart created")}
				</Stack.Item>
			</Stack>
		</div>
	);
}

@boundMethod
private _onChangeRegion(event: React.FormEvent<HTMLDivElement>, option?: IDropdownOption, index?: number): void {
	console.log("SafetyCharts | _conChangeRegion | option: ", option);
	console.log("SafetyCharts | _conChangeRegion | option.text: ", option.text);

	this.setState({
		region: option.text,
	});
}


private _createRegionCountChartForRegion(region: string, incidentReports: IIncidentReport[]): React.ReactNode {
	console.log("Charts | _createRegionCountChartForRegion| region: ", region);
	console.log("Charts | _createRegionCountChartForRegion| incidentReports: ", incidentReports);

	let branchTotalsList: {[key: string]: IIncidentReport[]} = {};

	if (incidentReports && region) {
		for (let incidentReport of incidentReports) {
			if (region === incidentReport.Region) {
				if (branchTotalsList[incidentReport.Branch]) {
					branchTotalsList[incidentReport.Branch].push(incidentReport);
				}
				else {
					branchTotalsList[incidentReport.Branch] = [incidentReport];
				}
			}
		}	

		let branchesData: number[] = [];

		for (let branchName in branchTotalsList) {
			branchesData.push(branchTotalsList[branchName].length);
		}

		console.log("Charts | _createRegionCountChartForRegion| branchesData: ", branchesData);

		let chartDatasets2: Chart.ChartDataSets = {
			label: "Count",
			data: branchesData,
		};

		let chartData2: Chart.ChartData = {
			labels: Object.keys(branchTotalsList),
			datasets: [chartDatasets2]
		};

		console.log("Charts | _createRegionCountChartForRegion| returning chart");

		return (
			<ChartControl
				type={ChartType.Bar}
				data={chartData2}
				options={{}}
			/>
		);
	}
	else {
		console.log("Charts | _createRegionCountChartForRegion| returning empty div");

		return <div></div>;
	}
}

`

When I use the dropdown to change the region, it calls the method _createRegionCountChartForRegion as expected, the data all updates as expected:

` Charts | _conChangeRegion | option: {key: "North Central", text: "North Central"} Charts .tsx:102 Charts | _conChangeRegion | option.text: North Central

Charts .tsx:52 Charts | render | this.state.region: North Central

Charts .tsx:81 test bla

Charts .tsx:178 Charts | _createRegionCountChartForRegion| region: North Central Charts .tsx:179 Charts | _createRegionCountChartForRegion| incidentReports: (11) [{…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}]

Charts .tsx:195 Charts | _createRegionCountChartForRegion| branchTotalsList: {Albertville, MN: Array(1), Green Bay, WI: Array(1), Kansas City, MO: Array(1)}

Charts .tsx:203 Charts | _createRegionCountChartForRegion| branchesData: (3) [1, 1, 1] Charts .tsx:215 Charts | _createRegionCountChartForRegion| returning chart

Charts .tsx:83 chart created

`

But doesn't show the chart correctly:

image

If I select any region in the dropdown, then the chart will show the data for North Central:

image

But the data for South East is logged:

Charts | _conChangeRegion | option: {key: "South East", text: "South East"} Charts .tsx:102 Charts | _conChangeRegion | option.text: South East Charts .tsx:52 Charts | render | this.state.region: South East Charts .tsx:81 test bla Charts .tsx:178 Charts | _createRegionCountChartForRegion| region: South East Charts .tsx:179 Charts | _createRegionCountChartForRegion | incidentReports: (11) [{…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}] Charts .tsx:195 Charts | _createRegionCountChartForRegion| branchTotalsList: {Charleston, SC: Array(1), Jackson, TN: Array(1)} Charts .tsx:203 Charts | _createRegionCountChartForRegion| branchesData: (2) [1, 1] Charts .tsx:215 Charts | _createRegionCountChartForRegion| returning chart Charts .tsx:83 chart created

It feels like the data is updated, the chart is created, but never rendered. On the next change, the previous chart is rendered.

What might I be doing wrong?

EDIT: It renders correctly when the webpart first loads. I set region to "South West" and the chart displays as expected.

P.S. Sorry for the formatting, the code brackets are giving me fits.

Holden15 avatar Dec 11 '19 22:12 Holden15

Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible.

ghost avatar Dec 11 '19 22:12 ghost

Is there an ETA for resolving this issue?

saddy31 avatar May 21 '20 18:05 saddy31

Hi @Holden15 & @saddy31,

I've tested with latest version and confirm the bug. I've located the glitch.

Seems that during the re-rendering, the component uses current props instead of new ones, which explains why there's always a delay with expected data.

Let's see if someone wants to handle this one otherwise, I'll deal with it when I'll be able to.

michaelmaillot avatar Sep 07 '23 23:09 michaelmaillot

Ok, I believe I found what the problem is.

The props.data is set correctly the first time, in the componentDidMount method, so the chart renders correctly. However, this method, as any React components, is called only the first time the component is rendered. If there is a re-render, the method called (in the case of this ChartControl component anyway) is UNSAFE_componentWillReceiveProps (see here).

If you look at the body of this method, at line 85, it uses this.props.data to re-init the chart (after it was destroyed). It should be using nextProps.data. I changed this on my end and it now refreshes properly when the view is not fully destroyed but only updated.

So to be clear, to fix this, line 85 should now look like this:

this._initChart(nextProps, nextProps.data);

larocheti avatar Feb 07 '24 16:02 larocheti