fix(text): Fix data label y position when all data are 0

- Update the condition for y position when all data values are 0 - add .getBoundingRect() to util, to cache element's .getBoundingClientRect() value Fix #1026
naver · Aug 19, 2019 · 90b6949 · 90b6949
1 parent 9a5d87b
commit 90b6949
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 14 deletions.
diff --git a/spec/internals/data-spec.js b/spec/internals/data-spec.js
@@ -926,7 +926,7 @@ describe("DATA", () => {
 				});
 
 				it("should locate labels above each data point", () => {
-					const expectedYs = [67, 49, 67, 423];
+					const expectedYs = [68, 50, 68, 423];
 					const expectedXs = [75, 225, 374, 524];
 
 					chart.internal.main.selectAll(`.${CLASS.texts}-data1 text`)
@@ -1524,6 +1524,44 @@ describe("DATA", () => {
 				}, 500);
 			});
 		});
+
+		describe("when all data values are 0", () => {
+			before(() => {
+				args = {
+					data: {
+						columns: [
+							["data1", 0, 0, 0, 0],
+						],
+						labels: true
+					},
+					axis: {
+						y: {
+							min: 0
+						}
+					}
+				};
+			});
+
+			it("label text should locate above the data points", () => {
+				const texts = chart.$.text.texts.nodes();
+
+				chart.$.line.circles.each(function(d, i) {
+					expect(+this.getAttribute("cy")).to.be.above(+texts[i].getAttribute("y"));
+				});
+			});
+
+			it("set options axis.rotated=true", () => {
+				args.axis.rotated = true;
+			});
+
+			it("label text should locate above the data points", () => {
+				const texts = chart.$.text.texts.nodes();
+
+				chart.$.line.circles.each(function(d, i) {
+					expect(+this.getAttribute("cx")).to.be.below(+texts[i].getAttribute("x"));
+				});
+			});
+		});
 	});
 
 	describe("inner functions", () => {

diff --git a/src/internals/text.js b/src/internals/text.js
@@ -8,7 +8,7 @@ import {
 } from "d3-selection";
 import ChartInternal from "./ChartInternal";
 import CLASS from "../config/classes";
-import {capitalize, extend, getRandom, isNumber, isObject, isString} from "./util";
+import {capitalize, extend, getBoundingRect, getRandom, isNumber, isObject, isString} from "./util";
 
 extend(ChartInternal.prototype, {
 	/**
@@ -148,7 +148,7 @@ extend(ChartInternal.prototype, {
 				.classed(className, true)
 				.text(text)
 				.call(v => {
-					rect = v.node().getBoundingClientRect();
+					rect = getBoundingRect(v.node());
 				})
 				.remove();
 
@@ -200,7 +200,7 @@ extend(ChartInternal.prototype, {
 		const isRotated = config.axis_rotated;
 
 		if (config.data_labels.centered && $$.isBarType(d)) {
-			const rect = textElement.getBoundingClientRect();
+			const rect = getBoundingRect(textElement);
 			const isPositive = d.value >= 0;
 
 			if (isRotated) {
@@ -249,7 +249,9 @@ extend(ChartInternal.prototype, {
 		// show labels regardless of the domain if value is null
 		if (d.value === null) {
 			if (xPos > $$.width) {
-				xPos = $$.width - textElement.getBoundingClientRect().width;
+				const {width} = getBoundingRect(textElement);
+
+				xPos = $$.width - width;
 			} else if (xPos < 0) {
 				xPos = 4;
 			}
@@ -275,7 +277,7 @@ extend(ChartInternal.prototype, {
 		const config = $$.config;
 		const isRotated = config.axis_rotated;
 		const r = config.point_r;
-		const rect = textElement.getBoundingClientRect();
+		const rect = getBoundingRect(textElement);
 		let baseY = 3;
 		let yPos;
 
@@ -288,14 +290,8 @@ extend(ChartInternal.prototype, {
 				baseY += config.point_r / 2.3;
 			}
 
-			if (d.value < 0 || (d.value === 0 && !$$.hasPositiveValue)) {
-				yPos += rect.height;
-
-				if ($$.isBarType(d)) {
-					yPos -= baseY;
-				} else if (!$$.isBarType(d)) {
-					yPos += baseY;
-				}
+			if (d.value < 0 || (d.value === 0 && !$$.hasPositiveValue && $$.hasNegativeValue)) {
+				yPos += rect.height + ($$.isBarType(d) ? -baseY : baseY);
 			} else {
 				let diff = -baseY * 2;
 

diff --git a/src/internals/util.js b/src/internals/util.js
@@ -159,6 +159,9 @@ const getBrushSelection = ctx => {
 	return selection;
 };
 
+// Get boundingClientRect. Cache the evaluated value once it was called.
+const getBoundingRect = node => node.rect || (node.rect = node.getBoundingClientRect());
+
 // retrun random number
 const getRandom = (asStr = true) => Math.random() + (asStr ? "" : 0);
 
@@ -414,6 +417,7 @@ export {
 	emulateEvent,
 	extend,
 	getBrushSelection,
+	getBoundingRect,
 	getCssRules,
 	getMinMax,
 	getOption,