Skip to content

Commit

Permalink
Bugfix: return nearest non-null point on interaction when spanGaps=tr…
Browse files Browse the repository at this point in the history
…ue (#11986)

* First step in fixing the bug of spanGaps null point interaction

* Complete bugfix of spanGaps null point interaction

* Add two tests in core.interaction.tests for the bugfix change

* Remove odd line break

* Use isNullOrUndef helper for point value checks

* Add 10 more test cases for nearest interaction when spanGaps=true
  • Loading branch information
marisst authored Jan 3, 2025
1 parent 1e3d6e5 commit a77a63b
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 4 deletions.
24 changes: 22 additions & 2 deletions src/core/core.interaction.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -22,10 +22,30 @@ import {_isPointInArea} from '../helpers/index.js';
function binarySearch(metaset, axis, value, intersect) {
const {controller, data, _sorted} = metaset;
const iScale = controller._cachedMeta.iScale;
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) {
return lookupMethod(data, axis, value);
const result = lookupMethod(data, axis, value);
if (spanGaps) {
const {vScale} = controller._cachedMeta;
const {_parsed} = metaset;

const distanceToDefinedLo = (_parsed
.slice(0, result.lo + 1)
.reverse()
.findIndex(
point => !isNullOrUndef(point[vScale.axis])));
result.lo -= Math.max(0, distanceToDefinedLo);

const distanceToDefinedHi = (_parsed
.slice(result.hi - 1)
.findIndex(
point => !isNullOrUndef(point[vScale.axis])));
result.hi += Math.max(0, distanceToDefinedHi);
}
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
Expand Down
5 changes: 3 additions & 2 deletions src/helpers/helpers.extras.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down
90 changes: 90 additions & 0 deletions test/specs/core.interaction.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -912,4 +912,94 @@ describe('Core.Interaction', function() {
expect(elements).toContain(firstElement);
});
});

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];

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]]);
});
});
});

0 comments on commit a77a63b

Please sign in to comment.