fix: marker positioning (tradingview#1538)

* fix: marker positioning
31031969 · Mar 11, 2024 · 3f66b76 · 3f66b76
1 parent f1fceb5
commit 3f66b76
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 8 deletions.
diff --git a/.size-limit.js b/.size-limit.js
@@ -4,21 +4,21 @@ module.exports = [
 	{
 		name: 'CJS',
 		path: 'dist/lightweight-charts.production.cjs',
-		limit: '47.98 KB',
+		limit: '48.13 KB',
 	},
 	{
 		name: 'ESM',
 		path: 'dist/lightweight-charts.production.mjs',
-		limit: '47.91 KB',
+		limit: '48.07 KB',
 	},
 	{
 		name: 'Standalone-ESM',
 		path: 'dist/lightweight-charts.standalone.production.mjs',
-		limit: '49.63 KB',
+		limit: '49.77 KB',
 	},
 	{
 		name: 'Standalone',
 		path: 'dist/lightweight-charts.standalone.production.js',
-		limit: '49.67 KB',
+		limit: '49.82 KB',
 	},
 ];
diff --git a/src/model/price-scale.ts b/src/model/price-scale.ts
@@ -946,7 +946,7 @@ export class PriceScale {
 					const margins = autoScaleInfo.margins();
 					if (margins !== null) {
 						marginAbove = Math.max(marginAbove, margins.above);
-						marginBelow = Math.max(marginAbove, margins.below);
+						marginBelow = Math.max(marginBelow, margins.below);
 					}
 				}
 			}

diff --git a/src/views/pane/series-markers-pane-view.ts b/src/views/pane/series-markers-pane-view.ts
@@ -131,14 +131,16 @@ export class SeriesMarkersPaneView implements IUpdatablePaneView {
 	}
 
 	public autoScaleMargins(): AutoScaleMargins | null {
-		if (this._autoScaleMarginsInvalidated) {
+		if (this._autoScaleMarginsInvalidated && this._dataInvalidated) {
 			if (this._series.indexedMarkers().length > 0) {
 				const barSpacing = this._model.timeScale().barSpacing();
 				const shapeMargin = calculateShapeMargin(barSpacing);
 				const marginsAboveAndBelow = calculateShapeHeight(barSpacing) * 1.5 + shapeMargin * 2;
+				const position = this._hasAllMarkerSamePosition();
+
 				this._autoScaleMargins = {
-					above: marginsAboveAndBelow as Coordinate,
-					below: marginsAboveAndBelow as Coordinate,
+					above: position === 'belowBar' ? 0 : marginsAboveAndBelow,
+					below: position === 'aboveBar' ? 0 : marginsAboveAndBelow,
 				};
 			} else {
 				this._autoScaleMargins = null;
@@ -149,6 +151,14 @@ export class SeriesMarkersPaneView implements IUpdatablePaneView {
 
 		return this._autoScaleMargins;
 	}
+	protected _hasAllMarkerSamePosition(): string|null {
+		const markers = this._series.indexedMarkers();
+		const markersPositions = markers.every((i: InternalSeriesMarker<TimePointIndex>) => i.position === markers[0].position);
+		if (markersPositions) {
+			return markers[0].position;
+		}
+		return null;
+	}
 
 	protected _makeValid(): void {
 		const priceScale = this._series.priceScale();

diff --git a/tests/e2e/graphics/test-cases/series-markers/series-markers-all-above.js b/tests/e2e/graphics/test-cases/series-markers/series-markers-all-above.js
@@ -0,0 +1,30 @@
+function generateData() {
+	const res = [];
+	const time = new Date(Date.UTC(2018, 0, 1, 0, 0, 0, 0));
+	for (let i = 0; i < 30; ++i) {
+		res.push({
+			time: time.getTime() / 1000,
+			value: i,
+		});
+
+		time.setUTCDate(time.getUTCDate() + 1);
+	}
+	return res;
+}
+
+function runTestCase(container) {
+	const chart = window.chart = LightweightCharts.createChart(container);
+
+	const mainSeries = chart.addHistogramSeries();
+
+	const data = generateData();
+	mainSeries.setData(data);
+
+	const markers = [
+		{ time: data[0].time, position: 'aboveBar', color: 'red', shape: 'arrowUp' },
+		{ time: data[1].time, position: 'aboveBar', color: 'red', shape: 'arrowUp' },
+
+	];
+
+	mainSeries.setMarkers(markers);
+}
diff --git a/tests/e2e/graphics/test-cases/series-markers/series-markers-all-below.js b/tests/e2e/graphics/test-cases/series-markers/series-markers-all-below.js
@@ -0,0 +1,29 @@
+function generateData() {
+	const res = [];
+	const time = new Date(Date.UTC(2018, 0, 1, 0, 0, 0, 0));
+	for (let i = 0; i < 30; ++i) {
+		res.push({
+			time: time.getTime() / 1000,
+			value: i,
+		});
+
+		time.setUTCDate(time.getUTCDate() + 1);
+	}
+	return res;
+}
+
+function runTestCase(container) {
+	const chart = window.chart = LightweightCharts.createChart(container);
+
+	const mainSeries = chart.addHistogramSeries();
+
+	const data = generateData();
+	mainSeries.setData(data);
+
+	const markers = [
+		{ time: data[data.length - 3].time, position: 'belowBar', color: 'red', shape: 'arrowUp' },
+		{ time: data[data.length - 2].time, position: 'belowBar', color: 'red', shape: 'arrowUp' },
+	];
+
+	mainSeries.setMarkers(markers);
+}
diff --git a/tests/e2e/graphics/test-cases/series-markers/series-markers-all-inbar.js b/tests/e2e/graphics/test-cases/series-markers/series-markers-all-inbar.js
@@ -0,0 +1,29 @@
+function generateData() {
+	const res = [];
+	const time = new Date(Date.UTC(2018, 0, 1, 0, 0, 0, 0));
+	for (let i = 0; i < 30; ++i) {
+		res.push({
+			time: time.getTime() / 1000,
+			value: i,
+		});
+
+		time.setUTCDate(time.getUTCDate() + 1);
+	}
+	return res;
+}
+
+function runTestCase(container) {
+	const chart = window.chart = LightweightCharts.createChart(container);
+
+	const mainSeries = chart.addHistogramSeries();
+
+	const data = generateData();
+	mainSeries.setData(data);
+
+	const markers = [
+		{ time: data[data.length - 3].time, position: 'inBar', color: 'red', shape: 'arrowUp' },
+		{ time: data[data.length - 2].time, position: 'inBar', color: 'red', shape: 'arrowUp' },
+	];
+
+	mainSeries.setMarkers(markers);
+}