Do not allow to select() or hover() non-existing points

Author: flekschas

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## Next
 
+## v0.18.4
+
+- Fix an issues when programmatically trying to `select()` or `hover()` non-existing points
+
 ## v0.18.3
 
 - Harmonize `hover(pointIdx, { showReticleOnce, preventEvent })` with `select()` API by allowing it to prevent the `pointover` and `pointout` from being published.

src/index.js

@@ -632,25 +632,35 @@ const createScatterplot = (initialProperties = {}) => {
   };
 
   const select = (pointIdxs, { merge = false, preventEvent = false } = {}) => {
+    const pointIdxsArr = Array.isArray(pointIdxs) ? pointIdxs : [pointIdxs];
+
     if (merge) {
-      selection = unionIntegers(selection, pointIdxs);
+      selection = unionIntegers(selection, pointIdxsArr);
     } else {
       // Unset previously highlight point connections
       if (selection && selection.length)
         setPointConnectionColorState(selection, 0);
-      selection = pointIdxs;
+      selection = pointIdxsArr;
     }
 
-    const selectionBuffer = new Float32Array(selection.length * 2);
+    const selectionBuffer = [];
 
     selectionSet.clear();
     selectionConnecionSet.clear();
-    selection.forEach((pointIdx, i) => {
-      selectionSet.add(pointIdx);
-      const texCoords = indexToStateTexCoord(pointIdx);
-      selectionBuffer[i * 2] = texCoords[0];
-      selectionBuffer[i * 2 + 1] = texCoords[1];
-    });
+
+    for (let i = selection.length - 1; i >= 0; i--) {
+      const pointIdx = selection[i];
+
+      if (pointIdx < 0 || pointIdx >= numPoints) {
+        // Remove invalid selection
+        selection.splice(i, 1);
+      } else {
+        selectionSet.add(pointIdx);
+        const texCoords = indexToStateTexCoord(pointIdx);
+        selectionBuffer.push(texCoords[0]);
+        selectionBuffer.push(texCoords[1]);
+      }
+    }
 
     selectedPointsIndexBuffer({
       usage: 'dynamic',
@@ -2606,7 +2616,7 @@ const createScatterplot = (initialProperties = {}) => {
   ) => {
     let needsRedraw = false;
 
-    if (point >= 0) {
+    if (point >= 0 && point < numPoints) {
       needsRedraw = true;
       const oldHoveredPoint = hoveredPoint;
       const newHoveredPoint = point !== hoveredPoint;

tests/index.js

@@ -1690,8 +1690,9 @@ test('select()', async (t) => {
 
   await wait(0);
 
-  t.ok(
-    flatArrayEqual([0, 2, 4], selectedPoints),
+  t.deepEqual(
+    selectedPoints,
+    [0, 2, 4],
     'should have selected point 0, 2, and 4'
   );
 
@@ -1716,11 +1717,38 @@ test('select()', async (t) => {
 
   await wait(0);
 
-  t.ok(
-    flatArrayEqual([0, 2, 4], selectedPoints),
+  t.deepEqual(
+    selectedPoints,
+    [0, 2, 4],
     'should have silently deselected points'
   );
 
+  scatterplot.select(2);
+
+  await wait(0);
+
+  t.deepEqual(selectedPoints, [2], 'should allow single point index selection');
+
+  scatterplot.select(-1);
+
+  await wait(0);
+
+  t.deepEqual(
+    selectedPoints,
+    [],
+    'should have not selected any point because -1 is invalid'
+  );
+
+  scatterplot.select([0, -1, 2, 4, 6]);
+
+  await wait(0);
+
+  t.deepEqual(
+    selectedPoints,
+    [0, 2, 4],
+    'should have filtered out invalid point selections'
+  );
+
   scatterplot.destroy();
 });