pnp / sp-dev-fx-controls-react

Reusable React controls for SPFx solutions
https://pnp.github.io/sp-dev-fx-controls-react/
MIT License
383 stars 378 forks source link

Chart not updating on re-render #435

Open Holden15 opened 4 years ago

Holden15 commented 4 years ago

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 { 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.

ghost commented 4 years ago

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

saddy31 commented 4 years ago

Is there an ETA for resolving this issue?

michaelmaillot commented 1 year ago

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.

larocheti commented 7 months ago

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);