fix(shape.line): Correct getting shape object (#47)

when axis x value is Date type, changed to compare with time numeric value, due to Date type comparison doesn't work as expected. Fix #10 Close #47
naver · Jul 7, 2017 · b2a0acf · b2a0acf
1 parent 676b1e5
commit b2a0acf
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 26 deletions.
diff --git a/spec/shape.line-spec.js b/spec/shape.line-spec.js
@@ -146,4 +146,45 @@ describe("SHAPE LINE", () => {
 			expect(to).to.be.equal("cardinal");
 		});
 	});
+
+	describe("timeseries stacked area when line.connectNull=true", () => {
+		before(() => {
+			args = {
+				data: {
+					columns: [
+						["timestamps", 1495584000000, 1495605600000, 1495627200000, 1495648800000, 1495670400000],
+						["data1", 300, 150,  null, 120, 140],
+						["data2", 100, null, 200,  100, 100],
+						["data3", 100, 160,  200,  null, 50]
+					],
+					x: "timestamps",
+					order: "asc",
+					type: "area",
+					groups: [
+						["data1", "data2", "data3"]
+					]
+				},
+				line: {
+					connectNull: true
+				},
+				axis: {
+					x: {
+						type: "timeseries",
+						tick: {
+							format: "%Y-%m-%d"
+						}
+					}
+				}
+			};
+		});
+
+		it("check for line path data count", () => {
+			chart.internal.main.selectAll("path.bb-line").each(function(d) {
+				const line = d3.select(this);
+
+				// it should have 4 lines
+				expect(line.attr("d").split("L").length).to.be.equal(4);
+			});
+		});
+	});
 });
diff --git a/src/axis/Axis.js b/src/axis/Axis.js
@@ -164,9 +164,7 @@ export default class Axis {
 			}
 		}
 
-		return isFunction(format) ? function(v) {
-			return format.call($$, v);
-		} : format;
+		return isFunction(format) ? v => format.call($$, v) : format;
 	}
 
 	getTickValues(tickValues, axis) {

diff --git a/src/data/data.js b/src/data/data.js
@@ -294,16 +294,19 @@ extend(ChartInternal.prototype, {
 		return this.checkValueInTargets(targets, v => v > 0);
 	},
 
-	isOrderDesc() {
+	_checkOrder(type) {
 		const config = this.config;
 
-		return typeof(config.data_order) === "string" && config.data_order.toLowerCase() === "desc";
+		return typeof(config.data_order) === "string" &&
+			config.data_order.toLowerCase() === type;
 	},
 
-	isOrderAsc() {
-		const config = this.config;
+	isOrderDesc() {
+		return this._checkOrder("desc");
+	},
 
-		return typeof(config.data_order) === "string" && config.data_order.toLowerCase() === "asc";
+	isOrderAsc() {
+		return this._checkOrder("asc");
 	},
 
 	orderTargets(targets) {
@@ -323,6 +326,7 @@ extend(ChartInternal.prototype, {
 		} else if (isFunction(config.data_order)) {
 			targets.sort(config.data_order);
 		} // TODO: accept name array for order
+
 		return targets;
 	},
 
@@ -343,14 +347,11 @@ extend(ChartInternal.prototype, {
 	},
 
 	hasDataLabel() {
-		const config = this.config;
+		const dataLabels = this.config.data_labels;
+		const type = typeof dataLabels;
 
-		if (typeof config.data_labels === "boolean" && config.data_labels) {
-			return true;
-		} else if (typeof config.data_labels === "object" && notEmpty(config.data_labels)) {
-			return true;
-		}
-		return false;
+		return (type === "boolean" && dataLabels) ||
+			(type === "object" && notEmpty(dataLabels));
 	},
 
 	getDataLabelLength(min, max, key) {
@@ -367,6 +368,7 @@ extend(ChartInternal.prototype, {
 				lengths[i] = this.getBoundingClientRect()[key] * paddingCoef;
 			})
 			.remove();
+
 		return lengths;
 	},
 

diff --git a/src/internals/ChartInternal.js b/src/internals/ChartInternal.js
@@ -807,7 +807,7 @@ export default class ChartInternal {
 	}
 
 	isCategorized() {
-		return this.config.axis_x_type.indexOf("categor") >= 0;
+		return this.config.axis_x_type.indexOf("category") >= 0;
 	}
 
 	isCustomX() {

diff --git a/src/internals/shape.js b/src/internals/shape.js
@@ -98,8 +98,12 @@ extend(ChartInternal.prototype, {
 					if (typeof values[i] === "undefined" || +values[i].x !== +d.x) {  // "+" for timeseries
 						// if not, try to find the value that does line up
 						i = -1;
+
 						values.forEach((v, j) => {
-							if (v.x === d.x) {
+							const x1 = v.x.constructor === Date ? +v.x : v.x;
+							const x2 = d.x.constructor === Date ? +d.x : d.x;
+
+							if (x1 === x2) {
 								i = j;
 							}
 						});
@@ -110,6 +114,7 @@ extend(ChartInternal.prototype, {
 					}
 				}
 			});
+
 			return offset;
 		};
 	},

diff --git a/src/internals/shape.line.js b/src/internals/shape.line.js
@@ -103,25 +103,27 @@ extend(ChartInternal.prototype, {
 	generateDrawLine(lineIndices, isSub) {
 		const $$ = this;
 		const config = $$.config;
+		const lineConnectNull = config.line_connectNull;
+		const axisRotated = config.axis_rotated;
 		const getPoints = $$.generateGetLinePoints(lineIndices, isSub);
 		const yScaleGetter = isSub ? $$.getSubYScale : $$.getYScale;
 		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)(d.value));
+
 		let line = d3Line();
 
-		line = config.axis_rotated ?
+		line = axisRotated ?
 			line.x(yValue).y(xValue) : line.x(xValue).y(yValue);
 
-		if (!config.line_connectNull) {
+		if (!lineConnectNull) {
 			line = line.defined(d => d.value !== null);
 		}
 
 		return d => {
 			const x = isSub ? $$.x : $$.subX;
 			const y = yScaleGetter.call($$, d.id);
-			let values = config.line_connectNull ?
-				$$.filterRemoveNull(d.values) : d.values;
+			let values = lineConnectNull ? $$.filterRemoveNull(d.values) : d.values;
 			let x0 = 0;
 			let y0 = 0;
 			let path;
@@ -142,7 +144,7 @@ extend(ChartInternal.prototype, {
 					y0 = y(values[0].value);
 				}
 
-				path = config.axis_rotated ? `M ${y0} ${x0}` : `M ${x0} ${y0}`;
+				path = axisRotated ? `M ${y0} ${x0}` : `M ${x0} ${y0}`;
 			}
 
 			return path || "M 0 0";
@@ -329,29 +331,32 @@ extend(ChartInternal.prototype, {
 	generateDrawArea(areaIndices, isSub) {
 		const $$ = this;
 		const config = $$.config;
+		const lineConnectNull = config.line_connectNull;
+		const axisRotated = config.axis_rotated;
 		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 ?
 			getPoints(d, i)[0][1] : yScaleGetter.call($$, d.id)($$.getAreaBaseValue(d.id)));
 		const value1 = (d, i) => (config.data_groups.length > 0 ?
 			getPoints(d, i)[1][1] : yScaleGetter.call($$, d.id)(d.value));
+
 		let area = d3Area();
 
-		area = config.axis_rotated ?
+		area = axisRotated ?
 			area.x0(value0)
 				.x1(value1)
 				.y(xValue) :
 			area.x(xValue)
 				.y0(config.area_above ? 0 : value0)
 				.y1(value1);
 
-		if (!config.line_connectNull) {
+		if (!lineConnectNull) {
 			area = area.defined(d => d.value !== null);
 		}
 
 		return d => {
-			let values = config.line_connectNull ? $$.filterRemoveNull(d.values) : d.values;
+			let values = lineConnectNull ? $$.filterRemoveNull(d.values) : d.values;
 			let x0 = 0;
 			let y0 = 0;
 			let path;
@@ -368,7 +373,7 @@ extend(ChartInternal.prototype, {
 					y0 = $$.getYScale(d.id)(values[0].value);
 				}
 
-				path = config.axis_rotated ? `M ${y0} ${x0}` : `M ${x0} ${y0}`;
+				path = axisRotated ? `M ${y0} ${x0}` : `M ${x0} ${y0}`;
 			}
 
 			return path || "M 0 0";