fix(shape): fix incorrect shape offset

- fix on getting shape offset value - fix tooltip default value formatter function - remove .isOrderDesc(), .isOrderAsc() Fix #2187
naver · Jul 21, 2021 · d2be8c0 · d2be8c0
1 parent 1d2ce8f
commit d2be8c0
Show file tree

Hide file tree

Showing 9 changed files with 229 additions and 62 deletions.
diff --git a/src/ChartInternal/data/data.ts b/src/ChartInternal/data/data.ts
@@ -584,21 +584,6 @@ export default {
 		return this.checkValueInTargets(targets, v => v > 0);
 	},
 
-	_checkOrder(type: string): boolean {
-		const {config} = this;
-		const order = config.data_order;
-
-		return isString(order) && order.toLowerCase() === type;
-	},
-
-	isOrderDesc(): boolean {
-		return this._checkOrder("desc");
-	},
-
-	isOrderAsc(): boolean {
-		return this._checkOrder("asc");
-	},
-
 	/**
 	 * Sort targets data
 	 * @param {Array} targetsValue Target value
@@ -624,22 +609,24 @@ export default {
 	getSortCompareFn(isArc = false): Function | null {
 		const $$ = this;
 		const {config} = $$;
-		const orderAsc = $$.isOrderAsc();
-		const orderDesc = $$.isOrderDesc();
+		const order = config.data_order;
+		const orderAsc = /asc/i.test(order);
+		const orderDesc = /desc/i.test(order);
 		let fn;
 
 		if (orderAsc || orderDesc) {
+			const reducer = (p, c) => p + Math.abs(c.value);
+
 			fn = (t1, t2) => {
-				const reducer = (p, c) => p + Math.abs(c.value);
 				const t1Sum = t1.values.reduce(reducer, 0);
 				const t2Sum = t2.values.reduce(reducer, 0);
 
 				return isArc ?
 					(orderAsc ? t1Sum - t2Sum : t2Sum - t1Sum) :
 					(orderAsc ? t2Sum - t1Sum : t1Sum - t2Sum);
 			};
-		} else if (isFunction(config.data_order)) {
-			fn = config.data_order.bind($$.api);
+		} else if (isFunction(order)) {
+			fn = order.bind($$.api);
 		}
 
 		return fn || null;

diff --git a/src/ChartInternal/internals/class.ts b/src/ChartInternal/internals/class.ts
@@ -85,8 +85,9 @@ export default {
 	},
 
 	getTargetSelectorSuffix(targetId?: string | number): string {
-		return targetId || targetId === 0 ?
-			`-${targetId}`.replace(/[\s?!@#$%^&*()_=+,.<>'":;\[\]\/|~`{}\\]/g, "-") : "";
+		const targetStr = targetId || targetId === 0 ? `-${targetId}` : "";
+
+		return targetStr.replace(/([\s?!@#$%^&*()_=+,.<>'":;\[\]\/|~`{}\\])/g, "-");
 	},
 
 	selectorTarget(id: string, prefix?: string): string {

diff --git a/src/ChartInternal/internals/format.ts b/src/ChartInternal/internals/format.ts
@@ -23,23 +23,6 @@ function getFormat($$, typeValue: AxisType, v: number): number | string {
 }
 
 export default {
-	getYFormat(forArc: boolean): Function {
-		const $$ = this;
-		let {yFormat, y2Format} = $$;
-
-		if (forArc && !$$.hasType("gauge")) {
-			yFormat = $$.defaultArcValueFormat;
-			y2Format = $$.defaultArcValueFormat;
-		}
-
-		return function(v, ratio, id) {
-			const format = $$.axis && $$.axis.getId(id) === "y2" ?
-				y2Format : yFormat;
-
-			return format.call($$, v, ratio);
-		};
-	},
-
 	yFormat(v: number): number | string {
 		return getFormat(this, "y", v);
 	},
@@ -48,7 +31,21 @@ export default {
 		return getFormat(this, "y2", v);
 	},
 
-	defaultValueFormat(v): number | string {
+	/**
+	 * Get default value format function
+	 * @returns {Function} formatter function
+	 * @private
+	 */
+	getDefaultValueFormat(): Function {
+		const $$ = this;
+		const hasArc = $$.hasArcType();
+
+		return hasArc && !$$.hasType("gauge") ?
+			$$.defaultArcValueFormat :
+			$$.defaultValueFormat;
+	},
+
+	defaultValueFormat(v): number|string {
 		return isValue(v) ? +v : "";
 	},
 

diff --git a/src/ChartInternal/internals/tooltip.ts b/src/ChartInternal/internals/tooltip.ts
@@ -68,7 +68,7 @@ export default {
 			$el.tooltip.html($$.getTooltipHTML(
 				data,
 				$$.axis && $$.axis.getXAxisTickFormat(),
-				$$.getYFormat($$.hasArcType(null, ["radar"])),
+				$$.getDefaultValueFormat(),
 				$$.color
 			));
 
@@ -325,7 +325,6 @@ export default {
 		const $$ = this;
 		const {config, state, $el: {tooltip}} = $$;
 		const {bindto} = config.tooltip_contents;
-		const forArc = $$.hasArcType(null, ["radar"]);
 		const dataToShow = selectedData.filter(d => d && isValue($$.getBaseValue(d)));
 
 		if (!tooltip || dataToShow.length === 0 || !config.tooltip_show) {
@@ -346,7 +345,7 @@ export default {
 				.html($$.getTooltipHTML(
 					selectedData, // data
 					$$.axis ? $$.axis.getXAxisTickFormat() : $$.categoryName.bind($$), // defaultTitleFormat
-					$$.getYFormat(forArc), // defaultValueFormat
+					$$.getDefaultValueFormat(), // defaultValueFormat
 					$$.color // color
 				))
 				.style("display", null)

diff --git a/src/ChartInternal/shape/bar.ts b/src/ChartInternal/shape/bar.ts
@@ -29,7 +29,12 @@ export default {
 
 		const mainBarUpdate = $$.$el.main.select(`.${CLASS.chartBars}`)
 			.selectAll(`.${CLASS.chartBar}`)
-			.data(targets)
+			.data(
+				// remove
+				targets.filter(
+					v => !v.values.every(d => !isNumber(d.value))
+				)
+			)
 			.attr("class", d => classChartBar(d) + classFocus(d));
 
 		const mainBarEnter = mainBarUpdate.enter().append("g")
@@ -83,7 +88,7 @@ export default {
 
 		return [
 			(withTransition ? bar.transition(getRandom()) : bar)
-				.attr("d", drawFn)
+				.attr("d", d => d.value && drawFn(d))
 				.style("fill", this.color)
 				.style("opacity", null)
 		];
@@ -143,7 +148,6 @@ export default {
 	generateGetBarPoints(barIndices, isSub?: boolean): Function {
 		const $$ = this;
 		const {config} = $$;
-
 		const axis = isSub ? $$.axis.subX : $$.axis.x;
 		const barTargetsNum = $$.getIndicesMax(barIndices) + 1;
 		const barW = $$.getBarW("bar", axis, barTargetsNum);

diff --git a/src/ChartInternal/shape/shape.ts b/src/ChartInternal/shape/shape.ts
@@ -271,26 +271,29 @@ export default {
 		const {shapeOffsetTargets, indexMapByTargetId} = $$.getShapeOffsetData(typeFilter);
 
 		return (d, idx) => {
-			const ind = $$.getIndices(indices, d.id);
-			const scale = $$.getYScaleById(d.id, isSub);
-			const y0 = scale($$.getShapeYMin(d.id));
+			const {id, value, x} = d;
+			const ind = $$.getIndices(indices, id);
+			const scale = $$.getYScaleById(id, isSub);
+			const y0 = scale($$.getShapeYMin(id));
 
-			const dataXAsNumber = Number(d.x);
+			const dataXAsNumber = Number(x);
 			let offset = y0;
 
 			shapeOffsetTargets
-				.filter(t => t.id !== d.id)
+				.filter(t => t.id !== id)
 				.forEach(t => {
-					if (ind[t.id] === ind[d.id] && indexMapByTargetId[t.id] < indexMapByTargetId[d.id]) {
-						let row = t.rowValues[idx];
+					const {id: tid, rowValueMapByXValue, rowValues, values: tvalues} = t;
+
+					if (ind[tid] === ind[id] && indexMapByTargetId[tid] < indexMapByTargetId[id]) {
+						let row = rowValues[idx];
 
 						// check if the x values line up
 						if (!row || Number(row.x) !== dataXAsNumber) {
-							row = t.rowValueMapByXValue[dataXAsNumber];
+							row = rowValueMapByXValue[dataXAsNumber];
 						}
 
-						if (row && row.value * d.value >= 0) {
-							offset += scale(t.values[dataXAsNumber]) - y0;
+						if (row && row.value * value >= 0 && isNumber(tvalues[dataXAsNumber])) {
+							offset += scale(tvalues[dataXAsNumber]) - y0;
 						}
 					}
 				});

diff --git a/test/internals/data-spec.ts b/test/internals/data-spec.ts
@@ -270,10 +270,6 @@ describe("DATA", () => {
 			};
 		});
 
-		it("should return false in isOrderAsc and isOrderDesc functions", () => {
-			expect(chart.internal.isOrderAsc() || chart.internal.isOrderDesc()).to.be.equal(false);
-		});
-
 		it("set options data.order=desc", () => {
 			args.data.order = "desc";
 		});

diff --git a/test/internals/tooltip-spec.ts b/test/internals/tooltip-spec.ts
@@ -797,8 +797,8 @@ describe("TOOLTIP", function() {
 
 			// check for circle point shape
 			util.hoverChart(chart, undefined, {clientX: 292, clientY: 34});
-			
-			value = +chart.$.tooltip.select(`.${CLASS.tooltipName}-data1 .value`).text();
+
+			value = +chart.$.tooltip.select(`.${CLASS.tooltipName}-data1 .value`).html();
 
 			expect(value).to.be.equal(1000);
 		});