From 7ece2ea8715d2b6b9c9fdd8dc309bca1dfa92be5 Mon Sep 17 00:00:00 2001 From: Mark Silverwood Date: Thu, 29 Jun 2023 16:55:30 +0100 Subject: [PATCH 1/5] fix percentage mode price scale bug --- .size-limit.js | 8 ++++---- src/model/price-range-impl.ts | 13 +++++++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/.size-limit.js b/.size-limit.js index 7512fb3e78..6ffa48b19b 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -4,21 +4,21 @@ module.exports = [ { name: 'CJS', path: 'dist/lightweight-charts.production.cjs', - limit: '46.94 KB', + limit: '46.96 KB', }, { name: 'ESM', path: 'dist/lightweight-charts.production.mjs', - limit: '46.86 KB', + limit: '46.89 KB', }, { name: 'Standalone-ESM', path: 'dist/lightweight-charts.standalone.production.mjs', - limit: '48.56 KB', + limit: '48.58 KB', }, { name: 'Standalone', path: 'dist/lightweight-charts.standalone.production.js', - limit: '48.61 KB', + limit: '48.63 KB', }, ]; diff --git a/src/model/price-range-impl.ts b/src/model/price-range-impl.ts index 539e7ed3b6..542e2b6792 100644 --- a/src/model/price-range-impl.ts +++ b/src/model/price-range-impl.ts @@ -2,6 +2,15 @@ import { isNumber } from '../helpers/strict-type-checks'; import { PriceRange } from './series-options'; +function computeFiniteResult( + values: number[], + method: (...values: number[]) => number, + fallback: number +): number { + const result = method(...values.filter(Number.isFinite)); + return Number.isFinite(result) ? result : fallback; +} + export class PriceRangeImpl { private _minValue: number; private _maxValue!: number; @@ -43,8 +52,8 @@ export class PriceRangeImpl { return this; } return new PriceRangeImpl( - Math.min(this.minValue(), anotherRange.minValue()), - Math.max(this.maxValue(), anotherRange.maxValue()) + computeFiniteResult([this.minValue(), anotherRange.minValue()], Math.min, -Infinity), + computeFiniteResult([this.maxValue(), anotherRange.maxValue()], Math.max, Infinity) ); } From b8afa6a4b05f4cfb1469bfce959d2cedf6b240a2 Mon Sep 17 00:00:00 2001 From: Mark Silverwood Date: Thu, 29 Jun 2023 16:55:57 +0100 Subject: [PATCH 2/5] add graphics e2e test case --- ...percentage-first-value-invisible-series.js | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 tests/e2e/graphics/test-cases/price-scale/percentage-first-value-invisible-series.js diff --git a/tests/e2e/graphics/test-cases/price-scale/percentage-first-value-invisible-series.js b/tests/e2e/graphics/test-cases/price-scale/percentage-first-value-invisible-series.js new file mode 100644 index 0000000000..022c63c1f5 --- /dev/null +++ b/tests/e2e/graphics/test-cases/price-scale/percentage-first-value-invisible-series.js @@ -0,0 +1,39 @@ +function runTestCase(container) { + const chartOptions = { + rightPriceScale: { + borderVisible: false, + mode: 2, + }, + layout: { + textColor: 'black', + background: { type: 'solid', color: 'white' }, + }, + }; + const chart = (window.chart = LightweightCharts.createChart( + container, + chartOptions + )); + + /* + * We expect the blue series to NOT be visible + * and the red series to BE visible + */ + + const series1 = chart.addLineSeries({ + color: '#2962FF', + }); + series1.setData([ + { time: 1522033200, value: 0 }, + { time: 1529895600, value: -3 }, + { time: 1537758000, value: 3 }, + ]); + const series2 = chart.addLineSeries({ + color: '#FF2962', + }); + series2.setData([ + { time: 1522033200, value: 1 }, + { time: 1529895600, value: -1 }, + { time: 1537758000, value: 2 }, + ]); + chart.timeScale().fitContent(); +} From bf08fb135ad53e79fb1af759918c6f72a4acff54 Mon Sep 17 00:00:00 2001 From: Mark Silverwood Date: Thu, 29 Jun 2023 17:20:06 +0100 Subject: [PATCH 3/5] avoid using unneeded arrays --- .size-limit.js | 8 ++++---- src/model/price-range-impl.ts | 19 ++++++++++++++----- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/.size-limit.js b/.size-limit.js index 6ffa48b19b..243d79f1e3 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -4,21 +4,21 @@ module.exports = [ { name: 'CJS', path: 'dist/lightweight-charts.production.cjs', - limit: '46.96 KB', + limit: '46.97 KB', }, { name: 'ESM', path: 'dist/lightweight-charts.production.mjs', - limit: '46.89 KB', + limit: '46.90 KB', }, { name: 'Standalone-ESM', path: 'dist/lightweight-charts.standalone.production.mjs', - limit: '48.58 KB', + limit: '48.59 KB', }, { name: 'Standalone', path: 'dist/lightweight-charts.standalone.production.js', - limit: '48.63 KB', + limit: '48.64 KB', }, ]; diff --git a/src/model/price-range-impl.ts b/src/model/price-range-impl.ts index 542e2b6792..c746f1a380 100644 --- a/src/model/price-range-impl.ts +++ b/src/model/price-range-impl.ts @@ -2,13 +2,22 @@ import { isNumber } from '../helpers/strict-type-checks'; import { PriceRange } from './series-options'; +function ensureFiniteWithFallback(value: number, fallback: number): number { + return Number.isFinite(value) ? value : fallback; +} + function computeFiniteResult( - values: number[], method: (...values: number[]) => number, + valueOne: number, + valueTwo: number, fallback: number ): number { - const result = method(...values.filter(Number.isFinite)); - return Number.isFinite(result) ? result : fallback; + const firstFinite = Number.isFinite(valueOne); + return firstFinite && Number.isFinite(valueTwo) + ? ensureFiniteWithFallback(method(valueOne, valueTwo), fallback) + : firstFinite + ? valueOne + : valueTwo; } export class PriceRangeImpl { @@ -52,8 +61,8 @@ export class PriceRangeImpl { return this; } return new PriceRangeImpl( - computeFiniteResult([this.minValue(), anotherRange.minValue()], Math.min, -Infinity), - computeFiniteResult([this.maxValue(), anotherRange.maxValue()], Math.max, Infinity) + computeFiniteResult(Math.min, this.minValue(), anotherRange.minValue(), -Infinity), + computeFiniteResult(Math.max, this.maxValue(), anotherRange.maxValue(), Infinity) ); } From 57e030f87c969e41bdc1c5d39870d2a00e7c5a32 Mon Sep 17 00:00:00 2001 From: Mark Silverwood Date: Thu, 29 Jun 2023 20:33:39 +0100 Subject: [PATCH 4/5] improve / fix computeFiniteResult method Whoops... the function now does what it says it does, even though the old logic was actually fine for the current requirement --- src/model/price-range-impl.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/model/price-range-impl.ts b/src/model/price-range-impl.ts index c746f1a380..e11ccb31b5 100644 --- a/src/model/price-range-impl.ts +++ b/src/model/price-range-impl.ts @@ -14,10 +14,10 @@ function computeFiniteResult( ): number { const firstFinite = Number.isFinite(valueOne); return firstFinite && Number.isFinite(valueTwo) - ? ensureFiniteWithFallback(method(valueOne, valueTwo), fallback) + ? method(valueOne, valueTwo) : firstFinite ? valueOne - : valueTwo; + : ensureFiniteWithFallback(valueTwo, fallback); } export class PriceRangeImpl { From 54dd023dd44884d013af04fbb0d28a52782eff2f Mon Sep 17 00:00:00 2001 From: Mark Silverwood Date: Mon, 3 Jul 2023 11:40:43 +0100 Subject: [PATCH 5/5] apply suggested changes to computeFiniteResult --- src/model/price-range-impl.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/model/price-range-impl.ts b/src/model/price-range-impl.ts index e11ccb31b5..04e9b13e77 100644 --- a/src/model/price-range-impl.ts +++ b/src/model/price-range-impl.ts @@ -2,10 +2,6 @@ import { isNumber } from '../helpers/strict-type-checks'; import { PriceRange } from './series-options'; -function ensureFiniteWithFallback(value: number, fallback: number): number { - return Number.isFinite(value) ? value : fallback; -} - function computeFiniteResult( method: (...values: number[]) => number, valueOne: number, @@ -13,11 +9,13 @@ function computeFiniteResult( fallback: number ): number { const firstFinite = Number.isFinite(valueOne); - return firstFinite && Number.isFinite(valueTwo) - ? method(valueOne, valueTwo) - : firstFinite - ? valueOne - : ensureFiniteWithFallback(valueTwo, fallback); + const secondFinite = Number.isFinite(valueTwo); + + if (firstFinite && secondFinite) { + return method(valueOne, valueTwo); + } + + return !firstFinite && !secondFinite ? fallback : (firstFinite ? valueOne : valueTwo); } export class PriceRangeImpl {