From 2a2389266e82e1392b71ef7e0a57f866d6996b0a Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Thu, 2 Jan 2025 15:41:54 +0100 Subject: [PATCH 1/6] First step in fixing the bug of spanGaps null point interaction --- src/core/core.interaction.js | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/core/core.interaction.js b/src/core/core.interaction.js index c35f8d1ae08..a0d6e0cb376 100644 --- a/src/core/core.interaction.js +++ b/src/core/core.interaction.js @@ -20,12 +20,35 @@ import {_isPointInArea} from '../helpers/index.js'; * @returns {{lo:number, hi:number}} indices to search data array between */ function binarySearch(metaset, axis, value, intersect) { + console.log("BINARY SEARCH"); + console.log("Metaset"); + console.log(metaset); + const {controller, data, _sorted} = metaset; const iScale = controller._cachedMeta.iScale; + const spanGaps = metaset.dataset.options.spanGaps; + console.log(`spanGaps: ${spanGaps}`); + if (iScale && axis === iScale.axis && axis !== 'r' && _sorted && data.length) { const lookupMethod = iScale._reversePixels ? _rlookupByKey : _lookupByKey; if (!intersect) { - return lookupMethod(data, axis, value); + const result = lookupMethod(data, axis, value) + + if (spanGaps) { + const vAxis = controller._cachedMeta.vScale.axis; + const { _parsed } = metaset; + + const distanceToDefinedLo = _parsed.slice(0, result.lo).reverse().findIndex(point => point[vAxis]); + console.log(data); + result.lo = distanceToDefinedLo !== -1 ? result.lo - distanceToDefinedLo : result.lo; + console.log(distanceToDefinedLo); + + const distanceToDefinedHi = _parsed.slice(result.hi - 1).findIndex(point => point[vAxis]); + result.hi = distanceToDefinedHi !== -1 ? result.hi + distanceToDefinedHi : result.hi; + } + console.log(`lo ${result.lo}, hi ${result.hi}`); + return result; + } else if (controller._sharedOptions) { // _sharedOptions indicates that each element has equal options -> equal proportions // So we can do a ranged binary search based on the range of first element and @@ -52,11 +75,19 @@ function binarySearch(metaset, axis, value, intersect) { * @param {boolean} [intersect] - consider intersecting items */ function evaluateInteractionItems(chart, axis, position, handler, intersect) { + //console.log("evaluateInteractionItems"); const metasets = chart.getSortedVisibleDatasetMetas(); const value = position[axis]; + //console.log(`value: ${value}`); for (let i = 0, ilen = metasets.length; i < ilen; ++i) { + //console.log("metaset"); + //console.log(metasets[i]); const {index, data} = metasets[i]; + const spanGaps = metasets[i].dataset.options.spanGaps; + //console.log(`spanGaps: ${spanGaps}`); const {lo, hi} = binarySearch(metasets[i], axis, value, intersect); + //console.log(`lo: ${lo}`); + //console.log(`hi: ${hi}`); for (let j = lo; j <= hi; ++j) { const element = data[j]; if (!element.skip) { @@ -146,6 +177,7 @@ function getNearestRadialItems(chart, position, axis, useFinalPosition) { * @return {InteractionItem[]} the nearest items */ function getNearestCartesianItems(chart, position, axis, intersect, useFinalPosition, includeInvisible) { + let items = []; const distanceMetric = getDistanceMetricForAxis(axis); let minDistance = Number.POSITIVE_INFINITY; From 2e4811a38a8972f1317db6f4c7b2ecfff424007b Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Thu, 2 Jan 2025 20:04:32 +0100 Subject: [PATCH 2/6] Complete bugfix of spanGaps null point interaction --- src/core/core.interaction.js | 45 ++++++++++++++---------------------- 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/src/core/core.interaction.js b/src/core/core.interaction.js index a0d6e0cb376..20156f5f462 100644 --- a/src/core/core.interaction.js +++ b/src/core/core.interaction.js @@ -20,35 +20,32 @@ import {_isPointInArea} from '../helpers/index.js'; * @returns {{lo:number, hi:number}} indices to search data array between */ function binarySearch(metaset, axis, value, intersect) { - console.log("BINARY SEARCH"); - console.log("Metaset"); - console.log(metaset); - const {controller, data, _sorted} = metaset; const iScale = controller._cachedMeta.iScale; - const spanGaps = metaset.dataset.options.spanGaps; - console.log(`spanGaps: ${spanGaps}`); + const spanGaps = metaset.dataset ? metaset.dataset.options ? metaset.dataset.options.spanGaps : null : null; if (iScale && axis === iScale.axis && axis !== 'r' && _sorted && data.length) { const lookupMethod = iScale._reversePixels ? _rlookupByKey : _lookupByKey; if (!intersect) { - const result = lookupMethod(data, axis, value) - + const result = lookupMethod(data, axis, value); if (spanGaps) { - const vAxis = controller._cachedMeta.vScale.axis; - const { _parsed } = metaset; - - const distanceToDefinedLo = _parsed.slice(0, result.lo).reverse().findIndex(point => point[vAxis]); - console.log(data); - result.lo = distanceToDefinedLo !== -1 ? result.lo - distanceToDefinedLo : result.lo; - console.log(distanceToDefinedLo); - - const distanceToDefinedHi = _parsed.slice(result.hi - 1).findIndex(point => point[vAxis]); - result.hi = distanceToDefinedHi !== -1 ? result.hi + distanceToDefinedHi : result.hi; + const {vScale} = controller._cachedMeta; + const {_parsed} = metaset; + + const distanceToDefinedLo = (_parsed + .slice(0, result.lo + 1) + .reverse() + .findIndex( + point => point[vScale.axis] || point[vScale.axis] === 0)); + result.lo -= Math.max(0, distanceToDefinedLo); + + const distanceToDefinedHi = (_parsed + .slice(result.hi - 1) + .findIndex( + point => point[vScale.axis] || point[vScale.axis] === 0)); + result.hi += Math.max(0, distanceToDefinedHi); } - console.log(`lo ${result.lo}, hi ${result.hi}`); return result; - } else if (controller._sharedOptions) { // _sharedOptions indicates that each element has equal options -> equal proportions // So we can do a ranged binary search based on the range of first element and @@ -75,19 +72,11 @@ function binarySearch(metaset, axis, value, intersect) { * @param {boolean} [intersect] - consider intersecting items */ function evaluateInteractionItems(chart, axis, position, handler, intersect) { - //console.log("evaluateInteractionItems"); const metasets = chart.getSortedVisibleDatasetMetas(); const value = position[axis]; - //console.log(`value: ${value}`); for (let i = 0, ilen = metasets.length; i < ilen; ++i) { - //console.log("metaset"); - //console.log(metasets[i]); const {index, data} = metasets[i]; - const spanGaps = metasets[i].dataset.options.spanGaps; - //console.log(`spanGaps: ${spanGaps}`); const {lo, hi} = binarySearch(metasets[i], axis, value, intersect); - //console.log(`lo: ${lo}`); - //console.log(`hi: ${hi}`); for (let j = lo; j <= hi; ++j) { const element = data[j]; if (!element.skip) { From 668baa66bd0e7da2284286fee87c476ce3177645 Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Thu, 2 Jan 2025 21:32:10 +0100 Subject: [PATCH 3/6] Add two tests in core.interaction.tests for the bugfix change --- test/specs/core.interaction.tests.js | 58 ++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/test/specs/core.interaction.tests.js b/test/specs/core.interaction.tests.js index bfd95ae352e..e0645cf5406 100644 --- a/test/specs/core.interaction.tests.js +++ b/test/specs/core.interaction.tests.js @@ -912,4 +912,62 @@ describe('Core.Interaction', function() { expect(elements).toContain(firstElement); }); }); + + it('should select closest non-null elements if spanGaps=true and closest non-null element is to the left', function() { + const chart = window.acquireChart({ + type: 'line', + data: { + labels: [1, 2, 3, 4, 5, 6, 7, 8, 9], + datasets: [{ + data: [12, 19, null, null, null, null, 5, 2, 3], + spanGaps: true, + }] + } + }); + chart.update(); + const interactionPointIndex = 3; + const meta = chart.getDatasetMeta(0); + const point = meta.data[interactionPointIndex]; + + const evt = { + type: 'click', + chart: chart, + native: true, // needed otherwise things its a DOM event + x: point.x, + y: point.y, + }; + + const expectedInteractionPointIndex = 1; + const elements = Chart.Interaction.modes.nearest(chart, evt, {axis: 'x', intersect: false}).map(item => item.element); + expect(elements).toEqual([meta.data[expectedInteractionPointIndex]]); + }); + + it('should select closest non-null elements if spanGaps=true and closest non-null element is to the right', function() { + const chart = window.acquireChart({ + type: 'line', + data: { + labels: [1, 2, 3, 4, 5, 6, 7, 8, 9], + datasets: [{ + data: [12, 19, null, null, null, null, 5, 2, 3], + spanGaps: true, + }] + } + }); + chart.update(); + const interactionPointIndex = 4; + const meta = chart.getDatasetMeta(0); + const point = meta.data[interactionPointIndex]; + + const evt = { + type: 'click', + chart: chart, + native: true, // needed otherwise things its a DOM event + x: point.x, + y: point.y, + }; + + const expectedInteractionPointIndex = 6; + const elements = Chart.Interaction.modes.nearest(chart, evt, {axis: 'x', intersect: false}).map(item => item.element); + expect(elements).toEqual([meta.data[expectedInteractionPointIndex]]); + }); }); From 95d37fdbb1948aba71bd0e74f6184e063684e711 Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Thu, 2 Jan 2025 21:33:39 +0100 Subject: [PATCH 4/6] Remove odd line break --- src/core/core.interaction.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/core.interaction.js b/src/core/core.interaction.js index 20156f5f462..1c75447206e 100644 --- a/src/core/core.interaction.js +++ b/src/core/core.interaction.js @@ -166,7 +166,6 @@ function getNearestRadialItems(chart, position, axis, useFinalPosition) { * @return {InteractionItem[]} the nearest items */ function getNearestCartesianItems(chart, position, axis, intersect, useFinalPosition, includeInvisible) { - let items = []; const distanceMetric = getDistanceMetricForAxis(axis); let minDistance = Number.POSITIVE_INFINITY; From fe0cd3d24044d2b563e1fd08027e284735bd1c81 Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Fri, 3 Jan 2025 15:23:51 +0100 Subject: [PATCH 5/6] Use isNullOrUndef helper for point value checks --- src/core/core.interaction.js | 6 +++--- src/helpers/helpers.extras.ts | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/core/core.interaction.js b/src/core/core.interaction.js index 1c75447206e..8a716023651 100644 --- a/src/core/core.interaction.js +++ b/src/core/core.interaction.js @@ -1,7 +1,7 @@ import {_lookupByKey, _rlookupByKey} from '../helpers/helpers.collection.js'; import {getRelativePosition} from '../helpers/helpers.dom.js'; import {_angleBetween, getAngleFromPoint} from '../helpers/helpers.math.js'; -import {_isPointInArea} from '../helpers/index.js'; +import {_isPointInArea, isNullOrUndef} from '../helpers/index.js'; /** * @typedef { import('./core.controller.js').default } Chart @@ -36,13 +36,13 @@ function binarySearch(metaset, axis, value, intersect) { .slice(0, result.lo + 1) .reverse() .findIndex( - point => point[vScale.axis] || point[vScale.axis] === 0)); + point => !isNullOrUndef(point[vScale.axis]))); result.lo -= Math.max(0, distanceToDefinedLo); const distanceToDefinedHi = (_parsed .slice(result.hi - 1) .findIndex( - point => point[vScale.axis] || point[vScale.axis] === 0)); + point => !isNullOrUndef(point[vScale.axis]))); result.hi += Math.max(0, distanceToDefinedHi); } return result; diff --git a/src/helpers/helpers.extras.ts b/src/helpers/helpers.extras.ts index beabb6d96f3..798e10c1e86 100644 --- a/src/helpers/helpers.extras.ts +++ b/src/helpers/helpers.extras.ts @@ -2,6 +2,7 @@ import type {ChartMeta, PointElement} from '../types/index.js'; import {_limitValue} from './helpers.math.js'; import {_lookupByKey} from './helpers.collection.js'; +import {isNullOrUndef} from './helpers.core.js'; export function fontString(pixelSize: number, fontStyle: string, fontFamily: string) { return fontStyle + ' ' + pixelSize + 'px ' + fontFamily; @@ -107,7 +108,7 @@ export function _getStartAndCountOfVisiblePoints(meta: ChartMeta<'line' | 'scatt .slice(0, start + 1) .reverse() .findIndex( - point => point[vScale.axis] || point[vScale.axis] === 0)); + point => !isNullOrUndef(point[vScale.axis]))); start -= Math.max(0, distanceToDefinedLo); } start = _limitValue(start, 0, pointCount - 1); @@ -122,7 +123,7 @@ export function _getStartAndCountOfVisiblePoints(meta: ChartMeta<'line' | 'scatt const distanceToDefinedHi = (_parsed .slice(end - 1) .findIndex( - point => point[vScale.axis] || point[vScale.axis] === 0)); + point => !isNullOrUndef(point[vScale.axis]))); end += Math.max(0, distanceToDefinedHi); } count = _limitValue(end, start, pointCount) - start; From 7c20804d013d008538f9584ad03b9620040aa330 Mon Sep 17 00:00:00 2001 From: Mariss Tubelis Date: Fri, 3 Jan 2025 16:01:14 +0100 Subject: [PATCH 6/6] Add 10 more test cases for nearest interaction when spanGaps=true --- test/specs/core.interaction.tests.js | 140 ++++++++++++++++----------- 1 file changed, 86 insertions(+), 54 deletions(-) diff --git a/test/specs/core.interaction.tests.js b/test/specs/core.interaction.tests.js index e0645cf5406..9d693f1488c 100644 --- a/test/specs/core.interaction.tests.js +++ b/test/specs/core.interaction.tests.js @@ -913,61 +913,93 @@ describe('Core.Interaction', function() { }); }); - it('should select closest non-null elements if spanGaps=true and closest non-null element is to the left', function() { - const chart = window.acquireChart({ - type: 'line', - data: { - labels: [1, 2, 3, 4, 5, 6, 7, 8, 9], - datasets: [{ - data: [12, 19, null, null, null, null, 5, 2, 3], - spanGaps: true, - }] - } - }); - chart.update(); - const interactionPointIndex = 3; - const meta = chart.getDatasetMeta(0); - const point = meta.data[interactionPointIndex]; - - const evt = { - type: 'click', - chart: chart, - native: true, // needed otherwise things its a DOM event - x: point.x, - y: point.y, - }; - - const expectedInteractionPointIndex = 1; - const elements = Chart.Interaction.modes.nearest(chart, evt, {axis: 'x', intersect: false}).map(item => item.element); - expect(elements).toEqual([meta.data[expectedInteractionPointIndex]]); - }); + const testCases = [ + { + data: [12, 19, null, null, null, null, 5, 2], + clickPointIndex: 0, + expectedNearestPointIndex: 0 + }, + { + data: [12, 19, null, null, null, null, 5, 2], + clickPointIndex: 1, + expectedNearestPointIndex: 1}, + { + data: [12, 19, null, null, null, null, 5, 2], + clickPointIndex: 2, + expectedNearestPointIndex: 1 + }, + { + data: [12, 19, null, null, null, null, 5, 2], + clickPointIndex: 3, + expectedNearestPointIndex: 1 + }, + { + data: [12, 19, null, null, null, null, 5, 2], + clickPointIndex: 4, + expectedNearestPointIndex: 6 + }, + { + data: [12, 19, null, null, null, null, 5, 2], + clickPointIndex: 5, + expectedNearestPointIndex: 6 + }, + { + data: [12, 19, null, null, null, null, 5, 2], + clickPointIndex: 6, + expectedNearestPointIndex: 6 + }, + { + data: [12, 19, null, null, null, null, 5, 2], + clickPointIndex: 7, + expectedNearestPointIndex: 7 + }, + { + data: [12, 0, null, null, null, null, 0, 2], + clickPointIndex: 3, + expectedNearestPointIndex: 1 + }, + { + data: [12, 0, null, null, null, null, 0, 2], + clickPointIndex: 4, + expectedNearestPointIndex: 6 + }, + { + data: [12, -1, null, null, null, null, -1, 2], + clickPointIndex: 3, + expectedNearestPointIndex: 1 + }, + { + data: [12, -1, null, null, null, null, -1, 2], + clickPointIndex: 4, + expectedNearestPointIndex: 6 + } + ]; + testCases.forEach(({data, clickPointIndex, expectedNearestPointIndex}, i) => { + it(`should select nearest non-null element with index ${expectedNearestPointIndex} when clicking on element with index ${clickPointIndex} in test case ${i + 1} if spanGaps=true`, function() { + const chart = window.acquireChart({ + type: 'line', + data: { + labels: [1, 2, 3, 4, 5, 6, 7, 8, 9], + datasets: [{ + data: data, + spanGaps: true, + }] + } + }); + chart.update(); + const meta = chart.getDatasetMeta(0); + const point = meta.data[clickPointIndex]; - it('should select closest non-null elements if spanGaps=true and closest non-null element is to the right', function() { - const chart = window.acquireChart({ - type: 'line', - data: { - labels: [1, 2, 3, 4, 5, 6, 7, 8, 9], - datasets: [{ - data: [12, 19, null, null, null, null, 5, 2, 3], - spanGaps: true, - }] - } + const evt = { + type: 'click', + chart: chart, + native: true, // needed otherwise things its a DOM event + x: point.x, + y: point.y, + }; + + const elements = Chart.Interaction.modes.nearest(chart, evt, {axis: 'x', intersect: false}).map(item => item.element); + expect(elements).toEqual([meta.data[expectedNearestPointIndex]]); }); - chart.update(); - const interactionPointIndex = 4; - const meta = chart.getDatasetMeta(0); - const point = meta.data[interactionPointIndex]; - - const evt = { - type: 'click', - chart: chart, - native: true, // needed otherwise things its a DOM event - x: point.x, - y: point.y, - }; - - const expectedInteractionPointIndex = 6; - const elements = Chart.Interaction.modes.nearest(chart, evt, {axis: 'x', intersect: false}).map(item => item.element); - expect(elements).toEqual([meta.data[expectedInteractionPointIndex]]); }); });