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

Added 'angle' option to Polar Charts #5279

Merged
merged 9 commits into from Jun 17, 2018
75 changes: 51 additions & 24 deletions src/controllers/controller.polarArea.js
Expand Up @@ -117,25 +117,44 @@ module.exports = function(Chart) {
linkScales: helpers.noop,

update: function(reset) {
var me = this;
var dataset = me.getDataset();
var meta = me.getMeta();
var start = me.chart.options.startAngle || 0;
var starts = me._starts = [];
var angles = me._angles = [];
var i, ilen, angle;

me.updateRadius();

meta.count = me.countVisibleElements();

for (i = 0, ilen = dataset.data.length; i < ilen; i++) {
starts[i] = start;
angle = me._computeAngle(i);
angles[i] = angle;
start += angle;
}

helpers.each(meta.data, function(arc, index) {
me.updateElement(arc, index, reset);
});
},

updateRadius: function() {
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be private:

/**
 * @private
 */
_updateRadius: function() {
  // ...

var me = this;
var chart = me.chart;
var chartArea = chart.chartArea;
var meta = me.getMeta();
var opts = chart.options;
var arcOpts = opts.elements.arc;
var minSize = Math.min(chartArea.right - chartArea.left, chartArea.bottom - chartArea.top);

chart.outerRadius = Math.max((minSize - arcOpts.borderWidth / 2) / 2, 0);
chart.innerRadius = Math.max(opts.cutoutPercentage ? (chart.outerRadius / 100) * (opts.cutoutPercentage) : 1, 0);
chart.radiusLength = (chart.outerRadius - chart.innerRadius) / chart.getVisibleDatasetCount();

me.outerRadius = chart.outerRadius - (chart.radiusLength * me.index);
me.innerRadius = me.outerRadius - chart.radiusLength;

meta.count = me.countVisibleElements();

helpers.each(meta.data, function(arc, index) {
me.updateElement(arc, index, reset);
});
},

updateElement: function(arc, index, reset) {
Expand All @@ -147,25 +166,14 @@ module.exports = function(Chart) {
var scale = chart.scale;
var labels = chart.data.labels;

var circumference = me.calculateCircumference(dataset.data[index]);
var centerX = scale.xCenter;
var centerY = scale.yCenter;

// If there is NaN data before us, we need to calculate the starting angle correctly.
// We could be way more efficient here, but its unlikely that the polar area chart will have a lot of data
var visibleCount = 0;
var meta = me.getMeta();
for (var i = 0; i < index; ++i) {
if (!isNaN(dataset.data[i]) && !meta.data[i].hidden) {
++visibleCount;
}
}

// var negHalfPI = -0.5 * Math.PI;
var datasetStartAngle = opts.startAngle;
var distance = arc.hidden ? 0 : scale.getDistanceFromCenterForValue(dataset.data[index]);
var startAngle = datasetStartAngle + (circumference * visibleCount);
Copy link
Member

Choose a reason for hiding this comment

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

Travis complains that visibleCount is not anymore used and it seems that the whole block from line 167 to 173 can be removed. Does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh right, thanks "Travis" 😅

var endAngle = startAngle + (arc.hidden ? 0 : circumference);
var startAngle = me._starts[index];
var endAngle = startAngle + (arc.hidden ? 0 : me._angles[index]);

var resetRadius = animationOpts.animateScale ? 0 : scale.getDistanceFromCenterForValue(dataset.data[index]);

Expand Down Expand Up @@ -211,12 +219,31 @@ module.exports = function(Chart) {
return count;
},

calculateCircumference: function(value) {
/**
* @private
*/
_computeAngle: function(index) {
Copy link
Member

Choose a reason for hiding this comment

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

We started to be even more explicit about private stuff by adding the following comment above:

/**
 * @private
 */
_computeAngle: function(index) {

var me = this;
var count = this.getMeta().count;
if (count > 0 && !isNaN(value)) {
return (2 * Math.PI) / count;
var dataset = me.getDataset();
var meta = me.getMeta();

if (isNaN(dataset.data[index]) || meta.data[index].hidden) {
return 0;
}
return 0;

// Scriptable options
var context = {
chart: me.chart,
dataIndex: index,
dataset: dataset,
datasetIndex: me.index
};

return helpers.options.resolve([
me.chart.options.angle,
(2 * Math.PI) / count
], context, index);
}
});
};
30 changes: 30 additions & 0 deletions test/fixtures/controller.polarArea/angle-array.json
@@ -0,0 +1,30 @@
{
"config": {
"type": "polarArea",
"data": {
"labels": ["A", "B", "C", "D", "E"],
"datasets": [{
"data": [11, 16, 21, 7, 10],
"backgroundColor": [
"rgba(255, 99, 132, 0.8)",
"rgba(54, 162, 235, 0.8)",
"rgba(255, 206, 86, 0.8)",
"rgba(75, 192, 192, 0.8)",
"rgba(153, 102, 255, 0.8)",
"rgba(255, 159, 64, 0.8)"
]
}]
},
"options": {
"angle": [
1.0566, 1.7566, 1.0566, 2.1566, 0.2566
],
"responsive": false,
"legend": false,
"title": false,
"scale": {
"display": false
}
}
Copy link
Member

Choose a reason for hiding this comment

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

We should remove all UI that are not part of the tests, which include the labels and the "grid lines". Maybe adding scale: { display: false } is enough.

Copy link
Member

Choose a reason for hiding this comment

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

I would also pick a more accentuated and different background color for each slices to make sure it fails if there is overlapping elements.

}
}
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
27 changes: 27 additions & 0 deletions test/fixtures/controller.polarArea/angle-undefined.json
@@ -0,0 +1,27 @@
{
"config": {
"type": "polarArea",
"data": {
"labels": ["A", "B", "C", "D", "E"],
"datasets": [{
"data": [11, 16, 21, 7, 10],
"backgroundColor": [
"rgba(255, 99, 132, 0.8)",
"rgba(54, 162, 235, 0.8)",
"rgba(255, 206, 86, 0.8)",
"rgba(75, 192, 192, 0.8)",
"rgba(153, 102, 255, 0.8)",
"rgba(255, 159, 64, 0.8)"
]
}]
},
"options": {
"responsive": false,
"legend": false,
"title": false,
"scale": {
"display": false
}
}
}
}
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions test/specs/controller.polarArea.tests.js
@@ -1,3 +1,5 @@
describe('auto', jasmine.specsFromFixtures('controller.polarArea'));

describe('Chart.controllers.polarArea', function() {
it('should be constructed', function() {
var chart = window.acquireChart({
Expand Down