reactjs / react-chartjs

common react charting components using chart.js
MIT License
2.93k stars 301 forks source link

Update() does not work with Radar Chart #80

Closed phil-lauffenburger closed 8 years ago

phil-lauffenburger commented 8 years ago

I am passing in data in the render() portion of my react component to a radarchart

<div className = "container-fluid">
     <RadarChart data={results} options={chartOptions}  width="600" height="250"/>
        </div>

When the component re-renders, I get the following error: Uncaught TypeError: Cannot read property 'length' of undefined

originating from this codeline:

 while (chart.scale.xLabels.length > nextProps.data.labels.length) {

If I change my radar chart to a bar chart it works just fine. Is there something I'm missing here?

Thanks!

blizzo521 commented 8 years ago

+1 on this, I am having the same problem. Radar chart is the whole reason i'm using this library so if I can't get it to work I will have to replace it :(

pardhaponugoti commented 8 years ago

I passed in redraw as a prop and it fixed the issue for me. It scraps the old chart and makes a whole new one every time though. For example, your code would look like this:

`

`

I saw it at the bottom of this page:

https://github.com/jhudson8/react-chartjs

phil-lauffenburger commented 8 years ago

Yeah, that works for me as well. I just don't like how redrawing the chart looks.

rossPatton commented 8 years ago

adding to this. redraw fixes the issue for me as well, but it shouldn't really be needed

cryptic-mystic commented 8 years ago

Hey guys, I was having an issue with my doughnut charts updating (which shares code with radar and circle charts). Old data points would not be removed when the new dataset was of a smaller length than the previous. I've made a PR under #88, if you think it might help your problems we can make an effort to get it merged.

austinpray commented 8 years ago

Can you guys check if https://github.com/jhudson8/react-chartjs/pull/88 fixed your issue?

phil-lauffenburger commented 8 years ago

Ok, so #88 does not fix the issue. #88 just applies to Polar/Pie/Doughnut Charts. However, chart.removeData() seemed promising. I mucked around with it a bit and removed the while loop just below the change in #88.

} else { chart.removeData(); nextProps.data.datasets.forEach(function(set, setIndex) {

This of course causes some other errors but I just added a few extra if statements to classData.componentWillReceiveProps:

if (chart.scale) { chart.scale.xLabels = nextProps.data.labels; if (chart.scale.calculateXLabelRotation){ chart.scale.calculateXLabelRotation(); } }

Now, my radar chart is updating without redrawing. This feels like a bit of a hack, so I want to poke at it a bit more before submitting a pull request.

Any thoughts?

cryptic-mystic commented 8 years ago

@lauffenp Yeah my commit only related to those charts you mentioned. I noticed that some of my charts have been updating slowly with my fix, so if you're able to remove the data without a loop that'll probably give us a speed boost.

phil-lauffenburger commented 8 years ago

92 should fix this.

phil-lauffenburger commented 8 years ago

fixed by #92