From 281698be150a4fbd1730037f0be8c30e7c748409 Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Tue, 31 Dec 2024 19:34:47 +0100 Subject: [PATCH 1/8] Bugfix: spanGaps not working near min and max limits --- src/helpers/helpers.extras.ts | 22 +++++++++++++++------- src/types/global.d.ts | 10 ++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) create mode 100644 src/types/global.d.ts diff --git a/src/helpers/helpers.extras.ts b/src/helpers/helpers.extras.ts index dc34ecf07f9..46a6ef8d053 100644 --- a/src/helpers/helpers.extras.ts +++ b/src/helpers/helpers.extras.ts @@ -91,25 +91,33 @@ export function _getStartAndCountOfVisiblePoints(meta: ChartMeta<'line' | 'scatt let count = pointCount; if (meta._sorted) { - const {iScale, _parsed} = meta; + const {iScale, vScale, _parsed, dataset: {options: {spanGaps} = {}}} = meta; const axis = iScale.axis; const {min, max, minDefined, maxDefined} = iScale.getUserBounds(); if (minDefined) { - start = _limitValue(Math.min( + start = Math.min( // @ts-expect-error Need to type _parsed _lookupByKey(_parsed, axis, min).lo, // @ts-expect-error Need to fix types on _lookupByKey - animationsDisabled ? pointCount : _lookupByKey(points, axis, iScale.getPixelForValue(min)).lo), - 0, pointCount - 1); + animationsDisabled ? pointCount : _lookupByKey(points, axis, iScale.getPixelForValue(min)).lo); + if (spanGaps) { + const lastDefinedLo = _parsed.slice(0, start + 1).findLastIndex(point => point[vScale.axis]); + start = lastDefinedLo !== -1 ? lastDefinedLo : start; + } + start = _limitValue(start, 0, pointCount - 1); } if (maxDefined) { - count = _limitValue(Math.max( + let end = Math.max( // @ts-expect-error Need to type _parsed _lookupByKey(_parsed, iScale.axis, max, true).hi + 1, // @ts-expect-error Need to fix types on _lookupByKey - animationsDisabled ? 0 : _lookupByKey(points, axis, iScale.getPixelForValue(max), true).hi + 1), - start, pointCount) - start; + animationsDisabled ? 0 : _lookupByKey(points, axis, iScale.getPixelForValue(max), true).hi + 1); + if (spanGaps) { + const lastDefinedHi = _parsed.slice(end - 1).findIndex(point => point[vScale.axis]); + end += lastDefinedHi !== -1 ? lastDefinedHi : 0; + } + count = _limitValue(end, start, pointCount) - start; } else { count = pointCount - start; } diff --git a/src/types/global.d.ts b/src/types/global.d.ts new file mode 100644 index 00000000000..8331d3fa085 --- /dev/null +++ b/src/types/global.d.ts @@ -0,0 +1,10 @@ +export {}; + +declare global { + interface Array { + findLastIndex( + predicate: (value: T, index: number, obj: T[]) => unknown, + thisArg?: any + ): number + } +} From 1572d110142c25b3a3afefd0bc9cb6cc8d6bc4cd Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Wed, 1 Jan 2025 13:16:29 +0100 Subject: [PATCH 2/8] Fix error when meta.dataset.options = null --- src/helpers/helpers.extras.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/helpers/helpers.extras.ts b/src/helpers/helpers.extras.ts index 46a6ef8d053..32386541f27 100644 --- a/src/helpers/helpers.extras.ts +++ b/src/helpers/helpers.extras.ts @@ -91,7 +91,8 @@ export function _getStartAndCountOfVisiblePoints(meta: ChartMeta<'line' | 'scatt let count = pointCount; if (meta._sorted) { - const {iScale, vScale, _parsed, dataset: {options: {spanGaps} = {}}} = meta; + const {iScale, vScale, _parsed} = meta; + const spanGaps = meta.dataset ? meta.dataset.options ? meta.dataset.options.spanGaps : null : null; const axis = iScale.axis; const {min, max, minDefined, maxDefined} = iScale.getUserBounds(); From df35934175f5e7f2d7cf15c38177fe0259e02724 Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Wed, 1 Jan 2025 22:53:05 +0100 Subject: [PATCH 3/8] Add tests for correct setting of line controller properties _drawStart and _drawCount --- test/specs/controller.line.tests.js | 84 +++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/test/specs/controller.line.tests.js b/test/specs/controller.line.tests.js index 368627d68a0..755cc815dc3 100644 --- a/test/specs/controller.line.tests.js +++ b/test/specs/controller.line.tests.js @@ -1071,6 +1071,90 @@ describe('Chart.controllers.line', function() { expect(visiblePoints.length).toBe(6); }, 500); + it('should correctly calc _drawStart and _drawCount when first points beyond scale limits are null and spanGaps=true', async() => { + var chart = window.acquireChart({ + type: 'line', + data: { + labels: [0, 10, 20, 30, 40, 50], + datasets: [{ + data: [3, null, 2, 3, null, 3], + spanGaps: true, + }] + }, + options: { + scales: { + x: { + type: 'linear', + min: 11, + max: 40, + } + } + } + }); + + chart.update(); + var controller = chart.getDatasetMeta(0).controller; + + expect(controller._drawStart).toBe(0); + expect(controller._drawCount).toBe(6); + }, 500); + + it('should correctly calc _drawStart and _drawCount when all points beyond scale limits are null and spanGaps=true', async() => { + var chart = window.acquireChart({ + type: 'line', + data: { + labels: [0, 10, 20, 30, 40, 50], + datasets: [{ + data: [null, null, 2, 3, null, null], + spanGaps: true, + }] + }, + options: { + scales: { + x: { + type: 'linear', + min: 11, + max: 40, + } + } + } + }); + + chart.update(); + var controller = chart.getDatasetMeta(0).controller; + + expect(controller._drawStart).toBe(1); + expect(controller._drawCount).toBe(4); + }, 500); + + it('should correctly calc _drawStart and _drawCount when spanGaps=false', async() => { + var chart = window.acquireChart({ + type: 'line', + data: { + labels: [0, 10, 20, 30, 40, 50], + datasets: [{ + data: [3, null, 2, 3, null, 3], + spanGaps: false, + }] + }, + options: { + scales: { + x: { + type: 'linear', + min: 11, + max: 40, + } + } + } + }); + + chart.update(); + var controller = chart.getDatasetMeta(0).controller; + + expect(controller._drawStart).toBe(1); + expect(controller._drawCount).toBe(4); + }, 500); + it('should not override tooltip title and label callbacks', async() => { const chart = window.acquireChart({ type: 'line', From 538487d63c1b3aff43f33ca2ca70d7192b1859a0 Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Wed, 1 Jan 2025 22:57:10 +0100 Subject: [PATCH 4/8] Fix spacing in controller line tests --- test/specs/controller.line.tests.js | 66 ++++++++++++++--------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/test/specs/controller.line.tests.js b/test/specs/controller.line.tests.js index 755cc815dc3..d779c5dee1f 100644 --- a/test/specs/controller.line.tests.js +++ b/test/specs/controller.line.tests.js @@ -1075,20 +1075,20 @@ describe('Chart.controllers.line', function() { var chart = window.acquireChart({ type: 'line', data: { - labels: [0, 10, 20, 30, 40, 50], - datasets: [{ - data: [3, null, 2, 3, null, 3], - spanGaps: true, - }] + labels: [0, 10, 20, 30, 40, 50], + datasets: [{ + data: [3, null, 2, 3, null, 3], + spanGaps: true, + }] }, options: { - scales: { - x: { - type: 'linear', - min: 11, - max: 40, - } + scales: { + x: { + type: 'linear', + min: 11, + max: 40, } + } } }); @@ -1103,20 +1103,20 @@ describe('Chart.controllers.line', function() { var chart = window.acquireChart({ type: 'line', data: { - labels: [0, 10, 20, 30, 40, 50], - datasets: [{ - data: [null, null, 2, 3, null, null], - spanGaps: true, - }] + labels: [0, 10, 20, 30, 40, 50], + datasets: [{ + data: [null, null, 2, 3, null, null], + spanGaps: true, + }] }, options: { - scales: { - x: { - type: 'linear', - min: 11, - max: 40, - } + scales: { + x: { + type: 'linear', + min: 11, + max: 40, } + } } }); @@ -1131,20 +1131,20 @@ describe('Chart.controllers.line', function() { var chart = window.acquireChart({ type: 'line', data: { - labels: [0, 10, 20, 30, 40, 50], - datasets: [{ - data: [3, null, 2, 3, null, 3], - spanGaps: false, - }] + labels: [0, 10, 20, 30, 40, 50], + datasets: [{ + data: [3, null, 2, 3, null, 3], + spanGaps: false, + }] }, options: { - scales: { - x: { - type: 'linear', - min: 11, - max: 40, - } + scales: { + x: { + type: 'linear', + min: 11, + max: 40, } + } } }); From 8345e65880ca7840be9297c09d4169e391df9e60 Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Wed, 1 Jan 2025 23:10:47 +0100 Subject: [PATCH 5/8] Add tension to test --- test/specs/controller.line.tests.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/specs/controller.line.tests.js b/test/specs/controller.line.tests.js index d779c5dee1f..79f9547b1b6 100644 --- a/test/specs/controller.line.tests.js +++ b/test/specs/controller.line.tests.js @@ -1079,6 +1079,7 @@ describe('Chart.controllers.line', function() { datasets: [{ data: [3, null, 2, 3, null, 3], spanGaps: true, + tension: 0.4 }] }, options: { @@ -1107,6 +1108,7 @@ describe('Chart.controllers.line', function() { datasets: [{ data: [null, null, 2, 3, null, null], spanGaps: true, + tension: 0.4 }] }, options: { @@ -1135,6 +1137,7 @@ describe('Chart.controllers.line', function() { datasets: [{ data: [3, null, 2, 3, null, 3], spanGaps: false, + tension: 0.4 }] }, options: { From a23496a566db1b7b82b8c146c63970f68c6e6856 Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Wed, 1 Jan 2025 23:14:19 +0100 Subject: [PATCH 6/8] Add a better test case --- test/specs/controller.line.tests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/specs/controller.line.tests.js b/test/specs/controller.line.tests.js index 79f9547b1b6..3924a79887d 100644 --- a/test/specs/controller.line.tests.js +++ b/test/specs/controller.line.tests.js @@ -1077,7 +1077,7 @@ describe('Chart.controllers.line', function() { data: { labels: [0, 10, 20, 30, 40, 50], datasets: [{ - data: [3, null, 2, 3, null, 3], + data: [3, null, 2, 3, null, 1.5], spanGaps: true, tension: 0.4 }] @@ -1135,7 +1135,7 @@ describe('Chart.controllers.line', function() { data: { labels: [0, 10, 20, 30, 40, 50], datasets: [{ - data: [3, null, 2, 3, null, 3], + data: [3, null, 2, 3, null, 1.5], spanGaps: false, tension: 0.4 }] From 8cada725afa06539d615fdb8724cb48f7fa3e508 Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Thu, 2 Jan 2025 12:03:39 +0100 Subject: [PATCH 7/8] Avoid the use of FindLastIndex --- src/helpers/helpers.extras.ts | 8 ++++---- src/types/global.d.ts | 10 ---------- 2 files changed, 4 insertions(+), 14 deletions(-) delete mode 100644 src/types/global.d.ts diff --git a/src/helpers/helpers.extras.ts b/src/helpers/helpers.extras.ts index 32386541f27..c741faca0db 100644 --- a/src/helpers/helpers.extras.ts +++ b/src/helpers/helpers.extras.ts @@ -103,8 +103,8 @@ export function _getStartAndCountOfVisiblePoints(meta: ChartMeta<'line' | 'scatt // @ts-expect-error Need to fix types on _lookupByKey animationsDisabled ? pointCount : _lookupByKey(points, axis, iScale.getPixelForValue(min)).lo); if (spanGaps) { - const lastDefinedLo = _parsed.slice(0, start + 1).findLastIndex(point => point[vScale.axis]); - start = lastDefinedLo !== -1 ? lastDefinedLo : start; + const distanceToDefinedLo = _parsed.slice(0, start + 1).reverse().findIndex(point => point[vScale.axis]); + start = distanceToDefinedLo !== -1 ? start - distanceToDefinedLo : start; } start = _limitValue(start, 0, pointCount - 1); } @@ -115,8 +115,8 @@ export function _getStartAndCountOfVisiblePoints(meta: ChartMeta<'line' | 'scatt // @ts-expect-error Need to fix types on _lookupByKey animationsDisabled ? 0 : _lookupByKey(points, axis, iScale.getPixelForValue(max), true).hi + 1); if (spanGaps) { - const lastDefinedHi = _parsed.slice(end - 1).findIndex(point => point[vScale.axis]); - end += lastDefinedHi !== -1 ? lastDefinedHi : 0; + const definedHiIndex = _parsed.slice(end - 1).findIndex(point => point[vScale.axis]); + end += definedHiIndex !== -1 ? definedHiIndex : 0; } count = _limitValue(end, start, pointCount) - start; } else { diff --git a/src/types/global.d.ts b/src/types/global.d.ts deleted file mode 100644 index 8331d3fa085..00000000000 --- a/src/types/global.d.ts +++ /dev/null @@ -1,10 +0,0 @@ -export {}; - -declare global { - interface Array { - findLastIndex( - predicate: (value: T, index: number, obj: T[]) => unknown, - thisArg?: any - ): number - } -} From 7aaced2ed11a052a76ef7fda680671e304371250 Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Thu, 2 Jan 2025 15:52:31 +0100 Subject: [PATCH 8/8] Avoid taking 0 for null value and improve naming --- src/helpers/helpers.extras.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/helpers/helpers.extras.ts b/src/helpers/helpers.extras.ts index c741faca0db..beabb6d96f3 100644 --- a/src/helpers/helpers.extras.ts +++ b/src/helpers/helpers.extras.ts @@ -103,8 +103,12 @@ export function _getStartAndCountOfVisiblePoints(meta: ChartMeta<'line' | 'scatt // @ts-expect-error Need to fix types on _lookupByKey animationsDisabled ? pointCount : _lookupByKey(points, axis, iScale.getPixelForValue(min)).lo); if (spanGaps) { - const distanceToDefinedLo = _parsed.slice(0, start + 1).reverse().findIndex(point => point[vScale.axis]); - start = distanceToDefinedLo !== -1 ? start - distanceToDefinedLo : start; + const distanceToDefinedLo = (_parsed + .slice(0, start + 1) + .reverse() + .findIndex( + point => point[vScale.axis] || point[vScale.axis] === 0)); + start -= Math.max(0, distanceToDefinedLo); } start = _limitValue(start, 0, pointCount - 1); } @@ -115,8 +119,11 @@ export function _getStartAndCountOfVisiblePoints(meta: ChartMeta<'line' | 'scatt // @ts-expect-error Need to fix types on _lookupByKey animationsDisabled ? 0 : _lookupByKey(points, axis, iScale.getPixelForValue(max), true).hi + 1); if (spanGaps) { - const definedHiIndex = _parsed.slice(end - 1).findIndex(point => point[vScale.axis]); - end += definedHiIndex !== -1 ? definedHiIndex : 0; + const distanceToDefinedHi = (_parsed + .slice(end - 1) + .findIndex( + point => point[vScale.axis] || point[vScale.axis] === 0)); + end += Math.max(0, distanceToDefinedHi); } count = _limitValue(end, start, pointCount) - start; } else {