valor-software / ng2-charts

Beautiful charts for Angular based on Chart.js
http://valor-software.github.io/ng2-charts/
MIT License
2.35k stars 573 forks source link

Chart doesn't redraw / update non data/dataset related settings #806

Closed olivermuc closed 5 years ago

olivermuc commented 7 years ago

Looking at the code I realized that dynamic changes to the data/datasets is indeed supported and will cause the chart to update.

BUT, no other change to a chart is being supported, eg. labels (amount or text), changes to the settings etc. this would require a complete chart redraw (chartJS dependency?) as far as I can tell.

In your code, I see that getChartBuilder() does that, triggered by refresh() which only happens on 2 occasions: ngOnInit() & ngOnChanges() IF not changes.hasOwnProperty('data') || ...

That's quite a bit of limitation right there.

I recommend to add public method to call a complete refresh or better yet, adapt the ngOnChanges() routine to look for new changes to other chart settings and if outside of data/dataset changes call refresh() instead of just an update().

olivermuc commented 7 years ago

Addendum:

Here is my local fix that solves the problem:

public ngOnChanges(changes: SimpleChanges): void {
    if (this.initFlag) {
        // Check if the changes are in the data or datasets
        if (changes.hasOwnProperty('data') || changes.hasOwnProperty('datasets')) {
            // Oliver, May 19th 17
            if (changes.hasOwnProperty('legend') ||
                changes.hasOwnProperty('colors') ||
                changes.hasOwnProperty('labels') ||
                changes.hasOwnProperty('options')) {
                this.refresh();
            } else /* Oliver, May 19th 17 */ if (changes['data']) {
                this.updateChartData(changes['data'].currentValue);
            } else {
                this.updateChartData(changes['datasets'].currentValue);
            }

            this.chart.update();
        } else {
            // otherwise rebuild the chart
            this.refresh();
        }
    }
}
ilanle commented 7 years ago

Is this going to get addressed in a fix? i am having the same issue

olivermuc commented 7 years ago

@ilanle pull request submitted

marcpursals commented 7 years ago

+1

NoWayJA commented 7 years ago

+1

thetafarm commented 7 years ago

+1

sincraianul commented 7 years ago

+1

RobertBakic commented 7 years ago

+1

kadosh commented 7 years ago

+1

olavomachadoshibata commented 7 years ago

+1

olivermuc commented 7 years ago

looking at all the open pull reqs, it seems though that the original author has lost interest in supporting this.

olavomachadoshibata commented 7 years ago

I found his email on the page (dmitriy.shekhovtsov@valor-software.com), I am going to send him a message. Let you guys know if he answers.

olavomachadoshibata commented 7 years ago

@valorkin

raugaral commented 7 years ago

some update?

Shadowlauch commented 7 years ago

I needed a quick solution and couldn't really wait for an update so I programmed a quick workaround:

First we need a reference to the baseChart element

<canvas baseChart #baseChart="base-chart" width="400" height="250"
                [datasets]="chartData"
                [labels]="chartLabels"
                [options]="chartOptions"
                [legend]="chartLegend"
                [chartType]="chartType"></canvas>

Then we can access and save the directive to a member of our component:

@ViewChild("baseChart")
    chart: BaseChartDirective;

And whenever we change something in the labels or other non updating options, we trigger this bit:

if(this.chart !== undefined){
       this.chart.ngOnDestroy();
       this.chart.chart = this.chart.getChartBuilder(this.chart.ctx);
}

This just destroys the charts and recreates is (basically copied from the refresh function). I hope that helps until its fixed.

rightisleft commented 7 years ago

@Shadowlauch - The above causes a null data error:

ModelSublabelComponent.html:59 ERROR TypeError: Cannot read property 'data' of undefined
    at charts.js:90
    at Array.forEach (<anonymous>)
    at BaseChartDirective.webpackJsonp.../../../../ng2-charts/charts/charts.js.BaseChartDirective.updateChartData (charts.js:89)
    at BaseChartDirective.webpackJsonp.../../../../ng2-charts/charts/charts.js.BaseChartDirective.ngOnChanges (charts.js:39)
    at checkAndUpdateDirectiveInline (core.es5.js:10891)
    at checkAndUpdateNodeInline (core.es5.js:12382)
    at checkAndUpdateNode (core.es5.js:12321)
    at debugCheckAndUpdateNode (core.es5.js:13180)
    at debugCheckDirectivesFn (core.es5.js:13121)
rightisleft commented 7 years ago
BaseChartDirective.prototype.updateChartData = function (newDataValues) {
    if (Array.isArray(newDataValues[0].data)) {
        this.chart.data.datasets.forEach(function (dataset, i) {
            dataset.data = newDataValues[i].data;
            if (newDataValues[i].label) {
                dataset.label = newDataValues[i].label; // this throws an undefined error 
            }
        });
    }
    else {
        this.chart.data.datasets[0].data = newDataValues;
    }
};
Shadowlauch commented 7 years ago

@rightisleft I can't reproduce that error.

One thing I noticed though while trying to figure out where the error occurs. My Chart does not correctly update, because when I trigger the chart redraw the changes to the labels have not yet been detected by the Angular change detection. So I had to injuect the ChangeDetectionRef and call .detectChanges() right before I call the ngOnDestroy.

rightisleft commented 7 years ago

The work arounds in this thread are a sad state of affairs ... :(

donothingloop commented 7 years ago

Fixed in my fork along with some other things. https://github.com/donothingloop/ng2-charts-x

juergen-vogel commented 7 years ago

I have the problem that labels are not correctly refreshed. The solutions from @rightisleft and @Shadowlauch didn't work for me.

That's my workaround:

Accessing the directive via ViewChild:

<canvas baseChart #baseChart="base-chart" ...></canvas>
@ViewChild("baseChart") chart: BaseChartDirective;

And after setting the new data I run this function:

reloadChart() {
    if (this.chart !== undefined) {
       this.chart.chart.destroy();
       this.chart.chart = 0;

       this.chart.datasets = this.datasets;
       this.chart.labels = this.labels;
       this.chart.ngOnInit();
    }
}

this.datasets and this.labels hold my data.

Thanks for all the input in this channel 👍

domske commented 6 years ago

+1 In my opinion an empty datasets array (no datasets) should be valid. Because adding datasets or / and data of datasets later. Chart.js supports that. http://www.chartjs.org/samples/latest/charts/line/basic.html

The workaround with destory and init works not really good. But the idea is good, Create the chart completely new instead of update.

But anyway, this issue should be fixed. The chart should update with every change. Also for ticks min, max, datasets and the other options.

Currently if you work on one dataset, you can update the chart with this.chart.chart.update(). If you added a dataset before and only updates the data of dataset. This feels so wrong 😄

egervari commented 6 years ago

I agree that empty arrays should be valid, and I also agree that the charts library should work in the same way angular itself works. I don't know it needs to have a different coding model for.

JacoRoos commented 6 years ago

@Shadowlauch Regarding this https://github.com/valor-software/ng2-charts/issues/806#issuecomment-317352086.

I had to add line 3 for my labels to properly update, even though I had already set the ngModel value for the labels to the same value.

if (this.chart !== undefined) {
        this.chart.ngOnDestroy();
        this.chart.labels = lineSeriesLabels;
        this.chart.chart = this.chart.getChartBuilder(this.chart.ctx);
      }
rdelriods01 commented 6 years ago

There is another way to do it:

In your HTML you have

<canvas baseChart 
            [datasets]="ChartData"
            //...other stuff >
</canvas>

and in the component I have a function which update the chart with new data, and then I clone the datasets and re-assign it

drawChart(){
    this.ChartData=[{data: this.dataX, label: 'X'}]; // this.dataX has new values from some place in my code
    //nothing happened with my chart yet, until I add this lines:        
    let clone = JSON.parse(JSON.stringify(this.ChartData));
    this.ChartData=clone;
   //other stuff like labels etc.
}

this works for me, hope it works for you too

andreyhideki commented 6 years ago

Use ngIf for

Error: ng-charts configuration error, data or datasets field are required to render char line

<div style="display: block" *ngIf="chartData.datasets.length > 0" >
<canvas baseChart #baseChart="base-chart" width="400" height="250"
                [datasets]="chartData"
                [labels]="chartLabels"
                [options]="chartOptions"
                [legend]="chartLegend"
                [chartType]="chartType"></canvas>
</div>
shiamalon commented 6 years ago

I have the problem that labels are not correctly refreshed. The solutions from @rightisleft and @Shadowlauch didn't work for me.

That's my workaround:

Accessing the directive via ViewChild:

<canvas baseChart #baseChart="base-chart" ...></canvas>
@ViewChild("baseChart") chart: BaseChartDirective;

And after setting the new data I run this function:

reloadChart() {
    if (this.chart !== undefined) {
       this.chart.chart.destroy();
       this.chart.chart = 0;

       this.chart.datasets = this.datasets;
       this.chart.labels = this.labels;
       this.chart.ngOnInit();
    }
}

this.datasets and this.labels hold my data.

Thanks for all the input in this channel

This is the only thing that worked. All the other solutions give some errors at some point!

vikashRawat commented 5 years ago

If there are two charts(Pie and Line) then only first rendered charts get refreshed.Can anyone please tell me how to refresh particular chart.

paviad commented 5 years ago

Closed by ef5d2f4

ohabash commented 1 year ago

Sure wish chartjs was not the only option