Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bar Charts not updating correctly on dataset change #29

Closed
sbatson5 opened this issue Jul 10, 2015 · 8 comments
Closed

Bar Charts not updating correctly on dataset change #29

sbatson5 opened this issue Jul 10, 2015 · 8 comments

Comments

@sbatson5
Copy link

Thanks for all the work on ember-cli-chart. I have an issue where we track interest payments over sets of years, with 2 data points:
screen shot 2015-07-10 at 1 38 11 pm

However, when the loan is paid off faster than 10 years, the chart should update. Here is the default behavior:
screen shot 2015-07-10 at 11 44 39 am

To solve this, I edited chart-data-updater to check to see if the bars length matched our new dataset length:

    if (chart.datasets.length !== datasets.length) {
      return this.set('redraw', true);
    } else if (chart.datasets[0].bars.length !== datasets[0].data.length) {
      return this.set('redraw', true);
    }

Is there a better way to do this? Would it make sense to also confirm that the type is bar?

@aomran
Copy link
Owner

aomran commented Jul 10, 2015

@sbatson5 the chart.datasets[0].bars won't work if it's not a bar chart as you might have guessed. chart.datasets[0].bars !== 'undefined' should do the trick.

It seems we keep running into edge cases with the automatic updating. I'm hoping Chartjs version 2 will fix all of these hacks once it's stable.

@sbatson5
Copy link
Author

what if you passed the type to chart-data-updater?

    var redraw = ChartDataUpdater.create({
      data: data,
      chart: chart,
      type: type
    }).updateByType();

Then specified the type:

    if (chart.datasets.length !== datasets.length) {
      return this.set('redraw', true);
    } else if (this.get('type') === 'Bar' && chart.datasets[0].bars.length !== datasets[0].data.length) {
      return this.set('redraw', true);
    }

@sbatson5
Copy link
Author

Misunderstood your first reply. Revised to:

    if (chart.datasets.length !== datasets.length) {
      return this.set('redraw', true);
    } else if (typeof chart.datasets[0].bars !== 'undefined') {
      if (chart.datasets[0].bars.length !== datasets[0].data.length) {
        return this.set('redraw', true);
      }
    }

@sbatson5
Copy link
Author

@aomra015 Opened a PR with the above revision and unit test.
#30

@sbatson5
Copy link
Author

@aomra015 Have you had a chance to look at the PR I opened regarding updating bar charts on dataset length changes? Let me know if it looks good or if you see any issues.

@aomran
Copy link
Owner

aomran commented Jul 23, 2015

I'll try to find some time this weekend to review this. Thanks !

@aomran
Copy link
Owner

aomran commented Aug 8, 2015

merged

@aomran aomran closed this as completed Aug 8, 2015
@sbatson5
Copy link
Author

👍 Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants