Skip to content

Commit

Permalink
fix(area-range): Correct handling null data
Browse files Browse the repository at this point in the history 
Fix for area-range data when contains null data

Fix #482
Close #487
  • Loading branch information
@netil
netil committed Jul 13, 2018
1 parent 4b78de5 commit cc1d4ee
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 75 deletions.
35 changes: 19 additions & 16 deletions spec/shape/shape.line-spec.js
View file
Expand Up @@ -236,7 +236,7 @@ describe("SHAPE LINE", () => {

describe("area-range type generation", () => {
const min = 120;
const max = 500
const max = 220;

before(() => {
args = {
Expand All @@ -245,22 +245,22 @@ describe("SHAPE LINE", () => {
columns: [
["timestamps", "2013-01-01", "2013-01-02", "2013-01-03", "2013-01-04", "2013-01-05", "2013-01-06"],
["data1",
[150, 140, 110],
[155, 130, 115],
[null, null, null],
null,
[160, 135, 120],
[135, min, 110],
[180, 150, 130],
[199, 160, 125]
],
["data2",
{high: 155, low: 145, mid: 150},
{high: 200, mid: 190, low: 150},
null,
{high: null, mid: null, low: null},
{high: 230, mid: max, low: 200},
{high: 210, mid: 200, low: 180},
{high: 220, mid: 210, low: 190},
{high: 200, mid: 180, low: 160}
],
["data3", 130, 340, 200, 500, 250, 350]
["data3", 130, 140, 200, 150, 210, 150]
],
type: "area-spline-range",
types: {
Expand All @@ -275,20 +275,23 @@ describe("SHAPE LINE", () => {
};
});

it("Should render the lines correctly when array data supplied", () => {
const target = chart.internal.main.select(`.${CLASS.chartLine}.${CLASS.target}-data1`);
const commands = target.select(`.${CLASS.line}-data1`).attr("d").split("C");
const dataLen = chart.data.values("data1").length;
const checkLineLen = dataName => {
const internal = chart.internal;
const target = internal.main.select(`.${CLASS.chartLine}.${CLASS.target}-${dataName}`);
const commands = target.select(`.${CLASS.line}-${dataName}`).attr("d").split("C");
const dataLen = internal.filterRemoveNull(chart.data(dataName)[0].values).length;

expect(commands.length).to.be.equal(dataLen);
});

it("Should render the lines correctly when array data supplied", () => {
const target = chart.internal.main.select(`.${CLASS.chartLine}.${CLASS.target}-data2`);
const commands = target.select(`.${CLASS.line}-data2`).attr("d").split("C");
const dataLen = chart.data.values("data2").length;
// null data points, shouldn't be showing
internal.main.selectAll(`.${CLASS.circles}-${dataName} circle`).each(function(d, i) {
expect(+this.style.opacity).to.be.equal(i > 1 ? 1 : 0);
})
};

expect(commands.length).to.be.equal(dataLen);
it("Should render the lines correctly when array data supplied", () => {
checkLineLen("data1");
checkLineLen("data2");
});

it("should use cardinal interpolation by default", () => {
Expand Down
19 changes: 10 additions & 9 deletions src/data/data.js
View file
Expand Up @@ -18,7 +18,6 @@ import {
isBoolean,
isDefined,
isFunction,
isObject,
isObjectType,
isString,
isUndefined,
Expand Down Expand Up @@ -202,8 +201,8 @@ extend(ChartInternal.prototype, {

/**
* Get base value isAreaRangeType
* @param data
* @return {*}
* @param data Data object
* @return {Number}
* @private
*/
getBaseValue(data) {
Expand All @@ -212,8 +211,8 @@ extend(ChartInternal.prototype, {

// In case of area-range, data is given as: [low, mid, high] or {low, mid, high}
// will take the 'mid' as the base value
if ($$.isAreaRangeType(data)) {
value = isArray(value) ? value[1] : (isObject(value) ? value.mid : 0);
if (value && $$.isAreaRangeType(data)) {
value = $$.getAreaRangeData(data, "mid");
}

return value;
Expand Down Expand Up @@ -500,7 +499,7 @@ extend(ChartInternal.prototype, {
},

filterRemoveNull(data) {
return data.filter(d => isValue(d.value));
return data.filter(d => isValue(this.getBaseValue(d)));
},

filterByXDomain(targets, xDomain) {
Expand Down Expand Up @@ -706,12 +705,14 @@ extend(ChartInternal.prototype, {
},

getAreaRangeData(d, type) {
if (isArray(d.value)) {
const value = d.value;

if (isArray(value)) {
const index = ["high", "mid", "low"].indexOf(type);

return index === -1 ? 0 : d.value[index];
return index === -1 ? null : value[index];
}

return d.value[type];
return value[type];
}
});
17 changes: 9 additions & 8 deletions src/internals/ChartInternal.js
View file
Expand Up @@ -913,19 +913,20 @@ export default class ChartInternal {
}

initialOpacity(d) {
return d.value !== null &&
return this.getBaseValue(d) !== null &&
this.withoutFadeIn[d.id] ? "1" : "0";
}

initialOpacityForCircle(d) {
return d.value !== null &&
return this.getBaseValue(d) !== null &&
this.withoutFadeIn[d.id] ? this.opacityForCircle(d) : "0";
}

opacityForCircle(d) {
const opacity = this.config.point_show ? "1" : "0";

return isValue(d.value) ? (this.isBubbleType(d) || this.isScatterType(d) ? "0.5" : opacity) : "0";
return isValue(this.getBaseValue(d)) ?
(this.isBubbleType(d) || this.isScatterType(d) ? "0.5" : opacity) : "0";
}

opacityForText() {
Expand All @@ -941,12 +942,12 @@ export default class ChartInternal {

xv(d) {
const $$ = this;
let value = d.value;
let value = $$.getBaseValue(d);

if ($$.isTimeSeries()) {
value = $$.parseDate(d.value);
} else if ($$.isCategorized() && isString(d.value)) {
value = $$.config.axis_x_categories.indexOf(d.value);
value = $$.parseDate(value);
} else if ($$.isCategorized() && isString(value)) {
value = $$.config.axis_x_categories.indexOf(value);
}

return Math.ceil($$.x(value));
Expand All @@ -956,7 +957,7 @@ export default class ChartInternal {
const $$ = this;
const yScale = d.axis && d.axis === "y2" ? $$.y2 : $$.y;

return Math.ceil(yScale(d.value));
return Math.ceil(yScale($$.getBaseValue(d)));
}

subxx(d) {
Expand Down
7 changes: 4 additions & 3 deletions src/internals/grid.js
View file
Expand Up @@ -297,7 +297,8 @@ extend(ChartInternal.prototype, {
showXGridFocus(selectedData) {
const $$ = this;
const config = $$.config;
const dataToShow = selectedData.filter(d => d && isValue(d.value));
const isRotated = config.axis_rotated;
const dataToShow = selectedData.filter(d => d && isValue($$.getBaseValue(d)));
const focusEl = $$.main.selectAll(`line.${CLASS.xgridFocus}`);
const xx = $$.xx.bind($$);

Expand All @@ -313,8 +314,8 @@ extend(ChartInternal.prototype, {
focusEl
.style("visibility", "visible")
.data([dataToShow[0]])
.attr(config.axis_rotated ? "y1" : "x1", xx)
.attr(config.axis_rotated ? "y2" : "x2", xx);
.attr(isRotated ? "y1" : "x1", xx)
.attr(isRotated ? "y2" : "x2", xx);

$$.smoothLines(focusEl, "grid");
},
Expand Down
6 changes: 2 additions & 4 deletions src/internals/tooltip.js
View file
Expand Up @@ -76,9 +76,7 @@ extend(ChartInternal.prototype, {
let name;
let bgcolor;

const getRowValue = function(row) {
return $$.isAreaRangeType(row) ? $$.getAreaRangeData(row, "mid") : row.value;
};
const getRowValue = row => $$.getBaseValue(row);

if (order === null && config.data_groups.length) {
// for stacked data, order should aligned with the visually displayed data
Expand Down Expand Up @@ -227,7 +225,7 @@ extend(ChartInternal.prototype, {
const $$ = this;
const config = $$.config;
const forArc = $$.hasArcType(null, ["radar"]);
const dataToShow = selectedData.filter(d => d && isValue(d.value));
const dataToShow = selectedData.filter(d => d && isValue($$.getBaseValue(d)));
const positionFunction = config.tooltip_position || $$.tooltipPosition;

if (dataToShow.length === 0 || !config.tooltip_show) {
Expand Down
60 changes: 25 additions & 35 deletions src/shape/line.js
View file
Expand Up @@ -155,17 +155,16 @@ extend(ChartInternal.prototype, {
const xValue = d => (isSub ? $$.subxx : $$.xx).call($$, d);
const yValue = (d, i) => (config.data_groups.length > 0 ?
getPoints(d, i)[0][1] :
yScaleGetter.call($$, d.id)(
$$.isAreaRangeType(d) ? $$.getAreaRangeData(d, "mid") : d.value
));
yScaleGetter.call($$, d.id)($$.getBaseValue(d))
);

let line = d3Line();

line = isRotated ?
line.x(yValue).y(xValue) : line.x(xValue).y(yValue);

if (!lineConnectNull) {
line = line.defined(d => d.value !== null);
line = line.defined(d => $$.getBaseValue(d) !== null);
}

return d => {
Expand Down Expand Up @@ -388,15 +387,24 @@ extend(ChartInternal.prototype, {
const config = $$.config;
const lineConnectNull = config.line_connectNull;
const isRotated = config.axis_rotated;
const isGrouped = config.data_groups.length > 0;

const getPoints = $$.generateGetAreaPoints(areaIndices, isSub);
const yScaleGetter = isSub ? $$.getSubYScale : $$.getYScale;

const xValue = d => (isSub ? $$.subxx : $$.xx).call($$, d);
const value0 = (d, i) => (config.data_groups.length > 0 ?
const value0 = (d, i) => (isGrouped ?
getPoints(d, i)[0][1] :
yScaleGetter.call($$, d.id)($$.getAreaBaseValue(d.id)));
const value1 = (d, i) => (config.data_groups.length > 0 ?
yScaleGetter.call($$, d.id)(
$$.isAreaRangeType(d) ?
$$.getAreaRangeData(d, "high") : $$.getAreaBaseValue(d.id)
));
const value1 = (d, i) => (isGrouped ?
getPoints(d, i)[1][1] :
yScaleGetter.call($$, d.id)(d.value));
yScaleGetter.call($$, d.id)(
$$.isAreaRangeType(d) ?
$$.getAreaRangeData(d, "low") : d.value
));

return d => {
let values = lineConnectNull ? $$.filterRemoveNull(d.values) : d.values;
Expand All @@ -405,33 +413,20 @@ extend(ChartInternal.prototype, {
let path;

if ($$.isAreaType(d)) {
const isAreaRangeType = $$.isAreaRangeType(d);
let area = d3Area();

if (isRotated) {
if (isAreaRangeType) {
area = area.x0(d => yScaleGetter.call($$, d.id)($$.getAreaRangeData(d, "high")))
.x1(d => yScaleGetter.call($$, d.id)($$.getAreaRangeData(d, "low")))
.y(xValue);
} else {
area = area.x0(value0)
.x1(value1)
.y(xValue);
}
area = area.y(xValue)
.x0(value0)
.x1(value1);
} else {
if (isAreaRangeType) {
area = area.x(xValue)
.y0(d => yScaleGetter.call($$, d.id)($$.getAreaRangeData(d, "high")))
.y1(d => yScaleGetter.call($$, d.id)($$.getAreaRangeData(d, "low")));
} else {
area = area.x(xValue)
.y0(config.area_above ? 0 : value0)
.y1(value1);
}
area = area.x(xValue)
.y0(config.area_above ? 0 : value0)
.y1(value1);
}

if (!lineConnectNull) {
area = area.defined(d => d.value !== null);
area = area.defined(d => $$.getBaseValue(d) !== null);
}

if ($$.isStepType(d)) {
Expand Down Expand Up @@ -552,14 +547,9 @@ extend(ChartInternal.prototype, {
lineIndices = $$.getShapeIndices($$.isLineType);
getPoints = $$.generateGetLinePoints(lineIndices);

$$.circleY = function(d, i) {
return getPoints(d, i)[0][1];
};
$$.circleY = (d, i) => getPoints(d, i)[0][1];
} else {
$$.circleY = function(d) {
return $$.isAreaRangeType(d) ? $$.getYScale(d.id)($$.getAreaRangeData(d, "mid")) :
$$.getYScale(d.id)(d.value);
};
$$.circleY = d => $$.getYScale(d.id)($$.getBaseValue(d));
}
},

Expand Down

0 comments on commit cc1d4ee

Please sign in to comment.