Introduce change of trace selection interface [1852]

rmoestl · rmoestl · commit 0eddcbc473df · 2018-07-18T13:32:10.000+02:00
- Reason: new approach is needed to support proper interplay of polygon and click select. See #1852 for a discussion. - New approach separates the two concerns 'what was selected' and 'set selected state of data points', that were previously handled by `selectPoints`, into multiple functions. - Also the polygons now longer implicitly carry selection state in cartesian/select.js but are only visual cues anymore. - Introduce a new Lib function for calculating the set difference of two arrays.
diff --git a/src/lib/index.js b/src/lib/index.js
@@ -72,6 +72,9 @@ lib.variance = statsModule.variance;
 lib.stdev = statsModule.stdev;
 lib.interp = statsModule.interp;
 
+var setOpsModule = require('./set_operations');
+lib.difference = setOpsModule.difference;
+
 var matrixModule = require('./matrix');
 lib.init2dArray = matrixModule.init2dArray;
 lib.transposeRagged = matrixModule.transposeRagged;
diff --git a/src/lib/set_operations.js b/src/lib/set_operations.js
@@ -0,0 +1,25 @@
+/**
+* Copyright 2012-2018, Plotly, Inc.
+* All rights reserved.
+*
+* This source code is licensed under the MIT license found in the
+* LICENSE file in the root directory of this source tree.
+*/
+
+'use strict';
+
+
+/*
+ * Computes the set difference of two arrays.
+ *
+ * Returns all elements of a that are not in b.
+ */
+function difference(a, b) {
+    return a.filter(function(e) {
+        return b.indexOf(e) < 0;
+    });
+}
+
+module.exports = {
+    difference: difference
+};
diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
@@ -15,6 +15,7 @@ var Registry = require('../../registry');
 var Color = require('../../components/color');
 var Fx = require('../../components/fx');
 
+var difference = require('../../lib/set_operations').difference;
 var polygon = require('../../lib/polygon');
 var throttle = require('../../lib/throttle');
 var makeEventData = require('../../components/fx/helpers').makeEventData;
@@ -51,6 +52,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
     var subtract = e.altKey;
 
     var filterPoly, testPoly, mergedPolygons, currentPolygon;
+    var pointsInPolygon = [];
     var i, cd, trace, searchInfo, eventData;
 
     var selectingOnSameSubplot = (
@@ -283,7 +285,15 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
                 for(i = 0; i < searchTraces.length; i++) {
                     searchInfo = searchTraces[i];
 
-                    traceSelection = searchInfo._module.selectPoints(searchInfo, testPoly, shouldRetainSelection(e));
+                    var currentPolygonTester = polygonTester(currentPolygon);
+                    var pointIds = searchInfo._module.getPointsIn(searchInfo, currentPolygonTester);
+                    traceSelection = searchInfo._module.selectPoints(searchInfo, pointIds);
+                    var pointsNoLongerSelected = difference(pointsInPolygon, pointIds);
+
+                    searchInfo._module.deselectPoints(searchInfo, pointsNoLongerSelected);
+                    pointsInPolygon = pointIds;
+
+                    // traceSelection = searchInfo._module.selectPoints(searchInfo, testPoly, shouldRetainSelection(e));
                     traceSelections.push(traceSelection);
 
                     thisSelection = fillSelectionItem(traceSelection, searchInfo);
@@ -313,7 +323,8 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
             if(numClicks === 2) {
                 for(i = 0; i < searchTraces.length; i++) {
                     searchInfo = searchTraces[i];
-                    searchInfo._module.selectPoints(searchInfo, false);
+                    // searchInfo._module.selectPoints(searchInfo, false);
+                    searchInfo._module.clearSelection(searchInfo);
                 }
 
                 updateSelectedState(gd, searchTraces);
@@ -371,23 +382,25 @@ function selectOnClick(gd, numClicks, evt) {
     if(selectPreconditionsMet) {
         var trace = calcData[0].trace,
             hoverDatum = hoverData[0],
-            module = trace._module;
+            module = trace._module,
+            searchInfo = _createSearchInfo(module, calcData, hoverDatum.xaxis, hoverDatum.yaxis);
 
         // Execute selection by delegating to respective module
         var retainSelection = shouldRetainSelection(evt),
             pointSelected = isPointSelected(trace, hoverDatum.pointNumber),
             onePointSelectedOnly = isOnePointSelectedOnly(trace);
 
+        if(!retainSelection) {
+            module.clearSelection(searchInfo);
+        }
+
         var shouldDeselectPoint = (pointSelected && onePointSelectedOnly) ||
           (pointSelected && !onePointSelectedOnly && retainSelection);
-
         var newTraceSelection = shouldDeselectPoint ?
-          module.deselectPoint(calcData, hoverDatum, retainSelection) :
-          module.selectPoint(calcData, hoverDatum, retainSelection);
+          module.deselectPoints(searchInfo, [hoverDatum.pointNumber]) :
+          module.selectPoints(searchInfo, [hoverDatum.pointNumber]);
 
         // Update selection state
-        var searchInfo =
-          _createSearchInfo(module, calcData, hoverDatum.xaxis, hoverDatum.yaxis);
         var selection = fillSelectionItem(newTraceSelection, searchInfo);
         var eventData = {points: selection};
 
diff --git a/src/traces/scatter/index.js b/src/traces/scatter/index.js
@@ -27,9 +27,10 @@ Scatter.colorbar = require('./marker_colorbar');
 Scatter.style = require('./style').style;
 Scatter.styleOnSelect = require('./style').styleOnSelect;
 Scatter.hoverPoints = require('./hover');
+Scatter.getPointsIn = require('./select').getPointsIn;
 Scatter.selectPoints = require('./select').selectPoints;
-Scatter.selectPoint = require('./select').selectPoint;
-Scatter.deselectPoint = require('./select').deselectPoint;
+Scatter.deselectPoints = require('./select').deselectPoints;
+Scatter.clearSelection = require('./select').clearSelection;
 Scatter.animatable = true;
 
 Scatter.moduleType = 'trace';
diff --git a/src/traces/scatter/select.js b/src/traces/scatter/select.js
@@ -11,86 +11,33 @@
 
 var subtypes = require('./subtypes');
 
-function selectPoints(searchInfo, polygon, retainOtherSelectModesState) {
-    var cd = searchInfo.cd,
-        xa = searchInfo.xaxis,
-        ya = searchInfo.yaxis,
-        selection = [],
-        trace = cd[0].trace,
-        i,
-        di,
-        x,
-        y;
-
-    var hasOnlyLines = (!subtypes.hasMarkers(trace) && !subtypes.hasText(trace));
-    if(hasOnlyLines) return [];
-
-    if(polygon === false) { // clear selection
-        _clearSelection(cd);
-    }
-    else {
-        for(i = 0; i < cd.length; i++) {
-            di = cd[i];
-            x = xa.c2p(di.x);
-            y = ya.c2p(di.y);
-
-            if(polygon.contains([x, y])) {
-                selection.push(_newSelectionItem(i, xa.c2d(di.x), ya.c2d(di.y)));
-                di.selected = 1;
-                di.selectedByPolygon = true;
-            } else {
-                if(retainOtherSelectModesState && !di.selectedByPolygon && di.selected === 1) {
-                    continue;
-                }
-                di.selected = 0;
-                delete di.selectedByPolygon;
-            }
-        }
-    }
-
-    return selection;
-}
-
-function selectPoint(calcData, hoverDataItem, retain) {
-    return _togglePointSelectedState(calcData, hoverDataItem, true, retain);
-}
-
-function deselectPoint(calcData, hoverDataItem, retain) {
-    return _togglePointSelectedState(calcData, hoverDataItem, false, retain);
-}
-
-function _togglePointSelectedState(calcData, hoverDataItem, selected, retain) {
+function _togglePointSelectedState(searchInfo, pointIds, selected) {
     var selection = [];
-    var selectedPointNumber = hoverDataItem.pointNumber;
-    var cdItem = calcData[selectedPointNumber];
 
-    if(!retain) _clearSelection(calcData);
+    var calcData = searchInfo.cd,
+        xAxis = searchInfo.xaxis,
+        yAxis = searchInfo.yaxis;
 
-    if(selected) {
-        cdItem.selected = 1;
-    } else {
-        cdItem.selected = 0;
+    // TODO use foreach?!
+    // Mutate state
+    for(var j = 0; j < pointIds.length; j++) {
+        var pointId = pointIds[j];
+        calcData[pointId].selected = selected ? 1 : 0;
     }
 
+    // Compute selection array from internal state
     for(var i = 0; i < calcData.length; i++) {
-        cdItem = calcData[i];
-        if(cdItem.selected === 1) {
+        if(calcData[i].selected === 1) {
             selection.push(_newSelectionItem(
               i,
-              hoverDataItem.xaxis.c2d(cdItem.x),
-              hoverDataItem.yaxis.c2d(cdItem.y)));
+              xAxis.c2d(calcData[i].x),
+              yAxis.c2d(calcData[i].y)));
         }
     }
 
     return selection;
 }
 
-function _clearSelection(calcData) {
-    for(var i = 0; i < calcData.length; i++) {
-        calcData[i].selected = 0;
-    }
-}
-
 // TODO May be needed in other trace types as well, so may centralize somewhere
 function _newSelectionItem(pointNumber, xInData, yInData) {
     return {
@@ -100,8 +47,45 @@ function _newSelectionItem(pointNumber, xInData, yInData) {
     };
 }
 
-module.exports = {
-    selectPoints: selectPoints,
-    selectPoint: selectPoint,
-    deselectPoint: deselectPoint
+function _clearSelection(calcData) {
+    for(var i = 0; i < calcData.length; i++) {
+        calcData[i].selected = 0;
+    }
+}
+
+exports.getPointsIn = function(searchInfo, polygon) {
+    var pointsIn = [];
+
+    var calcData = searchInfo.cd,
+        trace = calcData[0].trace,
+        xAxis = searchInfo.xaxis,
+        yAxis = searchInfo.yaxis,
+        i,
+        x, y;
+
+    var hasOnlyLines = !subtypes.hasMarkers(trace) && !subtypes.hasText(trace);
+    if(hasOnlyLines) return [];
+
+    for(i = 0; i < calcData.length; i++) {
+        x = xAxis.c2p(calcData[i].x);
+        y = yAxis.c2p(calcData[i].y);
+
+        if(polygon.contains([x, y])) {
+            pointsIn.push(i);
+        }
+    }
+
+    return pointsIn;
+};
+
+exports.selectPoints = function(searchInfo, pointIds) {
+    return _togglePointSelectedState(searchInfo, pointIds, true);
+};
+
+exports.deselectPoints = function(searchInfo, pointIds) {
+    return _togglePointSelectedState(searchInfo, pointIds, false);
+};
+
+exports.clearSelection = function(searchInfo) {
+    _clearSelection(searchInfo.cd);
 };
