Skip to content

Commit

Permalink
fix(arc,api): Correct on data representation and handling (#332)
Browse files Browse the repository at this point in the history
- Fix % value for pie when uses with padAngle or padding options
- Fix to return multiple data values as indicated

Fix #303 
Fix #331
Close #332
  • Loading branch information
netil committed Mar 14, 2018
1 parent 7e06851 commit 3eec8d7
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 10 deletions.
22 changes: 18 additions & 4 deletions spec/api/api.data-spec.js
Expand Up @@ -90,13 +90,27 @@ describe("API data", function() {

describe("data.values()", () => {
it("should return values for specified target", () => {
const values = chart.data.values("data1");
const expectedValues = [30, 200, 100, 400, 150, 250];
const expectedValues1 = [30, 200, 100, 400, 150, 250];
const expectedValues2 = [5000, 2000, 1000, 4000, 1500, 2500];

expect(values.length).to.be.equal(6);
// retrieve one data value
let values = chart.data.values("data1");

expect(values.length).to.be.equal(expectedValues1.length);

values.forEach((v, i) => {
expect(v).to.be.equal(expectedValues[i]);
expect(v).to.be.equal(expectedValues1[i]);
});

// retrieve two data values
values = chart.data.values(["data1", "data2"]);

expect(values.length).to.be.equal(expectedValues1.length + expectedValues2.length);

values.forEach((v, i) => {
const expected = expectedValues1.concat(expectedValues2);

expect(v).to.be.equal(expected[i]);
});
});

Expand Down
12 changes: 12 additions & 0 deletions spec/internals/arc-spec.js
Expand Up @@ -116,6 +116,12 @@ describe("ARC", () => {

expect(chart.internal.pie.padAngle()()).to.be.equal(value);
expect(chart.internal.main.selectAll(`text.${CLASS.chartArcsTitle} tspan`).size()).to.be.equal(3);

d3.selectAll(`.${CLASS.chartArc} text`).each(function(d) {
const value = parseInt(this.textContent);

expect(value).to.be.equal(d.value);
});
});

it("check for padding and innerRadius", () => {
Expand All @@ -138,6 +144,12 @@ describe("ARC", () => {

expect(internal.pie.padAngle()()).to.be.equal(padding * 0.01);
expect(internal.innerRadius).to.be.equal(innerRadius);

d3.selectAll(`.${CLASS.chartArc} text`).each(function(d) {
const value = parseInt(this.textContent);

expect(value).to.be.equal(d.value);
});
});
});

Expand Down
10 changes: 7 additions & 3 deletions src/api/api.data.js
Expand Up @@ -3,7 +3,7 @@
* billboard.js project is licensed under the MIT license
*/
import Chart from "../internals/Chart";
import {extend, isUndefined} from "../internals/util";
import {extend, isUndefined, isArray} from "../internals/util";

/**
* Get data loaded in the chart.
Expand Down Expand Up @@ -65,8 +65,12 @@ extend(data, {
if (targetId) {
const targets = this.data(targetId);

if (targets && targets[0]) {
values = targets[0].values.map(d => d.value);
if (targets && isArray(targets)) {
values = [];

targets.forEach(v => {
values = values.concat(v.values.map(d => d.value));
});
}
}

Expand Down
25 changes: 24 additions & 1 deletion src/data/data.js
Expand Up @@ -5,7 +5,8 @@
import {
min as d3Min,
max as d3Max,
merge as d3Merge
merge as d3Merge,
sum as d3Sum
} from "d3-array";
import {set as d3Set} from "d3-collection";
import CLASS from "../config/classes";
Expand Down Expand Up @@ -246,6 +247,28 @@ extend(ChartInternal.prototype, {
return $$.cache.$minMaxData;
},

/**
* Get total data sum
* @private
* @return {Number}
*/
getTotalDataSum() {
const $$ = this;

if (!$$.cache.$totalDataSum) {
let total = 0;

$$.data.targets.map(t => t.values)
.forEach(v => {
total += d3Sum(v, t => t.value);
});

$$.cache.$totalDataSum = total;
}

return $$.cache.$totalDataSum;
},

/**
* Get filtered data by value
* @param {Object} data
Expand Down
27 changes: 25 additions & 2 deletions src/shape/arc.js
Expand Up @@ -10,6 +10,7 @@ import {
arc as d3Arc,
pie as d3Pie
} from "d3-shape";
import {sum as d3Sum} from "d3-array";
import {interpolate as d3Interpolate} from "d3-interpolate";
import ChartInternal from "../internals/ChartInternal";
import CLASS from "../config/classes";
Expand Down Expand Up @@ -129,6 +130,7 @@ extend(ChartInternal.prototype, {

return newArc;
},

getSvgArcExpanded(rate) {
const $$ = this;
const arc = d3Arc()
Expand All @@ -141,6 +143,7 @@ extend(ChartInternal.prototype, {
return updated ? arc(updated) : "M 0 0";
};
},

getArc(d, withoutUpdate, force) {
return force || this.isArcType(d.data) ? this.svgArc(d, withoutUpdate) : "M 0 0";
},
Expand Down Expand Up @@ -181,9 +184,28 @@ extend(ChartInternal.prototype, {
getArcRatio(d) {
const $$ = this;
const config = $$.config;
const whole = Math.PI * ($$.hasType("gauge") && !config.gauge_fullCircle ? 1 : 2);
let val = null;

if (d) {
// if has padAngle set, calculate rate based on value
if ($$.pie.padAngle()()) {
let total = $$.getTotalDataSum();

if ($$.hiddenTargetIds.length) {
total -= d3Sum($$.api.data.values.call($$.api, $$.hiddenTargetIds));
}

val = d.value / total;

return d ? (d.endAngle - d.startAngle) / whole : null;
// otherwise, based on the rendered angle value
} else {
val = (d.endAngle - d.startAngle) / (
Math.PI * ($$.hasType("gauge") && !config.gauge_fullCircle ? 1 : 2)
);
}
}

return val;
},

convertToArcData(d) {
Expand Down Expand Up @@ -469,6 +491,7 @@ extend(ChartInternal.prototype, {
d.startAngle = config.gauge_startingAngle;
d.endAngle = config.gauge_startingAngle;
}

this._current = d;
})
.merge(mainArc);
Expand Down

0 comments on commit 3eec8d7

Please sign in to comment.