fix(size): Fix legend overflows with padding fit mode

On getting legend's height, make to use internal legend height value when padding fit mode is used Fix #3872
naver · Aug 30, 2024 · 8357d11 · 8357d11
1 parent 5c87363
commit 8357d11
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 17 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -31,7 +31,7 @@ jobs:
       run: npm run lint
 
     - name: Run Test
-      run: npm run coverage
+      run: npm run coverage:ci
 
     - name: Coveralls Parallel
       uses: coverallsapp/github-action@master

diff --git a/package.json b/package.json
@@ -41,11 +41,13 @@
 		"loc": "cloc --by-file src",
 		"test": "vitest",
 		"coverage": "vitest run",
+		"coverage:ci": "cross-env NODE_ENV=CI npm run coverage",
 		"coveralls": "cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js",
 		"jsdoc": "node ./config/jsdoc.js",
 		"jsdoc:cmd": "jsdoc -c jsdoc.json",
 		"lint-staged": "lint-staged --config ./config/.lintstagedrc.json",
-		"prepare": "husky install"
+		"prepare": "husky install",
+		"print": ""
 	},
 	"sideEffects": [
 		"dist/**/*.css",

diff --git a/src/ChartInternal/internals/legend.ts b/src/ChartInternal/internals/legend.ts
@@ -294,14 +294,15 @@ export default {
 		const $$ = this;
 		const {current, isLegendRight, legendItemHeight, legendStep} = $$.state;
 		const isFitPadding = $$.config.padding?.mode === "fit";
-
-		return $$.config.legend_show ?
+		const height = $$.config.legend_show ?
 			(
 				isLegendRight ? current.height : (
-					isFitPadding ? 10 : Math.max(20, legendItemHeight)
+					Math.max(isFitPadding ? 10 : 20, legendItemHeight)
 				) * (legendStep + 1)
 			) :
 			0;
+
+		return height;
 	},
 
 	/**

diff --git a/src/ChartInternal/internals/size.ts b/src/ChartInternal/internals/size.ts
@@ -255,7 +255,7 @@ export default {
 				padding += 1;
 			}
 		}
-
+		// console.log(type, padding + (axisSize * axesLen) - gap)
 		return padding + (axisSize * axesLen) - gap;
 	},
 

diff --git a/test/assets/util.ts b/test/assets/util.ts
@@ -123,6 +123,9 @@ function sandbox(obj: string | HTMLDivElement, prop?): HTMLDivElement {
 	return document.body.appendChild(tmp);
 }
 
+// test should executed from 'coverage:ci' command
+const isCI = process.env.NODE_ENV === "CI";
+
 export default {
 	destroyAll,
 	doDrag,
@@ -131,6 +134,7 @@ export default {
 	getBBox,
 	hexToRgb,
 	hoverChart,
+	isCI,
 	parseNum,
 	parseSvgPath,
 	print,

diff --git a/test/internals/padding-spec.ts b/test/internals/padding-spec.ts
@@ -436,16 +436,18 @@ describe("PADDING", () => {
 		});
 
 		describe("non-rotated axis", () => {
+			const bottom = 37 - (util.isCI ? 1 : 0);
+
 			it("outer y axis with legend", () => {
-				deepEqual({top: 0, right: 2, bottom: 30, left: 40.59375});
+				deepEqual({top: 0, right: 2, bottom, left: 40.59375});
 			});
 
 			it("set options: y2.show=true", () => {
 				args.axis.y2.show = true;
 			});
 
 			it("when y/y2 axes are displyed", () => {
-				deepEqual({top: 0, right: 40.59375, bottom: 30, left: 40.59375});
+				deepEqual({top: 0, right: 40.59375, bottom, left: 40.59375});
 			});
 
 			it("set options: axis.y.label", () => {
@@ -456,7 +458,7 @@ describe("PADDING", () => {
 			});
 
 			it("y axis with outer label text", () => {
-				deepEqual({top: 0, right: 40.59375, bottom: 30, left: 60.59375});
+				deepEqual({top: 0, right: 40.59375, bottom, left: 60.59375});
 			});
 
 			it("set options: axis.y2.label", () => {
@@ -467,39 +469,39 @@ describe("PADDING", () => {
 			});
 
 			it("y/y2 axes with outer label text", () => {
-				deepEqual({top: 0, right: 60.59375, bottom: 30, left: 60.59375});
+				deepEqual({top: 0, right: 60.59375, bottom, left: 60.59375});
 			});
 
 			it("set options: axis.y.inner=true", () => {
 				args.axis.y.inner = true;
 			});
 
 			it("inner y axis with outer label text", () => {
-				deepEqual({top: 0, right: 60.59375, bottom: 30, left: 20});
+				deepEqual({top: 0, right: 60.59375, bottom, left: 20});
 			});
 
 			it("set options: axis.y2.inner=true", () => {
 				args.axis.y2.inner = true;
 			});
 
 			it("inner y2 axis with outer label text", () => {
-				deepEqual({top: 0, right: 22, bottom: 30, left: 20});
+				deepEqual({top: 0, right: 22, bottom, left: 20});
 			});
 
 			it("set options: axis.y.label = {}", () => {
 				args.axis.y.label = {};
 			});
 
 			it("inner y axis without outer label text", () => {
-				deepEqual({top: 0, right: 22, bottom: 30, left: 0});
+				deepEqual({top: 0, right: 22, bottom, left: 0});
 			});
 
 			it("set options: axis.y2.label = {}", () => {
 				args.axis.y2.label = {};
 			});
 
 			it("inner y/y2 axes without outer label text", () => {
-				deepEqual({top: 0, right: 2, bottom: 30, left: 0});
+				deepEqual({top: 0, right: 2, bottom, left: 0});
 			});
 
 			it("set options: legend.show=false", () => {
@@ -546,7 +548,7 @@ describe("PADDING", () => {
 			});
 
 			it("when y is shown, y2 hidden and padding.right=0", () => {
-				deepEqual({top: 0, right: 2, bottom: 30, left: 28.359375});
+				deepEqual({top: 0, right: 2, bottom, left: 28.359375});
 			});
 
 			it("set options", () => {
@@ -664,5 +666,50 @@ describe("PADDING", () => {
 				deepEqual({top: 0, right: 2, bottom: 1, left: 16.125});
 			});
 		});
+
+		describe("pie", () => {
+			beforeAll(() => {
+				args = {
+					data: {
+						columns: [
+							["0:0", 97865], ["0:1", 54254], ["0:2", 331183], ["0:3", 82231], ["0:4", 20017], ["0:5", 56694], ["0:6", 14797], ["0:7", 44214], ["0:8", 54179], ["0:9", 136321], ["0:10", 1270], ["0:11", 707], ["0:12", 274], ["0:13", 5428], ["0:14", 5368], ["0:15", 187099]
+						],
+						names: {
+						  "0:0": "Protected dug well",
+						  "0:1": "Unprotected dug well",
+						  "0:2": "Borehole or tubewell",
+						  "0:3": "Protected spring",
+						  "0:4": "Unprotected spring",
+						  "0:5": "Rainwater",
+						  "0:6": "Surface water",
+						  "0:7": "Piped into dwelling",
+						  "0:8": "Piped into yard/plot",
+						  "0:9": "Piped into public tap / standpipe / basin",
+						  "0:10": "Bottled water",
+						  "0:11": "Tanker truck",
+						  "0:12": "Cart with small tank/drum",
+						  "0:13": "Other",
+						  "0:14": "Water kiosk",
+						  "0:15": "None"
+						},
+						type: "pie"
+					},
+					size: {
+						width: 680,
+						height: 460
+					},
+					padding: {
+						mode: "fit"
+					}
+				}
+			});
+
+			it("should legend stay inside of the container", () => {
+				const {current: {height}} = chart.internal.state;
+				const rect = chart.$.legend.node().getBoundingClientRect();
+
+				expect(rect.y + rect.height <= height);
+			});
+		});
 	});
 });
diff --git a/test/shape/arc-needle-spec.ts b/test/shape/arc-needle-spec.ts
@@ -73,8 +73,6 @@ describe("SHAPE ARC: NEEDLE option", () => {
 			const {$el: {arcs, needle}} = chart.internal;
 			const rx = /M-5 20 A0 0 0 0 0 5 20 L2\.5 -168\.\d+ A1 1 0 0 0 -2\.5 -168\.\d+ L-5 20 Z/;
 
-util.print.arg(args)
-console.log(needle.attr("d"))
 			expect(rx.test(needle.attr("d"))).to.be.true;
 			expect(getDegree(needle.style("transform"))).to.equal(118.8);
 			expect(needle.style("fill")).to.equal("red");
-Original file line number
+Diff line change
@@ Expand Up / @@ -255,7 +255,7 @@ export default { @@
     				padding += 1;
     			}
     		}
+    		// console.log(type, padding + (axisSize * axesLen) - gap)
     		return padding + (axisSize * axesLen) - gap;
     	},
@@ Expand Down @@