fix(scale): Fix non-shape's element positioned accurately

Previously positioning grid, tick, etc. elements coordination values are defined using Math.ceil. In some scales, this made inconsistency of shape with grid and other non-shape element. To fix this, remove the use of Math.ceil. Ref naver#3907
netil · Oct 24, 2024 · f6c6b63 · f6c6b63
1 parent 3febfd1
commit f6c6b63
Show file tree

Hide file tree

Showing 28 changed files with 226 additions and 228 deletions.
diff --git a/src/ChartInternal/Axis/Axis.ts b/src/ChartInternal/Axis/Axis.ts
@@ -779,7 +779,7 @@ class Axis {
 					$$.getXDomainMax($$.data.targets) + 1 + right
 				]);
 
-			tickOffset = Math.ceil((scale(1) - scale(0)) / 2);
+			tickOffset = (scale(1) - scale(0)) / 2;
 		}
 
 		return maxOverflow + tickOffset;

diff --git a/src/ChartInternal/Axis/AxisRenderer.ts b/src/ChartInternal/Axis/AxisRenderer.ts
@@ -93,7 +93,7 @@ export default class AxisRenderer {
 			$g = g;
 			this.__chart__ = scale1;
 
-			config.tickOffset = params.isCategory ? Math.ceil((scale1(1) - scale1(0)) / 2) : 0;
+			config.tickOffset = params.isCategory ? (scale1(1) - scale1(0)) / 2 : 0;
 
 			// update selection - data join
 			const path = g.selectAll(".domain").data([0]);
@@ -407,12 +407,10 @@ export default class AxisRenderer {
 			tickWidth = isLeftRight ? 95 : (
 				params.isCategory ?
 					(
-						Math.ceil(
-							params.isInverted ?
-								scale(ticks[0]) - scale(ticks[1]) :
-								scale(ticks[1]) - scale(ticks[0])
-						) - 12
-					) :
+						params.isInverted ?
+							scale(ticks[0]) - scale(ticks[1]) :
+							scale(ticks[1]) - scale(ticks[0])
+					) - 12 :
 					110
 			);
 		}

diff --git a/src/ChartInternal/Axis/AxisRendererHelper.ts b/src/ChartInternal/Axis/AxisRendererHelper.ts
@@ -80,7 +80,7 @@ export default class AxisRendererHelper {
 			selection.attr("transform", d => {
 				const x = scale(d);
 
-				return isValue(d) ? fn(Math.ceil(x)) : null;
+				return isValue(d) ? fn(x) : null;
 			});
 		};
 	}

diff --git a/src/ChartInternal/internals/grid.ts b/src/ChartInternal/internals/grid.ts
@@ -45,7 +45,7 @@ function smoothLines(el, type: string): void {
 			const g = d3Select(this);
 
 			["x1", "x2", "y1", "y2"]
-				.forEach(v => g.attr(v, Math.ceil(+g.attr(v))));
+				.forEach(v => g.attr(v, +g.attr(v)));
 		});
 	}
 }
@@ -138,7 +138,7 @@ export default {
 		const $$ = this;
 		const {axis, config, scale, state, $el: {grid, main}} = $$;
 		const isRotated = config.axis_rotated;
-		const pos = d => Math.ceil(scale.y(d));
+		const pos = d => scale.y(d);
 		const gridValues = axis.y.getGeneratedTicks(config.grid_y_ticks) ||
 			$$.scale.y.ticks(config.grid_y_ticks);
 

diff --git a/src/ChartInternal/internals/scale.ts b/src/ChartInternal/internals/scale.ts
@@ -103,14 +103,11 @@ export default {
 		/**
 		 * Get scaled value
 		 * @param {object} d Data object
-		 * @param {boolean} raw Get the raw value
 		 * @returns {number}
 		 * @private
 		 */
-		const scale = function(d: IDataRow, raw?: boolean): number {
-			const v = scaleValue(d) + offset();
-
-			return raw ? v : Math.ceil(v);
+		const scale = function(d: IDataRow): number {
+			return scaleValue(d) + offset();
 		};
 
 		// copy original scale methods
@@ -258,15 +255,15 @@ export default {
 			value = config.axis_x_categories.indexOf(value);
 		}
 
-		return Math.ceil(fn(value));
+		return fn(value);
 	},
 
 	yv(d: IGridData): number {
 		const $$ = this;
 		const {scale: {y, y2}} = $$;
 		const yScale = d.axis && d.axis === "y2" ? y2 : y;
 
-		return Math.ceil(yScale($$.getBaseValue(d)));
+		return yScale($$.getBaseValue(d));
 	},
 
 	subxx(d: IDataRow): number | null {

diff --git a/src/Plugin/stanford/Elements.ts b/src/Plugin/stanford/Elements.ts
@@ -163,14 +163,14 @@ export default class Elements {
 			value = config.axis_x_categories.indexOf(d.value);
 		}
 
-		return Math.ceil($$.scale.x(value));
+		return $$.scale.x(value);
 	}
 
 	yvCustom(d, xyValue): number {
 		const $$ = this;
 		const yScale = d.axis && d.axis === "y2" ? $$.scale.y2 : $$.scale.y;
 		const value = xyValue ? d[xyValue] : $$.getBaseValue(d);
 
-		return Math.ceil(yScale(value));
+		return yScale(value);
 	}
 }
diff --git a/test/api/axis-spec.ts b/test/api/axis-spec.ts
@@ -11,7 +11,7 @@ import {$AXIS} from "../../src/config/classes";
 describe("API axis", function() {
 	let chart;
 	let main;
-	const rx = /translate\((\d+),.*/;
+	const rx = /translate\(([^,]*),.*/;
 
 	beforeAll(() => {
 		return new Promise(resolve => {
@@ -115,7 +115,7 @@ describe("API axis", function() {
 
 			setTimeout(() => {
 				let tspan;
-
+				
 				// check for x max value
 				expect(xTickValue).to.be.below(+xAxisTick.getAttribute("transform").replace(rx, "$1"));
 

diff --git a/test/api/chart-spec.ts b/test/api/chart-spec.ts
@@ -62,7 +62,7 @@ describe("API chart", () => {
 			expect(chart.groups()[0].length).to.be.equal(chart.data().length);
 
 			// check for the bars were stacked
-			expect(util.getBBox(path).width).to.be.equal(barWidth * 2);
+			expect(util.getBBox(path).width).to.be.closeTo(barWidth * 2, 1);
 		});
 	});
 

diff --git a/test/api/export-spec.ts b/test/api/export-spec.ts
@@ -201,15 +201,15 @@ describe("API export", () => {
 
 			// pattern for local: preserveFontStyle=true
 			[
-				"i2iAoIcLvwVDzzAQUBmuodjmOmu+NeryqrOrq4jsyozKyLzy99vftNVcb343ou",
-				"JXSOKRvoLXeaeBnwyQ3q5TJIDtEMpaehdeQBLArBlL9P1Ni",
-				"iDzpkxgBoCd9qkExwC4O8qSCPVk3lyWshNuFqQAdJNm6rb+GVkH2CdOcenYIgj5Cy"
+				"UwUQECoIlWYZu6qgABsKvys3LDFCAAGmYQNocKRKQAATAiIVlMehQgAKbHluxJ",
+				"bAOgeAH1QlUcTZOjJOFPV7zN3zDlq/tQUvMb1kYAjNcQLZROAGxBtNTf0oIXkACY",
+				"CZAACZAACZAABSCfARIgARIgARIgARLIMQIUgDkWcHaXBEiABEiABEiABCgA"
 			],
 
 			// pattern for webdriverio
 			[
 				"nSetJ0vvOG1XcoZ8BQEeD3w9CCB6BGIkmCiAIze",
-				"BhTIwMPCQiOxewt4m8Xj8HxSAQVGnnXoQ0J7YGzDD",
+				"5Ku10DhdnT36vaRow3ZSgXoFLMsSYJMwL9XnRanBgYGBojufZVlvBCChjpwQMZJcsj",
 				"ANwDC7nf6iqhRcXno2cQP2wnXM4qpFnSLACzpHlfYysmAV1jezaWxGgmFrai6bp9Y6Ryp2AaM"
 			],
 
@@ -229,9 +229,9 @@ describe("API export", () => {
 
 			// pattern for CI: preserveFontStyle=true
 			[
-				"Y2qtWP1Xg3AYmsmlBQxSAnANJJZDP5y8H8Jkp4isBWDubzS4fbcPjIamzgX",
-				"Y9JUCsM6Jy+ZOENCezn8CehhUI917sy0C0BI05itQPV9yQz9yAiidIIEEE6AA",
-				"aWV7xEDDrkhf0fgX/bLgG8I6wWjMTYt7S5hUPATMJ8LVBquISlXg0MLWYtfrmGzn"
+				"MMcesnMiKAsgzhwTak0AtArgzgC+WwHEfALtR6FRltwPYGMDxAHYHYAA8B",
+				"2HTkHLCWAg4nrEehzQNDS997GQgCNORiBbCPDmWN4MpEACVRGoFYBrKz35t",
+				"gFcBOMOf8rUAftrP9AcCOMkvvxfA8hxSvQJglx51zgZwRQUAMMg5PQTg21nm"
 			]
 		];
 

diff --git a/test/api/flow-spec.ts b/test/api/flow-spec.ts
@@ -263,7 +263,7 @@ describe("API flow", () => {
 					const tick = this.internal.$el.axis.x.select(".tick");
 
 					expect(tick.text()).to.be.equal("17/01/11");
-					expect(tick.attr("transform")).to.be.equal("translate(6,0)");
+					expect(+tick.attr("transform").match(/\(([^,]+)/)[1]).to.be.closeTo(6, 1);
 
 					done(1);
 				}
@@ -450,12 +450,15 @@ describe("API flow", () => {
 				done() {
 					this.$.main.selectAll(`.${$LINE.chartLine} path`).each(function(d, i) {
 						const rect = this.getBoundingClientRect();
+						const pos = [rect.width, rect.x];
 						const expected = {
 							data1: [588, 46.5],
 							data2: [294, 340.5]
 						};
 
-						expect([rect.width, rect.x]).to.be.deep.equal(expected[d.id]);
+						expected[d.id].forEach((v, i) => {
+							expect(v).to.be.closeTo(pos[i], 1);
+						});
 					});
 
 					done(1);

diff --git a/test/api/grid-spec.ts b/test/api/grid-spec.ts
@@ -50,7 +50,7 @@ describe("API grid", function() {
 				grids.each(function (d, i) {
 					const y = +d3Select(this).select("line").attr("y1");
 					const text = d3Select(this).select("text").text();
-					const expectedY = Math.round(chart.internal.scale.y(expectedGrids[i].value));
+					const expectedY = chart.internal.scale.y(expectedGrids[i].value);
 					const expectedText = expectedGrids[i].text;
 
 					expect(y).to.be.equal(expectedY);
@@ -98,7 +98,7 @@ describe("API grid", function() {
 					grids.each(function(d, i) {
 						const x = +d3Select(this).select("line").attr("x1");
 						const text = d3Select(this).select("text").text();
-						const expectedX = Math.round(chart.internal.scale.x(expectedGrids[i].value));
+						const expectedX = chart.internal.scale.x(expectedGrids[i].value);
 						const expectedText = expectedGrids[i].text;
 
 						expect(x).to.be.equal(expectedX);
@@ -229,7 +229,7 @@ describe("API grid", function() {
 
 			const line = chart.internal.$el.gridLines.x.select("line");
 
-			expect(+line.attr("x1")).to.be.equal(298);
+			expect(+line.attr("x1")).to.be.equal(299.5);
 			expect(+line.attr("y2")).to.be.equal(426);
 		});
 	});

diff --git a/test/api/region-spec.ts b/test/api/region-spec.ts
@@ -242,7 +242,7 @@ describe("API regions", function() {
 
 			const rect = chart.internal.$el.region.list.select("rect").node().getBoundingClientRect();
 
-			expect(rect.width).to.be.equal(300);
+			expect(rect.width).to.be.equal(299.5);
 			expect(rect.height).to.be.equal(426);
 		});
 	});

diff --git a/test/api/subchart-spec.ts b/test/api/subchart-spec.ts
@@ -117,7 +117,7 @@ describe("API subchart", () => {
 					const currentPath =  this.internal.$el.subchart.line.attr("d");
 
 					expect(currentPath).to.be.not.equal(path);
-					expect(currentPath).to.be.equal("M6,38.694L300,55.083L594,5.917");
+					expect(currentPath).to.be.equal("M5.873,38.694L299.5,55.083L593.127,5.917");
 					done(1);
 				}
 			});

diff --git a/test/assets/util.ts b/test/assets/util.ts
@@ -125,8 +125,10 @@ function sandbox(obj: string | HTMLDivElement, prop?): HTMLDivElement {
 
 // test should executed from 'coverage:ci' command
 const isCI = process.env.NODE_ENV === "CI";
+const ceil = v => Math.ceil(v);
 
 export default {
+	ceil,
 	destroyAll,
 	doDrag,
 	fireEvent,

diff --git a/test/interactions/zoom-spec.ts b/test/interactions/zoom-spec.ts
@@ -392,7 +392,7 @@ describe("ZOOM", function() {
 			const getX = selector => chart.$.main.select(selector).node().getBoundingClientRect().x;
 
 			// when zoom in
-			chart.zoom([1,3]);
+			chart.zoom([1,3.5]);
 
 			setTimeout(() => {
 				expect(

diff --git a/test/internals/axis-spec.ts b/test/internals/axis-spec.ts
@@ -74,7 +74,7 @@ describe("AXIS", function() {
 
 			chart.$.main.selectAll(`.${$AXIS.axisX} .tick`).each(function(d, i) {
 				expect(
-					util.parseNum(this.getAttribute("transform").split(",")[0])
+					util.ceil(util.parseNum(this.getAttribute("transform").split(",")[0]))
 				).to.be.equal(expectedXPos[i]);
 			});
 		});
@@ -889,7 +889,7 @@ describe("AXIS", function() {
 							const text = d3Select(this);
 
 							expect(text.attr("x")).to.be.equal(expectedX);
-							expect(text.attr("y")).to.be.equal(expectedY);
+							expect(util.ceil(text.attr("y"))).to.be.equal(+expectedY);
 						});
 
 						if (i > 0) { // i === 0 should be checked in next test
@@ -943,7 +943,7 @@ describe("AXIS", function() {
 					const ticksText = chart.$.main.select(`.${$AXIS.axisX}`).selectAll("g.tick text");
 
 					ticksText.each(function() {
-						expect(+this.getAttribute("y")).to.be.equal(37);
+						expect(util.ceil(+this.getAttribute("y"))).to.be.equal(37);
 					});
 				});
 			});
@@ -2185,8 +2185,9 @@ describe("AXIS", function() {
 
 			chart.internal.state.eventReceiver.coords.forEach((d, idx) => {
 				const tick = d3Select(ticks[idx]);
+				const y = +tick.attr("transform").match(/,([^)]*)/)[1];
 
-				expect(tick.attr("transform")).to.be.equal(`translate(0,${d.y})`);
+				expect(y).to.closeTo(d.y, 1);
 			});
 		});
 
@@ -3200,8 +3201,10 @@ describe("AXIS", function() {
 		it("check if x axis min/max is not fitten.", () => {
 			const currWidth = chart.internal.state.current.width;
 
-			chart.$.main.selectAll(`.${$AXIS.axisX} .tick`).each(function(d, i) {
-				const xPos = +util.parseNum(this.getAttribute("transform")) / 10;
+			chart.internal.$el.axis.x.selectAll(".tick").each(function(d, i) {
+				const xPos = +util.parseNum(this.getAttribute("transform"));
+
+				console.log(xPos, this.getAttribute("transform"))
 
 				if (i === 0) { // check min
 					expect(xPos).to.be.below(0);
@@ -3468,7 +3471,7 @@ describe("AXIS", function() {
 
 		it("check the tick interval to be calculated based on the scale & ticks value", () => {
 			// tick's inteval shouldn't be based on the assumption of ticks having same interval
-			expect(chart.internal.axis.x.tickInterval()).to.be.equal(22);
+			expect(util.ceil(chart.internal.axis.x.tickInterval())).to.be.equal(23);
 
 			// bar widht's value should be set based on the scaled tick interval
 			expect(chart.$.bar.bars.node().getBoundingClientRect().width).to.be.closeTo(6, 1);

diff --git a/test/internals/axis-x-spec.ts b/test/internals/axis-x-spec.ts
@@ -216,7 +216,7 @@ describe("X AXIS", function() {
 
 				const {zoom} = chart.internal.scale;
 
-				expect(lines.attr("d")).to.be.equal("M1794,390.583L1196,227.858L597,323.579L-1,36.417L-599,275.718");
+				expect(lines.attr("d")).to.be.equal("M1797,390.583L1198,227.858L599,323.579L0,36.417L-599,275.718");
 
 				expect(zoom(2) - zoom(3)).to.be.closeTo(600, 5);
 

diff --git a/test/internals/data-spec.ts b/test/internals/data-spec.ts
@@ -4,7 +4,7 @@
  */
 /* eslint-disable */
 /* global describe, beforeEach, it, expect */
-import {beforeEach, beforeAll, afterAll, describe, expect, it} from "vitest";
+import {beforeEach, beforeAll, afterEach, afterAll, describe, expect, it} from "vitest";
 import {select as d3Select} from "d3-selection";
 import sinon from "sinon";
 import util from "../assets/util";
@@ -89,7 +89,7 @@ describe("DATA", () => {
 		});
 
 		it("should draw correctly", () => {
-			const expectedCx = {443: [98, 296, 492], 995: [98, 296, 492]};
+			const expectedCx = {443: [98, 294.5, 490], 995: [98, 294.5, 490]};
 			const expectedCy = {443: [194, 351, 36], 995: [391, 0, 351]};
 			const main = chart.$.main;
 
@@ -150,7 +150,7 @@ describe("DATA", () => {
 
 		it("should draw nested JSON correctly", () => {
 			const main = chart.$.main;
-			const expectedCx = [98, 296, 492];
+			const expectedCx = [98, 294.5, 490];
 			const expectedCy = {
 				443: [181, 326, 36],
 				995: [362, 0, 326],
@@ -623,7 +623,7 @@ describe("DATA", () => {
 		});
 
 		it("line path should rendered correctly.", () => {
-			expect(chart.$.line.lines.attr("d")).to.be.equal("M594,390.583L6,36.417L300,390.583");
+			expect(chart.$.line.lines.attr("d")).to.be.equal("M593.127,390.583L5.873,36.417L299.5,390.583");
 		});
 
 		it("check for tooltip show", () => {