Skip to content

Allow specifying bar chart via xy data points #4565

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.

Sign up for GitHub

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

Merged
simonbrunel merged 1 commit into from
Jul 29, 2017
Merged

Allow specifying bar chart via xy data points #4565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/charts/bar.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,12 @@ The `data` property of a dataset for a bar chart is specified as a an array of n
data: [20, 10]
```

You can also specify the dataset as x/y coordinates.

```javascript
data: [{x:'2016-12-25', y:20}, {'2016-12-26', y:10}]
```

# Stacked Bar Chart

Bar charts can be configured into stacked bar charts by changing the settings on the X and Y axes to enable stacking. Stacked bar charts can be used to show how one data series is made up of a number of smaller pieces.
Expand Down
9 changes: 4 additions & 5 deletions src/controllers/controller.bar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ module.exports = function(Chart) {
var meta = me.getMeta();
var scale = me.getValueScale();
var datasets = chart.data.datasets;
var value = Number(datasets[datasetIndex].data[index]);
var value = scale.getRightValue(datasets[datasetIndex].data[index]);
var stacked = scale.options.stacked;
var stack = meta.stack;
var start = 0;
Expand All @@ -280,7 +280,7 @@ module.exports = function(Chart) {
imeta.controller.getValueScaleId() === scale.id &&
chart.isDatasetVisible(i)) {

ivalue = Number(datasets[i].data[index]);
ivalue = scale.getRightValue(datasets[i].data[index]);
if ((value < 0 && ivalue < 0) || (value >= 0 && ivalue > 0)) {
start += ivalue;
}
Expand Down Expand Up @@ -327,17 +327,16 @@ module.exports = function(Chart) {
draw: function() {
var me = this;
var chart = me.chart;
var scale = me.getIndexScale();
var rects = me.getMeta().data;
var dataset = me.getDataset();
var ilen = rects.length;
var i = 0;
var d;

helpers.canvas.clipArea(chart.ctx, chart.chartArea);

for (; i < ilen; ++i) {
d = dataset.data[i];
if (d !== null && d !== undefined && !isNaN(d)) {

@simonbrunel simonbrunel Jul 26, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason of this change?

@benmccann benmccann Jul 26, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because isNaN returns true when you pass in an object

Also, I don't know if it's really the bar controller's responsibility to do such a check anyway. It shouldn't need to know the logic for whether the rectangle element can be drawn. That logic should be encapsulated in the rectangle element.

@simonbrunel simonbrunel Jul 26, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a regression, null/undefined/NaN data are currently used to skip element (@etimberg?) Should be the same logic as your other change: d = scale.getRightValue(dataset.data[i]).

I doubt that's the responsability of the element to decide whether it needs to drawn itself. The controller seems a better candidate, each controller can have different behavior.

@etimberg etimberg Jul 26, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the current behaviour for bars is that null/undefined/NaN are supposed to just not draw anything but I'm not 100% sure on that. I know we use those values to skip drawing grid lines in the axis though

@benmccann benmccann Jul 26, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I like scale.getRightValue much better than the old check. I've updated it to that

if (!isNaN(scale.getRightValue(dataset.data[i]))) {
rects[i].draw();
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/scales/scale.linearbase.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ module.exports = function(Chart) {
var noop = helpers.noop;

Chart.LinearScaleBase = Chart.Scale.extend({
getRightValue: function(value) {
if (typeof value === 'string') {
return +value;
}
return Chart.Scale.prototype.getRightValue.call(this, value);
},

handleTickRangeOptions: function() {
var me = this;
var opts = me.options;
Expand Down