jtblin / angular-chart.js

Reactive, responsive, beautiful charts for AngularJS using Chart.js: http://jtblin.github.io/angular-chart.js
Other
2.67k stars 759 forks source link

Hovering over line chart shows old chart data #187

Closed martellaj closed 8 years ago

martellaj commented 9 years ago

This is my scenario (using a line chart):

  1. Make HTTP request to get chart data from API.
  2. Add chart data to chart by assigning to a bound variable called chartData.
  3. User clicks a button to fetch different data, so another HTTP request is made to get new data.
  4. I clear the chartData array and then add the new data.

Expected outcome: Chart no longer has any trace of original data, but instead shows new data and user can interact with it without issues.

Actual outcome: Chart shows new data, but if user hovers over the chart, it'll flicker back and forth between original and new data.

olddata

jtblin commented 9 years ago

How do you clear the data? Passing an empty array does not clear the data, it's a chart.js issue. Otherwise can you post a repro case, see jsbin template in readme? It may be an issue with chart.js itself btw.

martellaj commented 9 years ago

So it looks like it's not a Chart.js "issue", but you just have to clear data using .clear() or .destroy(). Do you have a suggestion on how to do this with the Angular directive? According to README, this should be handled automatically. I'll work on setting up a repo for you.

jbmartinez commented 9 years ago

I have the same problem, I need to draw different data sets in the same chart and the old data keeps keeps showing. Is there a way to call .clear or .destroy on the chart?

jbmartinez commented 9 years ago

Found a way:

$scope.$on('create', function (event, chart) {
  ctrl.chart = chart; // ctrl is a controller reference
});

Then I could use ctrl.chart.destroy(). Surprisingly, that didn't solve the problem. I had to disable the tooltips as workaround. I would like to enable the tooltips, though.

andre-gois commented 9 years ago

any news regarding this?

pedrocatre commented 9 years ago

I was having the same issue. At first I tried to reproduce it using the jsbin template but there everything I tried worked. So I tried changing the values in my app the same way I was doing in the example and it also worked (basically onClick changed the values in the chart).

In the end what fixed for me was this:

In my app when a date range filter value changed I would set the values for labels and data in a service like this:

vm.labels = newLabels;
vm.data = newData;

when I changed it to this the bug stopped appearing:

$timeout(function() {
    vm.labels = newLabels;
    vm.data = newData;
});

Hope this helps others.

thomasduchatelle commented 9 years ago

@pedrocatre , that works, thanks for the workaround!

yaojingguo commented 9 years ago

Pie chart has the same problem. But this problem only occurs if the new data has less items than the old data.

And the workaround does not work for me. So I have disabled the tooltips.

snielson-dyn commented 9 years ago

The workaround did not work for me when I have multiple instances of charts being displayed. I read through some discussion on the Chart.js forum and they mention this problem occurring when there are multiple instances of the chart pointing to the same canvas. You can read the discussion here: https://github.com/nnnick/Chart.js/issues/920

I looked through the angular-charts source code and I see a number of places where the chart is being destroyed and recreated in other spots and some of them involving timeouts when the element is hidden. I'm wondering if there is a loose reference somewhere that is being retained in a closure that is causing this behavior when a hover event is occurring. I have to get back to my work items but I hope this initial investigation can lead to some solutions.

yayiekim commented 9 years ago

The work around does not work for Line chart. some data aren't destroyed

psmarcin commented 8 years ago

I think I finally found where is bug. For me it was duplicated initialization of chart. On page load I do ajax request and get data but also I do the same on datapicker change (it was triggered on page load too). After I remove one of them everything seams to work fine.

tobiastheil commented 8 years ago

The $timeout workaround didn't work for me. However, what worked is the following code inside my chart directive:

var $chart;
$scope.$on("create", function (event, chart) {
  if (typeof $chart !== "undefined") {
    $chart.destroy();
  }

  $chart = chart;
});

There are more than one chart instances... I just destroy every instance except the last one.

sandy240287 commented 8 years ago

@tobiastheil Thanks it worked for me.

kr4ng commented 8 years ago

@tobiastheil Thanks so much, sweet workaround.

jtblin commented 8 years ago

Very smart @tobiastheil, I'll try and include that in the lib directly :+1:

gkampjes commented 8 years ago

Bugfix is inbound, should have have a PR by the weekend.

jtblin commented 8 years ago

:+1:

kdomagal commented 8 years ago

+1

netmonster01 commented 8 years ago

+1

shubhampatil17 commented 8 years ago

I was facing the same issue, refreshing the page using $window.location.reload() after rendering new data stopped this flickering bug in my case. Check if it helps anyone.

robert-king commented 8 years ago

I put my chart canvas inside a div with an ng-if="chartLoaded". That way the dom element is destroyed when i load a new chart.

mihai-dinculescu commented 8 years ago

@tobiastheil your work around yields low quality charts for me. My investigation shows that I need to keep the last 2-3 instances, not just the last one.

I have come up with this modification:

// store created instances
var chartInstances = [];
$scope.$on("create", function (event, chart) {
  chartInstances.push(chart);
});
// destroy all the old instances when data changes
_.each(chartInstances, function(chart) {
    chart.destroy();
});
chartInstances = [];
micheledallatorre commented 8 years ago

Same problem here. The issue happens both on LineCharts and DoughnutCharts.

I am using something very similar to this example https://ga-dev-tools.appspot.com/embed-api/ to display Google Analytics data using ChartJs charts.

I added also a datepicker http://www.daterangepicker.com/ and whenever I change the daterange, I render the charts again.

Basically, what is done in the example above by Google is:

  /**
   * Update the activeUsers component, the Chartjs charts, and the dashboard
   * title whenever the user changes the view.
   */
  viewSelector.on('viewChange', function(data) {
    var title = document.getElementById('view-name');
    title.innerHTML = data.property.name + ' (' + data.view.name + ')';

    // Start tracking active users for this view.
    activeUsers.set(data).execute();

    // Render all the of charts for this view.
    renderWeekOverWeekChart(data.ids);
    renderYearOverYearChart(data.ids);
    renderTopBrowsersChart(data.ids);
    renderTopCountriesChart(data.ids);
  });

whereas what I do is the following.

        $('#daterange-btn').on('apply.daterangepicker', function(ev, picker) {
            // update date range for the Google Analytics queries
            query_start_date = picker.startDate.format('YYYY-MM-DD');
            query_end_date = picker.endDate.format('YYYY-MM-DD');
            // draw the charts again with the new chosen date range
            renderCharts('ga:' + VIEW_ID);
        });

Charts are updated correctly with the new chosen date range, but hovering on them display (sometimes) old data.

I tried some of the proposed solutions above, but with no luck so far... :(

UPDATE: the only thing that seems to be working for me is to do as follows. (see also http://stackoverflow.com/questions/24815851/how-do-clear-a-chart-from-a-canvas-so-that-hover-events-cannot-be-triggered).

        $('#daterange-btn').on('apply.daterangepicker', function(ev, picker) {
            query_start_date = picker.startDate.format('YYYY-MM-DD');
            query_end_date = picker.endDate.format('YYYY-MM-DD');
            // remove the canvas id containing the chart
            $('#chart-mylinechart').remove();
            // append the canvas again to its parent div
            $('#myChartParentDivt').append('<canvas id="chart-mylinechart"></canvas>');
            renderCharts('ga:' + VIEW_ID);
        });

Of course I would need to do that for each and every canvas, so I hope someone can suggest a better solution! Thanks!

UPDATE 2 I updated my code above, to fix the issue on every canvas/chart.

        $('#daterange-btn').on('apply.daterangepicker', function(ev, picker) {
            query_start_date = picker.startDate.format('YYYY-MM-DD');
            query_end_date = picker.endDate.format('YYYY-MM-DD');

            //get all canvas parent elements (i.e. div elements with class "chart")
            $('canvas').parent().each(function () {
                //get child canvas id
                childCanvasId = $(this).children().attr('id');
                //remove canvas
                $('#'+childCanvasId).remove();
                // append new canvas to the parent again
                $(this).append('<canvas id="'+childCanvasId+'"></canvas>');
            });

            // render charts again
            renderCharts('ga:' + VIEW_ID);
        });
jtblin commented 8 years ago

I've now pushed a fix to always destroy the previous chart if it exists before creating a new one so it should solve this problem.

Stefvg commented 8 years ago

It didn't solve it for me, my chart is still buggy like in the post.

jtblin commented 8 years ago

Ah, did you use the latest from master? I haven't pushed a new version yet. Do you have a jsbin template where I can see the issue? Saying "it doesn't solve it for me" is not supra helpful without more information. The problem with having multiple charts is clearly fixed now as I always call chart.destroy() before creating a chart, and it is the correct fix as per https://github.com/nnnick/Chart.js/issues/920#issuecomment-167166724.

Stefvg commented 8 years ago

Hi, yeah I even copy + pasted the code from the latest commit so I was sure that I had the last version. I don't have a jsbin template I think. An example of the issue is on this page: https://svg-apache.iminds-security.be/dashboard.html?app=Sample%20Filled%20Application -> select the Meter page in the top and play with the slider, you will see that the data gets updated, but if you hover over the line charts, it is buggy like in the starting post. I don't know what you have to get from me to solve this or what I have to do?

jtblin commented 8 years ago

Well you're using the minified file on this site, that hasn't been generated with the fix so your site is not using the latest version right now.

I've pushed the new version now, make sure you use 0.9.0 and let me know if that doesn't solve the problem.

Stefvg commented 8 years ago

Hi,

It did fix the problem, thank you for the quick fix!

jtblin commented 8 years ago

Glad to hear that! :)

johnedvard commented 8 years ago

The update seems to work for me as well! Thank you

micheledallatorre commented 8 years ago

Is this one the latest updated version? https://cdnjs.cloudflare.com/ajax/libs/Chart.js/1.0.2/Chart.min.js (source: https://cdnjs.com/#q=chart.js)

Thanks!

jtblin commented 8 years ago

@micheledallatorre the link you've put is for the latest stable chart.js.

The latest angular-chart.js which is the wrapper for chart.js would be //cdn.jsdelivr.net/angular.chartjs/latest/angular-chart.min.js and //cdn.jsdelivr.net/angular.chartjs/latest/angular-chart.css.

But you should install via npm as it will install all the dependencies for you npm install --save angular-chart.js.

micheledallatorre commented 8 years ago

@jtblin Yeah, thanks, but my question is the following. Is the link to the latest stable chart.js posted by me above referring to the version with the fix in this thread?

Thanks.

jtblin commented 8 years ago

Yes it is.

micheledallatorre commented 8 years ago

@jtblin: just tried it out.

Actually decommenting my piece of code above (See UPDATE2 https://github.com/jtblin/angular-chart.js/issues/187#issuecomment-190769767), hence only loading https://cdnjs.cloudflare.com/ajax/libs/Chart.js/1.0.2/Chart.min.js, causes the issue to appear again.

jtblin commented 8 years ago

Mate, your example code has nothing to do with this library, it's a mix of jQuery and calls to a renderChart function. I'm not sure how you hope I can help you with that but I'm having a hard time understanding anything about your issue and how this is related to this lib. If you think the issue is with this library then please post a jsbin repro case, otherwise I'd suggest you ask your question on http://stackoverflow.com/.

micheledallatorre commented 8 years ago

Thanks @jtblin. I created this https://jsfiddle.net/mdt86/pav5y8h1/6/ where your fix is working (i.e. updating the chart with new data, does not cause any issue hovering on the chart).

It must be some conflict in my full code, I will dig into it, thank you very much again!

jtblin commented 8 years ago

@micheledallatorre you are not even using my library! You are using raw Chart.js: https://github.com/nnnick/Chart.js...

micheledallatorre commented 8 years ago

Ehrrr... I think I got confused! Searching for my problem I ended up fixing it following this discussion, but did not notice it was referring to an AngularJS version/improvement of raw Chart.js! :|

Thanks and sorry again for the misunderstanding! :)

AlexZhenWang commented 7 years ago

I had the same problem. I fix this problem by deleting the div and rebuilding another div.

anantgulia commented 7 years ago

First of all I'm using chart.js with vue 2 and the best and easiest solution i found is same as @wangzhen10 mentioned removing the element from the dom and appending it again....

acidjazz commented 7 years ago

@anantgulia May I ask how you went about doing this in Vue?

kero-venturina commented 7 years ago

I did also what @wangzhen10 mention, it works completely fine.

gangxing commented 7 years ago

@wangzhen10 when the dataset changes, I dynamically generate canvas and append it to div. yes, it WORKS for me ,thanks so much.

cinqs commented 7 years ago

Hi, just for your infos Version: 2.6.0-non-minimized-java-webjars Creating the chart as follows:

var drawChart = function(labels, data, type) {
        type = type || 'bar';

        var ctx = document.getElementById('resultChart').getContext('2d');
        var chart = new Chart(ctx, {
            type: type,
            data: {
                labels: labels,
                datasets: [{
                    label: "result of test",
                    data: data
                }]
            },
            options: {
                scales: {
                    yAxes: [{
                        ticks: {
                            beginAtZero:true
                        }
                    }]
                }
            }
        })
    }

if re-invoke this method drawChart with a new dataset, the issues of the PO will be happening again.

But this works. (thanks @tobiastheil )


var chart;

var drawChart = function(labels, data, type) {
        type = type || 'bar';

        if(typeof chart !== "undefined") {
            chart.destroy();
        }

        var ctx = document.getElementById('resultChart').getContext('2d');
        chart = new Chart(ctx, {
            type: type,
            data: {
                labels: labels,
                datasets: [{
                    label: "result of test",
                    data: data
                }]
            },
            options: {
                scales: {
                    yAxes: [{
                        ticks: {
                            beginAtZero:true
                        }
                    }]
                }
            }
        })
    }
ghost commented 7 years ago

Hi all, Not sure if might help somebody or not, but I was going through the same just today. The comment to "delete old data" got me thinking that maybe pushing the the data to the used array might confuse it.

So, when I removed the loop from pushing to the chart label and data array, set it to push to temporary variables and then set the chart arrays to equal the final result, it got fixed for me.

Because I know I am not the best at explaining, I will post the code below:

Problematic Example:

servers.forEach(function (value, key) {
   AjaxController.data.dataServerWebsites.labels.push(value.name);
   AjaxController.data.dataServerWebsites.websites.push(value.total);
});
ChartController.functions.serverWebsites();

Fixed Example:

servers.forEach(function (value, key) {
   tempLabels.push(value.name);
   tempData.push(value.total);
});

AjaxController.data.dataServerWebsites.labels = tempLabels;
AjaxController.data.dataServerWebsites.websites = tempData;
ChartController.functions.serverWebsites();

Hope it helps! :)

strix25 commented 7 years ago

I still have this problem with version: https://cdnjs.cloudflare.com/ajax/libs/Chart.js/2.7.1/Chart.bundle.js

code: `function charterino(){

let myChart = document.getElementById('myChart').getContext('2d');

// Global Options
Chart.defaults.global.defaultFontFamily = 'Lato';
Chart.defaults.global.defaultFontSize = 18;
Chart.defaults.global.defaultFontColor = '#FFF';

let massPopChart = new Chart(myChart, {
  type:'bar', // bar, horizontalBar, pie, line, doughnut, radar, polarArea
  data:{
    labels:['January', 'February', 'March', 'April', 'May', 'June'],
    datasets:[{
      label:'Hours',
      data:[
        jan,
        feb,
        mar,
        apr,
        may,
        jun
      ],
      //backgroundColor:'green',
      backgroundColor:[
        'rgba(255, 99, 132, 0.6)',
        'rgba(54, 162, 235, 0.6)',
        'rgba(255, 206, 86, 0.6)',
        'rgba(75, 192, 192, 0.6)',
        'rgba(153, 102, 255, 0.6)',
        'rgba(255, 159, 64, 0.6)',
        'rgba(255, 99, 132, 0.6)'
      ],
      borderWidth:1,
    //borderColor:'#777',
      hoverBorderWidth:3,
      hoverBorderColor:'#000'
    }]
  },
  options:{
    title:{
      display:true,
      text:`Hours worked per month for: ${employeeName}`,
      fontSize:25
    },
    legend:{
      display:true,
      position:'right',
      labels:{
        fontColor:'#000'
      }
    },
    layout:{
      padding:{
        left:50,
        right:0,
        bottom:0,
        top:0
      }
    },
    tooltips:{
      enabled:false
    },
    scales: {

        yAxes: [{
            ticks: {
                suggestedMin: 140

            }
        }]
    }
  }
});

};`

jayanthjoseph commented 6 years ago

This worked for me.. Before calling create chart call destroy()

if(this.myChart){ this.myChart.destroy(); } createmyChart()

tamkeen-tms commented 6 years ago

I am still facing the same issue here! Every time the data is updated it's added to the existing chart not refreshed!

capture