From 25c56e42ce4630d84bf1725e7edfa0cc1bdb3bb2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Fri, 18 May 2018 15:03:36 +0200
Subject: [PATCH 01/45] Add click-to-select behavior for scatter [1852]

- Just a proof of concept only working
  for scatter trace type and with
  single trace plots only.
---
 src/plots/cartesian/dragbox.js | 19 +++++++++++-
 src/plots/cartesian/select.js  | 56 ++++++++++++++++++++++++++++++++--
 src/traces/scatter/index.js    |  3 +-
 src/traces/scatter/select.js   | 48 +++++++++++++++++++++++------
 4 files changed, 112 insertions(+), 14 deletions(-)

diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js
index 8d66a4cb4a5..1928240e96c 100644
--- a/src/plots/cartesian/dragbox.js
+++ b/src/plots/cartesian/dragbox.js
@@ -30,6 +30,8 @@ var doTicksSingle = require('./axes').doTicksSingle;
 var getFromId = require('./axis_ids').getFromId;
 var prepSelect = require('./select').prepSelect;
 var clearSelect = require('./select').clearSelect;
+var selectOnClick = require('./select').selectOnClick;
+
 var scaleZoom = require('./scale_zoom');
 
 var constants = require('./constants');
@@ -171,13 +173,16 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
         if(dragModeNow === 'lasso') dragOptions.minDrag = 1;
         else dragOptions.minDrag = undefined;
 
+        // TODO Maybe rename into isBoxOrLassoSelect
         if(isSelectOrLasso(dragModeNow)) {
             dragOptions.xaxes = xaxes;
             dragOptions.yaxes = yaxes;
             // this attaches moveFn, clickFn, doneFn on dragOptions
+            // TODO Maybe rename the function to prepSelectOnDrag
             prepSelect(e, startX, startY, dragOptions, dragModeNow);
         } else {
             dragOptions.clickFn = clickFn;
+            // TODO Is this good? prepFn will always clear and reset the selection when not in lasso or select mode, but now we've a select mode on click, a selectOnClick behavior so to speak
             clearAndResetSelect();
 
             if(!allFixedRanges) {
@@ -212,8 +217,11 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
         if(numClicks === 2 && !singleEnd) doubleClick();
 
         if(isMainDrag) {
-            Fx.click(gd, evt, plotinfo.id);
+            var clickHandler = obtainClickHandler();
+            clickHandler(gd, numClicks);
         }
+        // Allow manual editing of range bounds through an input field
+        // TODO consider extracting that to a method for clarity
         else if(numClicks === 1 && singleEnd) {
             var ax = ns ? ya0 : xa0,
                 end = (ns === 's' || ew === 'w') ? 0 : 1,
@@ -620,6 +628,15 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
         redrawObjs(gd._fullLayout.images || [], Registry.getComponentMethod('images', 'draw'), true);
     }
 
+    function obtainClickHandler() {
+        // TODO differentiate based on `clickmode` attr here
+        // Thought: we may end up returning thin wrapping functions that
+        // call the real functions based on the `clickmode` attr
+
+        // return Fx.click;
+        return selectOnClick;
+    }
+
     function doubleClick() {
         if(gd._transitioningWithDuration) return;
 
diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 498dfff60d2..bc3271453d2 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -278,6 +278,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
             function() {
                 selection = [];
 
+                // TODO What's the point of maintaining traceSelections array? Not used anywhere. Delete it.
                 var thisSelection, traceSelections = [], traceSelection;
                 for(i = 0; i < searchTraces.length; i++) {
                     searchInfo = searchTraces[i];
@@ -308,9 +309,10 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 
         throttle.done(throttleID).then(function() {
             throttle.clear(throttleID);
+
+            // clear visual boundaries of selection area if displayed
+            outlines.remove();
             if(numClicks === 2) {
-                // clear selection on doubleclick
-                outlines.remove();
                 for(i = 0; i < searchTraces.length; i++) {
                     searchInfo = searchTraces[i];
                     searchInfo._module.selectPoints(searchInfo, false);
@@ -320,6 +322,9 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
                 gd.emit('plotly_deselect', null);
             }
             else {
+                // TODO What to do with the code below because we now have behavior for a single click
+                selectOnClick(gd, numClicks);
+
                 // TODO: remove in v2 - this was probably never intended to work as it does,
                 // but in case anyone depends on it we don't want to break it now.
                 gd.emit('plotly_selected', undefined);
@@ -349,6 +354,50 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
     };
 }
 
+// Missing features
+// ----------------
+// TODO handle clearing selection, really?
+// TODO handle un-selecting a data-point
+// TODO do we have to consider multiple traces?
+function selectOnClick(gd, numClicks) {
+    var calcData = gd.calcdata[0];
+
+    var hoverData = gd._hoverdata;
+
+    var isHoverDataSet = hoverData && hoverData.length > 0;
+    var isSingleClick = numClicks === 1;
+    var selectPreconditionsMet = isHoverDataSet && isSingleClick;
+
+    if(selectPreconditionsMet) {
+        var pointAlreadySelected = false;
+        if(pointAlreadySelected) {
+            // Un-select data point
+        }
+        else {
+            var trace = calcData[0].trace;
+            var hoverDataItem = hoverData[0];
+            var traceSelection = trace._module.selectPoint(calcData, hoverDataItem);
+
+            var searchInfo =
+              _createSearchInfo(trace._module, calcData, hoverDataItem.xaxis, hoverDataItem.yaxis);
+            var selection = fillSelectionItem(traceSelection, searchInfo);
+            var eventData = {points: selection};
+
+            updateSelectedState(gd, [searchInfo], eventData);
+        }
+    }
+}
+
+// TODO Consider using in other places around here as well
+function _createSearchInfo(module, calcData, xaxis, yaxis) {
+    return {
+        _module: module,
+        cd: calcData,
+        xaxis: xaxis,
+        yaxis: yaxis
+    };
+}
+
 function updateSelectedState(gd, searchTraces, eventData) {
     var i, j, searchInfo, trace;
 
@@ -471,5 +520,6 @@ function clearSelect(zoomlayer) {
 
 module.exports = {
     prepSelect: prepSelect,
-    clearSelect: clearSelect
+    clearSelect: clearSelect,
+    selectOnClick: selectOnClick
 };
diff --git a/src/traces/scatter/index.js b/src/traces/scatter/index.js
index 020bc06a511..2cef6690604 100644
--- a/src/traces/scatter/index.js
+++ b/src/traces/scatter/index.js
@@ -27,7 +27,8 @@ Scatter.colorbar = require('./marker_colorbar');
 Scatter.style = require('./style').style;
 Scatter.styleOnSelect = require('./style').styleOnSelect;
 Scatter.hoverPoints = require('./hover');
-Scatter.selectPoints = require('./select');
+Scatter.selectPoints = require('./select').selectPoints;
+Scatter.selectPoint = require('./select').selectPoint;
 Scatter.animatable = true;
 
 Scatter.moduleType = 'trace';
diff --git a/src/traces/scatter/select.js b/src/traces/scatter/select.js
index 5d10050b494..11b4ebba14c 100644
--- a/src/traces/scatter/select.js
+++ b/src/traces/scatter/select.js
@@ -11,7 +11,7 @@
 
 var subtypes = require('./subtypes');
 
-module.exports = function selectPoints(searchInfo, polygon) {
+function selectPoints(searchInfo, polygon) {
     var cd = searchInfo.cd,
         xa = searchInfo.xaxis,
         ya = searchInfo.yaxis,
@@ -26,9 +26,7 @@ module.exports = function selectPoints(searchInfo, polygon) {
     if(hasOnlyLines) return [];
 
     if(polygon === false) { // clear selection
-        for(i = 0; i < cd.length; i++) {
-            cd[i].selected = 0;
-        }
+        _clearSelection(cd);
     }
     else {
         for(i = 0; i < cd.length; i++) {
@@ -37,11 +35,7 @@ module.exports = function selectPoints(searchInfo, polygon) {
             y = ya.c2p(di.y);
 
             if(polygon.contains([x, y])) {
-                selection.push({
-                    pointNumber: i,
-                    x: xa.c2d(di.x),
-                    y: ya.c2d(di.y)
-                });
+                selection.push(_newSelectionItem(i, xa.c2d(di.x), ya.c2d(di.y)));
                 di.selected = 1;
             } else {
                 di.selected = 0;
@@ -50,4 +44,40 @@ module.exports = function selectPoints(searchInfo, polygon) {
     }
 
     return selection;
+}
+
+function selectPoint(calcData, hoverDataItem) {
+    var selection = [];
+    var selectedPointNumber = hoverDataItem.pointNumber;
+    var cdItem = calcData[selectedPointNumber];
+
+    _clearSelection(calcData);
+
+    cdItem.selected = 1;
+    selection.push(_newSelectionItem(
+      selectedPointNumber,
+      hoverDataItem.xaxis.c2d(cdItem.x),
+      hoverDataItem.yaxis.c2d(cdItem.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 {
+        pointNumber: pointNumber,
+        x: xInData,
+        y: yInData
+    };
+}
+
+module.exports = {
+    selectPoints: selectPoints,
+    selectPoint: selectPoint
 };

From 5602335d2498adb237700b435ff05bb15d25d649 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 7 Jun 2018 14:25:12 +0200
Subject: [PATCH 02/45] Support deselect on click for scatter [1852]

- Selection state doesn't move back to nothing selected state though.
---
 src/plots/cartesian/select.js | 45 ++++++++++++++++++++++-------------
 src/traces/scatter/index.js   |  1 +
 src/traces/scatter/select.js  | 25 ++++++++++++++-----
 3 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index bc3271453d2..216373971a4 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -369,25 +369,27 @@ function selectOnClick(gd, numClicks) {
     var selectPreconditionsMet = isHoverDataSet && isSingleClick;
 
     if(selectPreconditionsMet) {
-        var pointAlreadySelected = false;
-        if(pointAlreadySelected) {
-            // Un-select data point
-        }
-        else {
-            var trace = calcData[0].trace;
-            var hoverDataItem = hoverData[0];
-            var traceSelection = trace._module.selectPoint(calcData, hoverDataItem);
-
-            var searchInfo =
-              _createSearchInfo(trace._module, calcData, hoverDataItem.xaxis, hoverDataItem.yaxis);
-            var selection = fillSelectionItem(traceSelection, searchInfo);
-            var eventData = {points: selection};
-
-            updateSelectedState(gd, [searchInfo], eventData);
-        }
+        var trace = calcData[0].trace;
+        var hoverDatum = hoverData[0];
+
+        var pointAlreadySelected = isPointAlreadySelected(trace, hoverDatum.pointNumber);
+        var traceSelection = pointAlreadySelected ?
+          trace._module.deselectPoint(calcData, hoverDatum) :
+          trace._module.selectPoint(calcData, hoverDatum);
+        var searchInfo =
+          _createSearchInfo(trace._module, calcData, hoverDatum.xaxis, hoverDatum.yaxis);
+        var selection = fillSelectionItem(traceSelection, searchInfo);
+        var eventData = {points: selection};
+
+        updateSelectedState(gd, [searchInfo], eventData);
     }
 }
 
+function isPointAlreadySelected(trace, pointNumber) {
+    if(!trace.selectedpoints && !Array.isArray(trace.selectedpoints)) return false;
+    return trace.selectedpoints.indexOf(pointNumber) > -1;
+}
+
 // TODO Consider using in other places around here as well
 function _createSearchInfo(module, calcData, xaxis, yaxis) {
     return {
@@ -398,6 +400,17 @@ function _createSearchInfo(module, calcData, xaxis, yaxis) {
     };
 }
 
+/**
+ * Updates the selection state properties of the passed traces
+ * and initiates proper selection styling.
+ *
+ * If no eventData is passed, the selection state is cleared
+ * for the traces passed.
+ *
+ * @param gd
+ * @param searchTraces
+ * @param eventData
+ */
 function updateSelectedState(gd, searchTraces, eventData) {
     var i, j, searchInfo, trace;
 
diff --git a/src/traces/scatter/index.js b/src/traces/scatter/index.js
index 2cef6690604..f1b5c64b8d3 100644
--- a/src/traces/scatter/index.js
+++ b/src/traces/scatter/index.js
@@ -29,6 +29,7 @@ Scatter.styleOnSelect = require('./style').styleOnSelect;
 Scatter.hoverPoints = require('./hover');
 Scatter.selectPoints = require('./select').selectPoints;
 Scatter.selectPoint = require('./select').selectPoint;
+Scatter.deselectPoint = require('./select').deselectPoint;
 Scatter.animatable = true;
 
 Scatter.moduleType = 'trace';
diff --git a/src/traces/scatter/select.js b/src/traces/scatter/select.js
index 11b4ebba14c..461f8049abd 100644
--- a/src/traces/scatter/select.js
+++ b/src/traces/scatter/select.js
@@ -47,17 +47,29 @@ function selectPoints(searchInfo, polygon) {
 }
 
 function selectPoint(calcData, hoverDataItem) {
+    return _togglePointSelectedState(calcData, hoverDataItem, true);
+}
+
+function deselectPoint(calcData, hoverDataItem) {
+    return _togglePointSelectedState(calcData, hoverDataItem, false);
+}
+
+function _togglePointSelectedState(calcData, hoverDataItem, selected) {
     var selection = [];
     var selectedPointNumber = hoverDataItem.pointNumber;
     var cdItem = calcData[selectedPointNumber];
 
     _clearSelection(calcData);
 
-    cdItem.selected = 1;
-    selection.push(_newSelectionItem(
-      selectedPointNumber,
-      hoverDataItem.xaxis.c2d(cdItem.x),
-      hoverDataItem.yaxis.c2d(cdItem.y)));
+    if(selected) {
+        cdItem.selected = 1;
+        selection.push(_newSelectionItem(
+          selectedPointNumber,
+          hoverDataItem.xaxis.c2d(cdItem.x),
+          hoverDataItem.yaxis.c2d(cdItem.y)));
+    } else {
+        cdItem.selected = 0;
+    }
 
     return selection;
 }
@@ -79,5 +91,6 @@ function _newSelectionItem(pointNumber, xInData, yInData) {
 
 module.exports = {
     selectPoints: selectPoints,
-    selectPoint: selectPoint
+    selectPoint: selectPoint,
+    deselectPoint: deselectPoint
 };

From f4243b4515dd14f3c7675465109b90d043d3f869 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 7 Jun 2018 14:33:52 +0200
Subject: [PATCH 03/45] Transition to no selection style after deselecting a
 data point [1852]

- Note: discussion needed if new behavior is wanted.
---
 src/plots/cartesian/select.js | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 216373971a4..c9f4f2673ab 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -356,8 +356,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 
 // Missing features
 // ----------------
-// TODO handle clearing selection, really?
-// TODO handle un-selecting a data-point
+// TODO handle clearing selection when no point is clicked (based on hoverData)
 // TODO do we have to consider multiple traces?
 function selectOnClick(gd, numClicks) {
     var calcData = gd.calcdata[0];
@@ -414,8 +413,13 @@ function _createSearchInfo(module, calcData, xaxis, yaxis) {
 function updateSelectedState(gd, searchTraces, eventData) {
     var i, j, searchInfo, trace;
 
-    if(eventData) {
-        var pts = eventData.points || [];
+    // TODO previously eventData without a point would still set a selection
+    // and all points would appear as non-selected. Moving to another drag mode like
+    // zoom would leave this state. Discuss if the new behavior is better.
+    var selectionNonEmpty = eventData && eventData.points && eventData.points.length > 0;
+    if(selectionNonEmpty) {
+        // var pts = eventData.points || []; TODO remove eventually
+        var pts = eventData.points;
 
         for(i = 0; i < searchTraces.length; i++) {
             trace = searchTraces[i].cd[0].trace;

From b552ccf027a8acc7ff324249ecf2f87e926e1175 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 7 Jun 2018 15:26:58 +0200
Subject: [PATCH 04/45] Allow to retain current selection with pressing Shift
 key [1852]

---
 src/plots/cartesian/dragbox.js |  2 +-
 src/plots/cartesian/select.js  |  9 +++++----
 src/traces/scatter/select.js   | 26 ++++++++++++++++----------
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js
index 1928240e96c..5fdf3513806 100644
--- a/src/plots/cartesian/dragbox.js
+++ b/src/plots/cartesian/dragbox.js
@@ -218,7 +218,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
 
         if(isMainDrag) {
             var clickHandler = obtainClickHandler();
-            clickHandler(gd, numClicks);
+            clickHandler(gd, numClicks, evt);
         }
         // Allow manual editing of range bounds through an input field
         // TODO consider extracting that to a method for clarity
diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index c9f4f2673ab..5bc0ff2d6f9 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -323,7 +323,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
             }
             else {
                 // TODO What to do with the code below because we now have behavior for a single click
-                selectOnClick(gd, numClicks);
+                selectOnClick(gd, numClicks, evt);
 
                 // TODO: remove in v2 - this was probably never intended to work as it does,
                 // but in case anyone depends on it we don't want to break it now.
@@ -358,7 +358,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 // ----------------
 // TODO handle clearing selection when no point is clicked (based on hoverData)
 // TODO do we have to consider multiple traces?
-function selectOnClick(gd, numClicks) {
+function selectOnClick(gd, numClicks, evt) {
     var calcData = gd.calcdata[0];
 
     var hoverData = gd._hoverdata;
@@ -371,10 +371,11 @@ function selectOnClick(gd, numClicks) {
         var trace = calcData[0].trace;
         var hoverDatum = hoverData[0];
 
+        var retainCurrentSelection = evt.shiftKey;
         var pointAlreadySelected = isPointAlreadySelected(trace, hoverDatum.pointNumber);
         var traceSelection = pointAlreadySelected ?
-          trace._module.deselectPoint(calcData, hoverDatum) :
-          trace._module.selectPoint(calcData, hoverDatum);
+          trace._module.deselectPoint(calcData, hoverDatum, retainCurrentSelection) :
+          trace._module.selectPoint(calcData, hoverDatum, retainCurrentSelection);
         var searchInfo =
           _createSearchInfo(trace._module, calcData, hoverDatum.xaxis, hoverDatum.yaxis);
         var selection = fillSelectionItem(traceSelection, searchInfo);
diff --git a/src/traces/scatter/select.js b/src/traces/scatter/select.js
index 461f8049abd..985c7e8bee5 100644
--- a/src/traces/scatter/select.js
+++ b/src/traces/scatter/select.js
@@ -46,31 +46,37 @@ function selectPoints(searchInfo, polygon) {
     return selection;
 }
 
-function selectPoint(calcData, hoverDataItem) {
-    return _togglePointSelectedState(calcData, hoverDataItem, true);
+function selectPoint(calcData, hoverDataItem, retain) {
+    return _togglePointSelectedState(calcData, hoverDataItem, true, retain);
 }
 
-function deselectPoint(calcData, hoverDataItem) {
-    return _togglePointSelectedState(calcData, hoverDataItem, false);
+function deselectPoint(calcData, hoverDataItem, retain) {
+    return _togglePointSelectedState(calcData, hoverDataItem, false, retain);
 }
 
-function _togglePointSelectedState(calcData, hoverDataItem, selected) {
+function _togglePointSelectedState(calcData, hoverDataItem, selected, retain) {
     var selection = [];
     var selectedPointNumber = hoverDataItem.pointNumber;
     var cdItem = calcData[selectedPointNumber];
 
-    _clearSelection(calcData);
+    if(!retain) _clearSelection(calcData);
 
     if(selected) {
         cdItem.selected = 1;
-        selection.push(_newSelectionItem(
-          selectedPointNumber,
-          hoverDataItem.xaxis.c2d(cdItem.x),
-          hoverDataItem.yaxis.c2d(cdItem.y)));
     } else {
         cdItem.selected = 0;
     }
 
+    for(var i = 0; i < calcData.length; i++) {
+        cdItem = calcData[i];
+        if(cdItem.selected === 1) {
+            selection.push(_newSelectionItem(
+              i,
+              hoverDataItem.xaxis.c2d(cdItem.x),
+              hoverDataItem.yaxis.c2d(cdItem.y)));
+        }
+    }
+
     return selection;
 }
 

From b9c893f26815ab1e0f7f22fcc854e72c5d99a28d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 7 Jun 2018 17:24:09 +0200
Subject: [PATCH 05/45] Take retention mode into acount on deselect [1852]

- Given multiple points selected, deselecting one of those should
  start a new selection, or when Shift is pressed the point should be
  removed from retained selection.
---
 src/plots/cartesian/select.js | 37 ++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 5bc0ff2d6f9..7a5c83126d7 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -358,6 +358,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 // ----------------
 // TODO handle clearing selection when no point is clicked (based on hoverData)
 // TODO do we have to consider multiple traces?
+// TODO implement selection retention (Shift key) for lasso and box
 function selectOnClick(gd, numClicks, evt) {
     var calcData = gd.calcdata[0];
 
@@ -368,28 +369,42 @@ function selectOnClick(gd, numClicks, evt) {
     var selectPreconditionsMet = isHoverDataSet && isSingleClick;
 
     if(selectPreconditionsMet) {
-        var trace = calcData[0].trace;
-        var hoverDatum = hoverData[0];
-
-        var retainCurrentSelection = evt.shiftKey;
-        var pointAlreadySelected = isPointAlreadySelected(trace, hoverDatum.pointNumber);
-        var traceSelection = pointAlreadySelected ?
-          trace._module.deselectPoint(calcData, hoverDatum, retainCurrentSelection) :
-          trace._module.selectPoint(calcData, hoverDatum, retainCurrentSelection);
+        var trace = calcData[0].trace,
+            hoverDatum = hoverData[0],
+            module = trace._module;
+
+        // Execute selection by delegating to respective module
+        var retainSelection = evt.shiftKey,
+            pointSelected = isPointSelected(trace, hoverDatum.pointNumber),
+            onePointSelectedOnly = isOnePointSelectedOnly(trace);
+
+        var shouldDeselectPoint = (pointSelected && onePointSelectedOnly) ||
+          (pointSelected && !onePointSelectedOnly && retainSelection);
+
+        var newTraceSelection = shouldDeselectPoint ?
+          module.deselectPoint(calcData, hoverDatum, retainSelection) :
+          module.selectPoint(calcData, hoverDatum, retainSelection);
+
+        // Update selection state
         var searchInfo =
-          _createSearchInfo(trace._module, calcData, hoverDatum.xaxis, hoverDatum.yaxis);
-        var selection = fillSelectionItem(traceSelection, searchInfo);
+          _createSearchInfo(module, calcData, hoverDatum.xaxis, hoverDatum.yaxis);
+        var selection = fillSelectionItem(newTraceSelection, searchInfo);
         var eventData = {points: selection};
 
         updateSelectedState(gd, [searchInfo], eventData);
     }
 }
 
-function isPointAlreadySelected(trace, pointNumber) {
+function isPointSelected(trace, pointNumber) {
     if(!trace.selectedpoints && !Array.isArray(trace.selectedpoints)) return false;
     return trace.selectedpoints.indexOf(pointNumber) > -1;
 }
 
+function isOnePointSelectedOnly(trace) {
+    if(!trace.selectedpoints && !Array.isArray(trace.selectedpoints)) return false;
+    return trace.selectedpoints.length === 1;
+}
+
 // TODO Consider using in other places around here as well
 function _createSearchInfo(module, calcData, xaxis, yaxis) {
     return {

From c1925eae7ecaf06b60eb0628f30df3249ced5683 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 14 Jun 2018 15:57:57 +0200
Subject: [PATCH 06/45] Retain click-selected points in box or lasso select
 mode [1852]

- If the Shift key is down, data points previously selected by a click
  are now retained when user adds to a selection through box or lasso
  select mode.
---
 src/plots/cartesian/select.js | 16 ++++++++++------
 src/traces/scatter/select.js  |  7 ++++++-
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 7a5c83126d7..81ec8d5dbbc 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -71,7 +71,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
         (!e.shiftKey && !e.altKey) ||
         ((e.shiftKey || e.altKey) && !plotinfo.selection)
     ) {
-        // create new polygons, if shift mode or selecting across different subplots
+        // create new polygons, if not shift mode or selecting across different subplots
         plotinfo.selection = {};
         plotinfo.selection.polygons = dragOptions.polygons = [];
         plotinfo.selection.mergedPolygons = dragOptions.mergedPolygons = [];
@@ -283,7 +283,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
                 for(i = 0; i < searchTraces.length; i++) {
                     searchInfo = searchTraces[i];
 
-                    traceSelection = searchInfo._module.selectPoints(searchInfo, testPoly);
+                    traceSelection = searchInfo._module.selectPoints(searchInfo, testPoly, shouldRetainSelection(e));
                     traceSelections.push(traceSelection);
 
                     thisSelection = fillSelectionItem(traceSelection, searchInfo);
@@ -310,8 +310,6 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
         throttle.done(throttleID).then(function() {
             throttle.clear(throttleID);
 
-            // clear visual boundaries of selection area if displayed
-            outlines.remove();
             if(numClicks === 2) {
                 for(i = 0; i < searchTraces.length; i++) {
                     searchInfo = searchTraces[i];
@@ -320,6 +318,9 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 
                 updateSelectedState(gd, searchTraces);
                 gd.emit('plotly_deselect', null);
+
+                // clear visual boundaries of selection area if displayed
+                outlines.remove();
             }
             else {
                 // TODO What to do with the code below because we now have behavior for a single click
@@ -358,7 +359,6 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 // ----------------
 // TODO handle clearing selection when no point is clicked (based on hoverData)
 // TODO do we have to consider multiple traces?
-// TODO implement selection retention (Shift key) for lasso and box
 function selectOnClick(gd, numClicks, evt) {
     var calcData = gd.calcdata[0];
 
@@ -374,7 +374,7 @@ function selectOnClick(gd, numClicks, evt) {
             module = trace._module;
 
         // Execute selection by delegating to respective module
-        var retainSelection = evt.shiftKey,
+        var retainSelection = shouldRetainSelection(evt),
             pointSelected = isPointSelected(trace, hoverDatum.pointNumber),
             onePointSelectedOnly = isOnePointSelectedOnly(trace);
 
@@ -395,6 +395,10 @@ function selectOnClick(gd, numClicks, evt) {
     }
 }
 
+function shouldRetainSelection(evt) {
+    return evt.shiftKey;
+}
+
 function isPointSelected(trace, pointNumber) {
     if(!trace.selectedpoints && !Array.isArray(trace.selectedpoints)) return false;
     return trace.selectedpoints.indexOf(pointNumber) > -1;
diff --git a/src/traces/scatter/select.js b/src/traces/scatter/select.js
index 985c7e8bee5..ddd9d2a0588 100644
--- a/src/traces/scatter/select.js
+++ b/src/traces/scatter/select.js
@@ -11,7 +11,7 @@
 
 var subtypes = require('./subtypes');
 
-function selectPoints(searchInfo, polygon) {
+function selectPoints(searchInfo, polygon, retainOtherSelectModesState) {
     var cd = searchInfo.cd,
         xa = searchInfo.xaxis,
         ya = searchInfo.yaxis,
@@ -37,8 +37,13 @@ function selectPoints(searchInfo, polygon) {
             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;
             }
         }
     }

From 0eddcbc473dfd5483197bc188dfa2212e850f089 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 21 Jun 2018 11:44:18 +0200
Subject: [PATCH 07/45] Introduce change of trace selection interface [1852]

- Reason: new approach is needed to support proper
  interplay of polygon and click select. See
  https://github.com/plotly/plotly.js/issues/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.
---
 src/lib/index.js              |   3 +
 src/lib/set_operations.js     |  25 +++++++
 src/plots/cartesian/select.js |  29 +++++---
 src/traces/scatter/index.js   |   5 +-
 src/traces/scatter/select.js  | 124 +++++++++++++++-------------------
 5 files changed, 106 insertions(+), 80 deletions(-)
 create mode 100644 src/lib/set_operations.js

diff --git a/src/lib/index.js b/src/lib/index.js
index 6e0a3e24a61..b8d536dfb7a 100644
--- 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
new file mode 100644
index 00000000000..6119c3d402c
--- /dev/null
+++ 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
index 81ec8d5dbbc..7c4681411af 100644
--- 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
index f1b5c64b8d3..ab4f289b3da 100644
--- 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
index ddd9d2a0588..b53b35ded06 100644
--- 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);
 };

From ca80c894a6849c996e13a04ffdd6aae74809daf0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 21 Jun 2018 15:19:09 +0200
Subject: [PATCH 08/45] Implement polygon subtract mode in new approach [1852]

---
 src/plots/cartesian/select.js | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 7c4681411af..ad8a2c7cbf0 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -111,7 +111,6 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
     // find the traces to search for selection points
     var searchTraces = [];
     var throttleID = fullLayout._uid + constants.SELECTID;
-    var selection = [];
 
     for(i = 0; i < gd.calcdata.length; i++) {
         cd = gd.calcdata[i];
@@ -278,22 +277,31 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
             throttleID,
             constants.SELECTDELAY,
             function() {
-                selection = [];
+                var selection = [],
+                    retainSelection = shouldRetainSelection(e),
+                    module,
+                    searchInfo;
 
                 // TODO What's the point of maintaining traceSelections array? Not used anywhere. Delete it.
                 var thisSelection, traceSelections = [], traceSelection;
                 for(i = 0; i < searchTraces.length; i++) {
                     searchInfo = searchTraces[i];
+                    module = searchInfo._module;
+
+                    if(!retainSelection) module.clearSelection(searchInfo);
 
                     var currentPolygonTester = polygonTester(currentPolygon);
-                    var pointIds = searchInfo._module.getPointsIn(searchInfo, currentPolygonTester);
-                    traceSelection = searchInfo._module.selectPoints(searchInfo, pointIds);
+                    var pointIds = module.getPointsIn(searchInfo, currentPolygonTester);
+                    if(!subtract) {
+                        module.selectPoints(searchInfo, pointIds);
+                    } else {
+                        module.deselectPoints(searchInfo, pointIds);
+                    }
                     var pointsNoLongerSelected = difference(pointsInPolygon, pointIds);
 
-                    searchInfo._module.deselectPoints(searchInfo, pointsNoLongerSelected);
+                    traceSelection = module.deselectPoints(searchInfo, pointsNoLongerSelected);
                     pointsInPolygon = pointIds;
 
-                    // traceSelection = searchInfo._module.selectPoints(searchInfo, testPoly, shouldRetainSelection(e));
                     traceSelections.push(traceSelection);
 
                     thisSelection = fillSelectionItem(traceSelection, searchInfo);
@@ -370,6 +378,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 // ----------------
 // TODO handle clearing selection when no point is clicked (based on hoverData)
 // TODO do we have to consider multiple traces?
+// TODO a click without shift doesn't remove previous polygon outlines
 function selectOnClick(gd, numClicks, evt) {
     var calcData = gd.calcdata[0];
 
@@ -390,9 +399,7 @@ function selectOnClick(gd, numClicks, evt) {
             pointSelected = isPointSelected(trace, hoverDatum.pointNumber),
             onePointSelectedOnly = isOnePointSelectedOnly(trace);
 
-        if(!retainSelection) {
-            module.clearSelection(searchInfo);
-        }
+        if(!retainSelection) module.clearSelection(searchInfo);
 
         var shouldDeselectPoint = (pointSelected && onePointSelectedOnly) ||
           (pointSelected && !onePointSelectedOnly && retainSelection);

From 2fb35cfac2181389b4641b44b8fb8b3b35daa57c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 21 Jun 2018 15:27:51 +0200
Subject: [PATCH 09/45] Remove polygon outlines when select-on-click [1852]

---
 src/plots/cartesian/select.js | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index ad8a2c7cbf0..fa6358bdffd 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -343,7 +343,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
             }
             else {
                 // TODO What to do with the code below because we now have behavior for a single click
-                selectOnClick(gd, numClicks, evt);
+                selectOnClick(gd, numClicks, evt, outlines);
 
                 // TODO: remove in v2 - this was probably never intended to work as it does,
                 // but in case anyone depends on it we don't want to break it now.
@@ -378,8 +378,8 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 // ----------------
 // TODO handle clearing selection when no point is clicked (based on hoverData)
 // TODO do we have to consider multiple traces?
-// TODO a click without shift doesn't remove previous polygon outlines
-function selectOnClick(gd, numClicks, evt) {
+// TODO remove polygon outlines if last selected point is deselected and none get selected
+function selectOnClick(gd, numClicks, evt, outlines) {
     var calcData = gd.calcdata[0];
 
     var hoverData = gd._hoverdata;
@@ -407,6 +407,12 @@ function selectOnClick(gd, numClicks, evt) {
           module.deselectPoints(searchInfo, [hoverDatum.pointNumber]) :
           module.selectPoints(searchInfo, [hoverDatum.pointNumber]);
 
+        // When not retaining or when the sole selected
+        // point gets deselected, remove outlines
+        if(!retainSelection || (pointSelected && onePointSelectedOnly)) {
+            outlines.remove();
+        }
+
         // Update selection state
         var selection = fillSelectionItem(newTraceSelection, searchInfo);
         var eventData = {points: selection};

From 436a7464f08cbf0b0de2e75547a9d4fa11dc0e86 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 21 Jun 2018 16:55:43 +0200
Subject: [PATCH 10/45] Clean up click-on-select code [1852]

---
 src/plots/cartesian/select.js | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index fa6358bdffd..fbd2e8b4108 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -257,6 +257,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
         if(dragOptions.polygons && dragOptions.polygons.length) {
             mergedPolygons = mergePolygons(dragOptions.mergedPolygons, currentPolygon, subtract);
             currentPolygon.subtract = subtract;
+            // TODO Probably isn't needed anymore
             testPoly = multipolygonTester(dragOptions.polygons.concat([currentPolygon]));
         }
         else {
@@ -291,16 +292,16 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
                     if(!retainSelection) module.clearSelection(searchInfo);
 
                     var currentPolygonTester = polygonTester(currentPolygon);
-                    var pointIds = module.getPointsIn(searchInfo, currentPolygonTester);
+                    var pointsInCurrentPolygon = module.getPointsIn(searchInfo, currentPolygonTester);
                     if(!subtract) {
-                        module.selectPoints(searchInfo, pointIds);
+                        module.selectPoints(searchInfo, pointsInCurrentPolygon);
                     } else {
-                        module.deselectPoints(searchInfo, pointIds);
+                        module.deselectPoints(searchInfo, pointsInCurrentPolygon);
                     }
-                    var pointsNoLongerSelected = difference(pointsInPolygon, pointIds);
+                    var pointsNoLongerSelected = difference(pointsInPolygon, pointsInCurrentPolygon);
 
                     traceSelection = module.deselectPoints(searchInfo, pointsNoLongerSelected);
-                    pointsInPolygon = pointIds;
+                    pointsInPolygon = pointsInCurrentPolygon;
 
                     traceSelections.push(traceSelection);
 
@@ -378,7 +379,6 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 // ----------------
 // TODO handle clearing selection when no point is clicked (based on hoverData)
 // TODO do we have to consider multiple traces?
-// TODO remove polygon outlines if last selected point is deselected and none get selected
 function selectOnClick(gd, numClicks, evt, outlines) {
     var calcData = gd.calcdata[0];
 

From 64705e17017957447d9ac95b2314db7a2f0dcc61 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Tue, 26 Jun 2018 15:42:06 +0200
Subject: [PATCH 11/45] Fix bug in deletion of polygon select outlines [1852]

- Select-on-click wasn't checking if an outlines
  object was passed which is the case in zoom and
  pan mode for example.
---
 src/plots/cartesian/select.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index fbd2e8b4108..607ce25d114 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -409,7 +409,8 @@ function selectOnClick(gd, numClicks, evt, outlines) {
 
         // When not retaining or when the sole selected
         // point gets deselected, remove outlines
-        if(!retainSelection || (pointSelected && onePointSelectedOnly)) {
+        if(outlines &&
+          (!retainSelection || (pointSelected && onePointSelectedOnly))) {
             outlines.remove();
         }
 

From 3900d5c9fbfc0e5a293f9c13021d83fda91bfc6d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Tue, 26 Jun 2018 16:12:45 +0200
Subject: [PATCH 12/45] Condense traces' selection interface surface [1852]

- Instead of four functions only have two anymore.
- Also update modebar to detect a selectable trace.
- Also update supplyDefaults code.
---
 src/components/modebar/manage.js   |  5 ++---
 src/plots/cartesian/select.js      | 24 +++++++++-------------
 src/plots/plots.js                 |  2 +-
 src/traces/scatter/index.js        |  4 +---
 src/traces/scatter/select.js       | 33 +++++++++++++++---------------
 test/jasmine/tests/modebar_test.js | 14 ++++++-------
 6 files changed, 38 insertions(+), 44 deletions(-)

diff --git a/src/components/modebar/manage.js b/src/components/modebar/manage.js
index d455fcf8856..f4ee57af541 100644
--- a/src/components/modebar/manage.js
+++ b/src/components/modebar/manage.js
@@ -185,7 +185,6 @@ function areAllAxesFixed(fullLayout) {
 }
 
 // look for traces that support selection
-// to be updated as we add more selectPoints handlers
 function isSelectable(fullData) {
     var selectable = false;
 
@@ -194,7 +193,7 @@ function isSelectable(fullData) {
 
         var trace = fullData[i];
 
-        if(!trace._module || !trace._module.selectPoints) continue;
+        if(!trace._module || !trace._module.getPointsIn || !trace._module.toggleSelected) continue;
 
         if(Registry.traceIs(trace, 'scatter-like')) {
             if(scatterSubTypes.hasMarkers(trace) || scatterSubTypes.hasText(trace)) {
@@ -205,7 +204,7 @@ function isSelectable(fullData) {
                 selectable = true;
             }
         }
-        // assume that in general if the trace module has selectPoints,
+        // assume that in general if the trace module has getPointsIn and toggleSelected,
         // then it's selectable. Scatter is an exception to this because it must
         // have markers or text, not just be a scatter type.
         else selectable = true;
diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 607ce25d114..4c3a8a8a4b1 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -116,7 +116,8 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
         cd = gd.calcdata[i];
         trace = cd[0].trace;
 
-        if(trace.visible !== true || !trace._module || !trace._module.selectPoints) continue;
+        if(trace.visible !== true || !trace._module ||
+          !trace._module.toggleSelected || !trace._module.getPointsIn) continue;
 
         if(dragOptions.subplot) {
             if(
@@ -289,18 +290,15 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
                     searchInfo = searchTraces[i];
                     module = searchInfo._module;
 
-                    if(!retainSelection) module.clearSelection(searchInfo);
+                    if(!retainSelection) module.toggleSelected(searchInfo, false);
 
                     var currentPolygonTester = polygonTester(currentPolygon);
                     var pointsInCurrentPolygon = module.getPointsIn(searchInfo, currentPolygonTester);
-                    if(!subtract) {
-                        module.selectPoints(searchInfo, pointsInCurrentPolygon);
-                    } else {
-                        module.deselectPoints(searchInfo, pointsInCurrentPolygon);
-                    }
+                    module.toggleSelected(searchInfo, !subtract, pointsInCurrentPolygon);
+
                     var pointsNoLongerSelected = difference(pointsInPolygon, pointsInCurrentPolygon);
 
-                    traceSelection = module.deselectPoints(searchInfo, pointsNoLongerSelected);
+                    traceSelection = module.toggleSelected(searchInfo, false, pointsNoLongerSelected);
                     pointsInPolygon = pointsInCurrentPolygon;
 
                     traceSelections.push(traceSelection);
@@ -332,8 +330,7 @@ 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.clearSelection(searchInfo);
+                    searchInfo._module.toggleSelected(searchInfo, false);
                 }
 
                 updateSelectedState(gd, searchTraces);
@@ -399,13 +396,12 @@ function selectOnClick(gd, numClicks, evt, outlines) {
             pointSelected = isPointSelected(trace, hoverDatum.pointNumber),
             onePointSelectedOnly = isOnePointSelectedOnly(trace);
 
-        if(!retainSelection) module.clearSelection(searchInfo);
+        if(!retainSelection) module.toggleSelected(searchInfo, false);
 
         var shouldDeselectPoint = (pointSelected && onePointSelectedOnly) ||
           (pointSelected && !onePointSelectedOnly && retainSelection);
-        var newTraceSelection = shouldDeselectPoint ?
-          module.deselectPoints(searchInfo, [hoverDatum.pointNumber]) :
-          module.selectPoints(searchInfo, [hoverDatum.pointNumber]);
+        var newTraceSelection =
+          module.toggleSelected(searchInfo, !shouldDeselectPoint, [hoverDatum.pointNumber]);
 
         // When not retaining or when the sole selected
         // point gets deselected, remove outlines
diff --git a/src/plots/plots.js b/src/plots/plots.js
index 15fddfc844c..1b0774b083d 100644
--- a/src/plots/plots.js
+++ b/src/plots/plots.js
@@ -1171,7 +1171,7 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac
             traceOut.visible = !!traceOut.visible;
         }
 
-        if(_module && _module.selectPoints) {
+        if(_module && _module.getPointsIn && _module.toggleSelected) {
             coerce('selectedpoints');
         }
 
diff --git a/src/traces/scatter/index.js b/src/traces/scatter/index.js
index ab4f289b3da..4d365304745 100644
--- a/src/traces/scatter/index.js
+++ b/src/traces/scatter/index.js
@@ -28,9 +28,7 @@ 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.deselectPoints = require('./select').deselectPoints;
-Scatter.clearSelection = require('./select').clearSelection;
+Scatter.toggleSelected = require('./select').toggleSelected;
 Scatter.animatable = true;
 
 Scatter.moduleType = 'trace';
diff --git a/src/traces/scatter/select.js b/src/traces/scatter/select.js
index b53b35ded06..0d00de0a2f5 100644
--- a/src/traces/scatter/select.js
+++ b/src/traces/scatter/select.js
@@ -47,12 +47,6 @@ function _newSelectionItem(pointNumber, xInData, yInData) {
     };
 }
 
-function _clearSelection(calcData) {
-    for(var i = 0; i < calcData.length; i++) {
-        calcData[i].selected = 0;
-    }
-}
-
 exports.getPointsIn = function(searchInfo, polygon) {
     var pointsIn = [];
 
@@ -78,14 +72,21 @@ exports.getPointsIn = function(searchInfo, polygon) {
     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);
+/**
+ * Update the selected flag of the given points. Omitting which points
+ * to modify will update all points of the passed trace.
+ *
+ * @param searchInfo - info about trace to modify
+ * @param {boolean} selected - are these points to be selected (true) or deselected (false)
+ * @param {integer[]} pointsIds - the points to modify - omit to modify all points
+ *        in the trace. i.e. clearSelection is toggleSelection(searchInfo, false).
+ */
+exports.toggleSelected = function(searchInfo, selected, pointsIds) {
+    if(!Array.isArray(pointsIds)) {
+        pointsIds = [];
+        for(var i = 0; i < searchInfo.cd.length; i++) {
+            pointsIds.push(i);
+        }
+    }
+    return _togglePointSelectedState(searchInfo, pointsIds, selected);
 };
diff --git a/test/jasmine/tests/modebar_test.js b/test/jasmine/tests/modebar_test.js
index d0e447d21a4..31b5c8acfeb 100644
--- a/test/jasmine/tests/modebar_test.js
+++ b/test/jasmine/tests/modebar_test.js
@@ -356,7 +356,7 @@ describe('ModeBar', function() {
                 type: 'scatter',
                 visible: true,
                 mode: 'markers',
-                _module: {selectPoints: true}
+                _module: {getPointsIn: true, toggleSelected: true}
             }];
 
             manageModeBar(gd);
@@ -380,7 +380,7 @@ describe('ModeBar', function() {
                 type: 'box',
                 visible: true,
                 boxpoints: 'all',
-                _module: {selectPoints: true}
+                _module: {getPointsIn: true, toggleSelected: true}
             }];
 
             manageModeBar(gd);
@@ -452,7 +452,7 @@ describe('ModeBar', function() {
                 type: 'scattergeo',
                 visible: true,
                 mode: 'markers',
-                _module: {selectPoints: true}
+                _module: {getPointsIn: true, toggleSelected: true}
             }];
 
             manageModeBar(gd);
@@ -492,7 +492,7 @@ describe('ModeBar', function() {
                 type: 'scatter',
                 visible: true,
                 mode: 'markers',
-                _module: {selectPoints: true}
+                _module: {getPointsIn: true, toggleSelected: true}
             }];
 
             manageModeBar(gd);
@@ -584,7 +584,7 @@ describe('ModeBar', function() {
                 type: 'scatter',
                 visible: true,
                 mode: 'markers',
-                _module: {selectPoints: true}
+                _module: {getPointsIn: true, toggleSelected: true}
             }];
 
             manageModeBar(gd);
@@ -606,7 +606,7 @@ describe('ModeBar', function() {
                 type: 'scatter',
                 visible: true,
                 mode: 'markers',
-                _module: {selectPoints: true}
+                _module: {getPointsIn: true, toggleSelected: true}
             }];
             gd._fullLayout.xaxis = {fixedrange: false};
             gd._fullLayout._basePlotModules = [{ name: 'cartesian' }, { name: 'pie' }];
@@ -662,7 +662,7 @@ describe('ModeBar', function() {
                 type: 'scatterternary',
                 visible: true,
                 mode: 'markers',
-                _module: {selectPoints: true}
+                _module: {getPointsIn: true, toggleSelected: true}
             }];
             gd._fullLayout._basePlotModules = [{ name: 'ternary' }];
 

From 6a672ed945eace5aa070c3563f1174acde9d5c07 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Tue, 17 Jul 2018 16:12:59 +0200
Subject: [PATCH 13/45] Fix typo scatter's select module [1852]

- Just rename a parameter name.
---
 src/traces/scatter/select.js | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/traces/scatter/select.js b/src/traces/scatter/select.js
index 0d00de0a2f5..8a99678f47e 100644
--- a/src/traces/scatter/select.js
+++ b/src/traces/scatter/select.js
@@ -78,15 +78,15 @@ exports.getPointsIn = function(searchInfo, polygon) {
  *
  * @param searchInfo - info about trace to modify
  * @param {boolean} selected - are these points to be selected (true) or deselected (false)
- * @param {integer[]} pointsIds - the points to modify - omit to modify all points
+ * @param {integer[]} pointIds - the points to modify - omit to modify all points
  *        in the trace. i.e. clearSelection is toggleSelection(searchInfo, false).
  */
-exports.toggleSelected = function(searchInfo, selected, pointsIds) {
-    if(!Array.isArray(pointsIds)) {
-        pointsIds = [];
+exports.toggleSelected = function(searchInfo, selected, pointIds) {
+    if(!Array.isArray(pointIds)) {
+        pointIds = [];
         for(var i = 0; i < searchInfo.cd.length; i++) {
-            pointsIds.push(i);
+            pointIds.push(i);
         }
     }
-    return _togglePointSelectedState(searchInfo, pointsIds, selected);
+    return _togglePointSelectedState(searchInfo, pointIds, selected);
 };

From 2bc81a2afb500620ca5a900686d83fae9d1225f3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Tue, 17 Jul 2018 16:13:34 +0200
Subject: [PATCH 14/45] Transition bar trace to new selection mechanism [1852]

---
 src/lib/set_operations.js     | 12 +++++-
 src/plots/cartesian/select.js |  8 ++--
 src/traces/bar/index.js       |  3 +-
 src/traces/bar/select.js      | 79 ++++++++++++++++++++++++-----------
 4 files changed, 73 insertions(+), 29 deletions(-)

diff --git a/src/lib/set_operations.js b/src/lib/set_operations.js
index 6119c3d402c..26473084d30 100644
--- a/src/lib/set_operations.js
+++ b/src/lib/set_operations.js
@@ -12,9 +12,19 @@
 /*
  * Computes the set difference of two arrays.
  *
- * Returns all elements of a that are not in b.
+ * @param {array} a
+ * @param {array} b
+ * @returns all elements of a that are not in b.
+ *      If a is not an array, an empty array is returned.
+ *      If b is not an array, a is returned.
  */
 function difference(a, b) {
+    if(!Array.isArray(a)) {
+        return [];
+    }
+    if(!Array.isArray(b)) {
+        return a;
+    }
     return a.filter(function(e) {
         return b.indexOf(e) < 0;
     });
diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 4c3a8a8a4b1..3e22d6f69ff 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -282,7 +282,8 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
                 var selection = [],
                     retainSelection = shouldRetainSelection(e),
                     module,
-                    searchInfo;
+                    searchInfo,
+                    i;
 
                 // TODO What's the point of maintaining traceSelections array? Not used anywhere. Delete it.
                 var thisSelection, traceSelections = [], traceSelection;
@@ -296,10 +297,10 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
                     var pointsInCurrentPolygon = module.getPointsIn(searchInfo, currentPolygonTester);
                     module.toggleSelected(searchInfo, !subtract, pointsInCurrentPolygon);
 
-                    var pointsNoLongerSelected = difference(pointsInPolygon, pointsInCurrentPolygon);
+                    var pointsNoLongerSelected = difference(pointsInPolygon[i], pointsInCurrentPolygon);
 
                     traceSelection = module.toggleSelected(searchInfo, false, pointsNoLongerSelected);
-                    pointsInPolygon = pointsInCurrentPolygon;
+                    pointsInPolygon[i] = pointsInCurrentPolygon;
 
                     traceSelections.push(traceSelection);
 
@@ -376,6 +377,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 // ----------------
 // TODO handle clearing selection when no point is clicked (based on hoverData)
 // TODO do we have to consider multiple traces?
+// TODO Only execute selectOnClick functionality if the trace of hoverData supports it
 function selectOnClick(gd, numClicks, evt, outlines) {
     var calcData = gd.calcdata[0];
 
diff --git a/src/traces/bar/index.js b/src/traces/bar/index.js
index b05ddd8b79d..458568c02e7 100644
--- a/src/traces/bar/index.js
+++ b/src/traces/bar/index.js
@@ -23,7 +23,8 @@ Bar.plot = require('./plot');
 Bar.style = require('./style').style;
 Bar.styleOnSelect = require('./style').styleOnSelect;
 Bar.hoverPoints = require('./hover');
-Bar.selectPoints = require('./select');
+Bar.getPointsIn = require('./select').getPointsIn;
+Bar.toggleSelected = require('./select').toggleSelected;
 
 Bar.moduleType = 'trace';
 Bar.name = 'bar';
diff --git a/src/traces/bar/select.js b/src/traces/bar/select.js
index 04ede09356c..13bcd525ab5 100644
--- a/src/traces/bar/select.js
+++ b/src/traces/bar/select.js
@@ -8,34 +8,65 @@
 
 'use strict';
 
-module.exports = function selectPoints(searchInfo, polygon) {
-    var cd = searchInfo.cd;
-    var xa = searchInfo.xaxis;
-    var ya = searchInfo.yaxis;
+// TODO DRY
+function _togglePointSelectedState(searchInfo, pointIds, selected) {
     var selection = [];
-    var i;
 
-    if(polygon === false) {
-        // clear selection
-        for(i = 0; i < cd.length; i++) {
-            cd[i].selected = 0;
-        }
-    } else {
-        for(i = 0; i < cd.length; i++) {
-            var di = cd[i];
-
-            if(polygon.contains(di.ct)) {
-                selection.push({
-                    pointNumber: i,
-                    x: xa.c2d(di.x),
-                    y: ya.c2d(di.y)
-                });
-                di.selected = 1;
-            } else {
-                di.selected = 0;
-            }
+    var calcData = searchInfo.cd,
+        xAxis = searchInfo.xaxis,
+        yAxis = searchInfo.yaxis;
+
+    // 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++) {
+        if(calcData[i].selected === 1) {
+            selection.push(_newSelectionItem(
+              i,
+              xAxis.c2d(calcData[i].x),
+              yAxis.c2d(calcData[i].y)));
         }
     }
 
     return selection;
+}
+
+// TODO DRY
+function _newSelectionItem(pointNumber, xInData, yInData) {
+    return {
+        pointNumber: pointNumber,
+        x: xInData,
+        y: yInData
+    };
+}
+
+exports.getPointsIn = function(searchInfo, polygon) {
+    var pointsIn = [];
+
+    var calcData = searchInfo.cd,
+        i;
+
+    for(i = 0; i < calcData.length; i++) {
+        if(polygon.contains(calcData[i].ct)) {
+            pointsIn.push(i);
+        }
+    }
+
+    return pointsIn;
+};
+
+exports.toggleSelected = function(searchInfo, selected, pointIds) {
+    if(!Array.isArray(pointIds)) {
+        // TODO Use arrayRange maybe
+        pointIds = [];
+        for(var i = 0; i < searchInfo.cd.length; i++) {
+            pointIds.push(i);
+        }
+    }
+    return _togglePointSelectedState(searchInfo, pointIds, selected);
 };

From f51afcf063fb575ecba83a47d48ecbeabfb86c1a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Fri, 29 Jun 2018 07:45:43 +0200
Subject: [PATCH 15/45] Support click-to-select in a multi-traces setting
 [1852]

---
 src/plots/cartesian/select.js | 147 ++++++++++++++++++++++++----------
 src/traces/scatter/select.js  |   4 +-
 2 files changed, 108 insertions(+), 43 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 3e22d6f69ff..553cfa141a3 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -14,6 +14,7 @@ var polybool = require('polybooljs');
 var Registry = require('../../registry');
 var Color = require('../../components/color');
 var Fx = require('../../components/fx');
+var Axes = require('./axes');
 
 var difference = require('../../lib/set_operations').difference;
 var polygon = require('../../lib/polygon');
@@ -285,8 +286,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
                     searchInfo,
                     i;
 
-                // TODO What's the point of maintaining traceSelections array? Not used anywhere. Delete it.
-                var thisSelection, traceSelections = [], traceSelection;
+                var thisSelection, traceSelection;
                 for(i = 0; i < searchTraces.length; i++) {
                     searchInfo = searchTraces[i];
                     module = searchInfo._module;
@@ -302,8 +302,6 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
                     traceSelection = module.toggleSelected(searchInfo, false, pointsNoLongerSelected);
                     pointsInPolygon[i] = pointsInCurrentPolygon;
 
-                    traceSelections.push(traceSelection);
-
                     thisSelection = fillSelectionItem(traceSelection, searchInfo);
 
                     if(selection.length) {
@@ -376,47 +374,103 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 // Missing features
 // ----------------
 // TODO handle clearing selection when no point is clicked (based on hoverData)
-// TODO do we have to consider multiple traces?
-// TODO Only execute selectOnClick functionality if the trace of hoverData supports it
+// TODO Only execute selectOnClick functionality if the trace of hoverData implements selection interface
+// TODO Why not use forEach to iterate arrays?
 function selectOnClick(gd, numClicks, evt, outlines) {
-    var calcData = gd.calcdata[0];
-
     var hoverData = gd._hoverdata;
+    var isHoverDataSet = hoverData && Array.isArray(hoverData);
+    var retainSelection = shouldRetainSelection(evt);
+    var searchTraces;
+    var searchInfo;
+    var trace;
+    var multiPtsSelected;
+    var clickedPts;
+    var clickedPt;
+    var shouldSelect;
+    var traceSelection;
+    var allSelectionItems;
+    var eventData;
+    var i;
+    var j;
+
+    if(isHoverDataSet && numClicks === 1) {
+        allSelectionItems = [];
+
+        searchTraces = determineSearchTraces(gd);
+        multiPtsSelected = areMultiplePointsSelected(searchTraces);
+
+        // TODO Use forEach
+        for(i = 0; i < searchTraces.length; i++) {
+            searchInfo = searchTraces[i];
+            trace = searchInfo.cd[0].trace;
+
+            // Start new selection if needed
+            if(!retainSelection) {
+                searchInfo._module.toggleSelected(searchInfo, false);
+                if(outlines) outlines.remove();
+            }
+
+            // Determine clicked points,
+            // call selection modification functions of the trace's module
+            // and collect the resulting set of selected points
+            clickedPts = clickedPtsFor(searchInfo, hoverData);
+            if(clickedPts.length > 0) {
+                // TODO Use forEach
+                for(j = 0; j < clickedPts.length; j++) {
+                    clickedPt = clickedPts[j];
+                    var ptSelected = isPointSelected(trace, clickedPt);
+                    shouldSelect = !ptSelected || (ptSelected && multiPtsSelected && !retainSelection);
+                    traceSelection = searchInfo._module.toggleSelected(searchInfo, shouldSelect, [clickedPt]);
+                }
+            } else {
+                // If current trace has no pts clicked, we at least call toggleSelected
+                // with an empty array to obtain currently selected points for this trace.
+                traceSelection = searchInfo._module.toggleSelected(searchInfo, true, []);
+            }
+
+            // Merge this trace's selection with the other ones
+            // to prepare the grand selection state update
+            allSelectionItems = allSelectionItems.concat(fillSelectionItem(traceSelection, searchInfo));
+        }
+
+        // Grand selection state update needs to be done once for the entire plot
+        eventData = {points: allSelectionItems};
+        updateSelectedState(gd, searchTraces, eventData);
+
+        // Remove outlines if no point is selected anymore
+        if(allSelectionItems.length === 0 && outlines) outlines.remove();
+    }
+
+    function clickedPtsFor(searchInfo, hoverData) {
+        var clickedPts = [];
 
-    var isHoverDataSet = hoverData && hoverData.length > 0;
-    var isSingleClick = numClicks === 1;
-    var selectPreconditionsMet = isHoverDataSet && isSingleClick;
-
-    if(selectPreconditionsMet) {
-        var trace = calcData[0].trace,
-            hoverDatum = hoverData[0],
-            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.toggleSelected(searchInfo, false);
-
-        var shouldDeselectPoint = (pointSelected && onePointSelectedOnly) ||
-          (pointSelected && !onePointSelectedOnly && retainSelection);
-        var newTraceSelection =
-          module.toggleSelected(searchInfo, !shouldDeselectPoint, [hoverDatum.pointNumber]);
-
-        // When not retaining or when the sole selected
-        // point gets deselected, remove outlines
-        if(outlines &&
-          (!retainSelection || (pointSelected && onePointSelectedOnly))) {
-            outlines.remove();
+        for(var i = 0; i < hoverData.length; i++) {
+            var hoverDatum = hoverData[i];
+            if(hoverDatum.fullData._expandedIndex === searchInfo.cd[0].trace._expandedIndex) {
+                clickedPts.push(hoverDatum.pointNumber);
+            }
         }
 
-        // Update selection state
-        var selection = fillSelectionItem(newTraceSelection, searchInfo);
-        var eventData = {points: selection};
+        return clickedPts;
+    }
 
-        updateSelectedState(gd, [searchInfo], eventData);
+    // TODO DRY
+    function determineSearchTraces(gd) {
+        var searchTraces = [];
+
+        for(var i = 0; i < gd.calcdata.length; i++) {
+            var calcDataItem = gd.calcdata[i];
+            var trace = calcDataItem[0].trace;
+            // TODO Check if trace is selectable
+            var module = trace._module;
+            var searchInfo = _createSearchInfo(module, calcDataItem,
+              Axes.getFromTrace(gd, trace, 'x'),
+              Axes.getFromTrace(gd, trace, 'y'));
+
+            searchTraces.push(searchInfo);
+        }
+
+        return searchTraces;
     }
 }
 
@@ -429,9 +483,18 @@ function isPointSelected(trace, pointNumber) {
     return trace.selectedpoints.indexOf(pointNumber) > -1;
 }
 
-function isOnePointSelectedOnly(trace) {
-    if(!trace.selectedpoints && !Array.isArray(trace.selectedpoints)) return false;
-    return trace.selectedpoints.length === 1;
+function areMultiplePointsSelected(searchTraces) {
+    var ptsSelected = 0;
+    for(var i = 0; i < searchTraces.length; i++) {
+        var trace = searchTraces[i].cd[0].trace;
+        if(Array.isArray(trace.selectedpoints)) {
+            ptsSelected += trace.selectedpoints.length;
+        }
+
+        if(ptsSelected > 1) return true;
+    }
+
+    return ptsSelected > 1;
 }
 
 // TODO Consider using in other places around here as well
diff --git a/src/traces/scatter/select.js b/src/traces/scatter/select.js
index 8a99678f47e..e0c82a3a624 100644
--- a/src/traces/scatter/select.js
+++ b/src/traces/scatter/select.js
@@ -76,10 +76,12 @@ exports.getPointsIn = function(searchInfo, polygon) {
  * Update the selected flag of the given points. Omitting which points
  * to modify will update all points of the passed trace.
  *
- * @param searchInfo - info about trace to modify
+ * @param {object} searchInfo - info about trace to modify
  * @param {boolean} selected - are these points to be selected (true) or deselected (false)
  * @param {integer[]} pointIds - the points to modify - omit to modify all points
  *        in the trace. i.e. clearSelection is toggleSelection(searchInfo, false).
+ *
+ * @return {object[]} an array of all points selected after modification
  */
 exports.toggleSelected = function(searchInfo, selected, pointIds) {
     if(!Array.isArray(pointIds)) {

From a1efe01ba956158aaa0d319d3307f30ac8e48c04 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Fri, 29 Jun 2018 11:26:06 +0200
Subject: [PATCH 16/45] Extract 'finding selectable traces' in cartesian/select
 [1852]

- Reason: DRY with selectOnClick, whereas extracted method has logic
  in it to determine if a trace is selectable.
- Prepare scattergl as next target of new selection interface migration.
---
 src/plots/cartesian/dragbox.js |   3 +-
 src/plots/cartesian/select.js  | 132 ++++++++++++++-------------------
 2 files changed, 59 insertions(+), 76 deletions(-)

diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js
index 5fdf3513806..b9e286c1396 100644
--- a/src/plots/cartesian/dragbox.js
+++ b/src/plots/cartesian/dragbox.js
@@ -218,7 +218,8 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
 
         if(isMainDrag) {
             var clickHandler = obtainClickHandler();
-            clickHandler(gd, numClicks, evt);
+            // TODO not sure a uniform interface of click handlers applies, probably better be explicit
+            clickHandler(gd, numClicks, evt, xaxes, yaxes);
         }
         // Allow manual editing of range bounds through an input field
         // TODO consider extracting that to a method for clarity
diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 553cfa141a3..72b67707eab 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -14,7 +14,6 @@ var polybool = require('polybooljs');
 var Registry = require('../../registry');
 var Color = require('../../components/color');
 var Fx = require('../../components/fx');
-var Axes = require('./axes');
 
 var difference = require('../../lib/set_operations').difference;
 var polygon = require('../../lib/polygon');
@@ -47,14 +46,12 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
     var path0 = 'M' + x0 + ',' + y0;
     var pw = dragOptions.xaxes[0]._length;
     var ph = dragOptions.yaxes[0]._length;
-    var xAxisIds = dragOptions.xaxes.map(getAxId);
-    var yAxisIds = dragOptions.yaxes.map(getAxId);
     var allAxes = dragOptions.xaxes.concat(dragOptions.yaxes);
     var subtract = e.altKey;
 
     var filterPoly, testPoly, mergedPolygons, currentPolygon;
     var pointsInPolygon = [];
-    var i, cd, trace, searchInfo, eventData;
+    var i, searchInfo, eventData;
 
     var selectingOnSameSubplot = (
         fullLayout._lastSelectedSubplot &&
@@ -109,52 +106,10 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
         .attr('d', 'M0,0Z');
 
 
-    // find the traces to search for selection points
-    var searchTraces = [];
     var throttleID = fullLayout._uid + constants.SELECTID;
 
-    for(i = 0; i < gd.calcdata.length; i++) {
-        cd = gd.calcdata[i];
-        trace = cd[0].trace;
-
-        if(trace.visible !== true || !trace._module ||
-          !trace._module.toggleSelected || !trace._module.getPointsIn) continue;
-
-        if(dragOptions.subplot) {
-            if(
-                trace.subplot === dragOptions.subplot ||
-                trace.geo === dragOptions.subplot
-            ) {
-                searchTraces.push({
-                    _module: trace._module,
-                    cd: cd,
-                    xaxis: dragOptions.xaxes[0],
-                    yaxis: dragOptions.yaxes[0]
-                });
-            }
-        } else if(
-            trace.type === 'splom' &&
-            // FIXME: make sure we don't have more than single axis for splom
-            trace._xaxes[xAxisIds[0]] && trace._yaxes[yAxisIds[0]]
-        ) {
-            searchTraces.push({
-                _module: trace._module,
-                cd: cd,
-                xaxis: dragOptions.xaxes[0],
-                yaxis: dragOptions.yaxes[0]
-            });
-        } else {
-            if(xAxisIds.indexOf(trace.xaxis) === -1) continue;
-            if(yAxisIds.indexOf(trace.yaxis) === -1) continue;
-
-            searchTraces.push({
-                _module: trace._module,
-                cd: cd,
-                xaxis: getFromId(gd, trace.xaxis),
-                yaxis: getFromId(gd, trace.yaxis)
-            });
-        }
-    }
+    // find the traces to search for selection points
+    var searchTraces = determineSearchTraces(gd, dragOptions.xaxes, dragOptions.yaxes);
 
     function axValue(ax) {
         var index = (ax._id.charAt(0) === 'y') ? 1 : 0;
@@ -340,7 +295,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
             }
             else {
                 // TODO What to do with the code below because we now have behavior for a single click
-                selectOnClick(gd, numClicks, evt, outlines);
+                selectOnClick(gd, numClicks, evt, dragOptions.xaxes, dragOptions.yaxes, outlines);
 
                 // TODO: remove in v2 - this was probably never intended to work as it does,
                 // but in case anyone depends on it we don't want to break it now.
@@ -376,7 +331,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 // TODO handle clearing selection when no point is clicked (based on hoverData)
 // TODO Only execute selectOnClick functionality if the trace of hoverData implements selection interface
 // TODO Why not use forEach to iterate arrays?
-function selectOnClick(gd, numClicks, evt, outlines) {
+function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
     var hoverData = gd._hoverdata;
     var isHoverDataSet = hoverData && Array.isArray(hoverData);
     var retainSelection = shouldRetainSelection(evt);
@@ -396,7 +351,7 @@ function selectOnClick(gd, numClicks, evt, outlines) {
     if(isHoverDataSet && numClicks === 1) {
         allSelectionItems = [];
 
-        searchTraces = determineSearchTraces(gd);
+        searchTraces = determineSearchTraces(gd, xAxes, yAxes);
         multiPtsSelected = areMultiplePointsSelected(searchTraces);
 
         // TODO Use forEach
@@ -453,25 +408,62 @@ function selectOnClick(gd, numClicks, evt, outlines) {
 
         return clickedPts;
     }
+}
 
-    // TODO DRY
-    function determineSearchTraces(gd) {
-        var searchTraces = [];
+function determineSearchTraces(gd, xAxes, yAxes) {
+    var searchTraces = [];
+    var xAxisIds = xAxes.map(getAxId);
+    var yAxisIds = yAxes.map(getAxId);
+    var cd;
+    var trace;
+    var i;
 
-        for(var i = 0; i < gd.calcdata.length; i++) {
-            var calcDataItem = gd.calcdata[i];
-            var trace = calcDataItem[0].trace;
-            // TODO Check if trace is selectable
-            var module = trace._module;
-            var searchInfo = _createSearchInfo(module, calcDataItem,
-              Axes.getFromTrace(gd, trace, 'x'),
-              Axes.getFromTrace(gd, trace, 'y'));
+    for(i = 0; i < gd.calcdata.length; i++) {
+        cd = gd.calcdata[i];
+        trace = cd[0].trace;
 
-            searchTraces.push(searchInfo);
-        }
+        if(trace.visible !== true || !trace._module ||
+          !trace._module.toggleSelected || !trace._module.getPointsIn) continue;
+
+        // TODO is dragOptions.subplot is ever set? If not, delete.
+        // if(dragOptions.subplot) {
+        //     if(
+        //       trace.subplot === dragOptions.subplot ||
+        //       trace.geo === dragOptions.subplot
+        //     ) {
+        //         searchTraces.push({
+        //             _module: trace._module,
+        //             cd: cd,
+        //             xaxis: xAxes[0],
+        //             yaxis: yAxes[0]
+        //         });
+        //     }
+        // } else if(
+        if(
+          trace.type === 'splom' &&
+          // FIXME: make sure we don't have more than single axis for splom
+          trace._xaxes[xAxisIds[0]] && trace._yaxes[yAxisIds[0]]
+        ) {
+            searchTraces.push(createSearchInfo(trace._module, cd, xAxes[0], yAxes[0]));
+        } else {
+            if(xAxisIds.indexOf(trace.xaxis) === -1) continue;
+            if(yAxisIds.indexOf(trace.yaxis) === -1) continue;
 
-        return searchTraces;
+            searchTraces.push(createSearchInfo(trace._module, cd,
+              getFromId(gd, trace.xaxis), getFromId(gd, trace.yaxis)));
+        }
     }
+
+    return searchTraces;
+}
+
+function createSearchInfo(module, calcData, xaxis, yaxis) {
+    return {
+        _module: module,
+        cd: calcData,
+        xaxis: xaxis,
+        yaxis: yaxis
+    };
 }
 
 function shouldRetainSelection(evt) {
@@ -497,16 +489,6 @@ function areMultiplePointsSelected(searchTraces) {
     return ptsSelected > 1;
 }
 
-// TODO Consider using in other places around here as well
-function _createSearchInfo(module, calcData, xaxis, yaxis) {
-    return {
-        _module: module,
-        cd: calcData,
-        xaxis: xaxis,
-        yaxis: yaxis
-    };
-}
-
 /**
  * Updates the selection state properties of the passed traces
  * and initiates proper selection styling.

From 722805e2b1f95a807d913fd8d97a5949bd0a09cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Tue, 3 Jul 2018 07:20:42 +0200
Subject: [PATCH 17/45] Transition scattergl to new selection interface [1852]

- Thereby had to change controlling code in
  cartesian/select#selectOnClick.
- No longer restrict selected mode rendering to
  lasso/box select mode.
---
 src/plots/cartesian/select.js |  14 +++-
 src/traces/scattergl/index.js | 147 ++++++++++++++++++++++++++++------
 2 files changed, 132 insertions(+), 29 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 72b67707eab..59bc07a9c36 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -329,8 +329,6 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 // Missing features
 // ----------------
 // TODO handle clearing selection when no point is clicked (based on hoverData)
-// TODO Only execute selectOnClick functionality if the trace of hoverData implements selection interface
-// TODO Why not use forEach to iterate arrays?
 function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
     var hoverData = gd._hoverdata;
     var isHoverDataSet = hoverData && Array.isArray(hoverData);
@@ -354,7 +352,6 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
         searchTraces = determineSearchTraces(gd, xAxes, yAxes);
         multiPtsSelected = areMultiplePointsSelected(searchTraces);
 
-        // TODO Use forEach
         for(i = 0; i < searchTraces.length; i++) {
             searchInfo = searchTraces[i];
             trace = searchInfo.cd[0].trace;
@@ -370,7 +367,6 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
             // and collect the resulting set of selected points
             clickedPts = clickedPtsFor(searchInfo, hoverData);
             if(clickedPts.length > 0) {
-                // TODO Use forEach
                 for(j = 0; j < clickedPts.length; j++) {
                     clickedPt = clickedPts[j];
                     var ptSelected = isPointSelected(trace, clickedPt);
@@ -388,6 +384,16 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
             allSelectionItems = allSelectionItems.concat(fillSelectionItem(traceSelection, searchInfo));
         }
 
+        // Hack to achieve regl traces to set selectBatch to null in case no point is selected anymore
+        // TODO check in advance if a click clear the entire selection, because in this
+        // case just call toggleSelected(searchInfo, false) on all traces and be done. The `shouldSelect` above might
+        // become obsolete.
+        if(allSelectionItems.length === 0) {
+            for(i = 0; i < searchTraces.length; i++) {
+                searchTraces[i]._module.toggleSelected(searchTraces[i], false);
+            }
+        }
+
         // Grand selection state update needs to be done once for the entire plot
         eventData = {points: allSelectionItems};
         updateSelectedState(gd, searchTraces, eventData);
diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js
index ab398fa97a9..26d1e529b14 100644
--- a/src/traces/scattergl/index.js
+++ b/src/traces/scattergl/index.js
@@ -516,6 +516,7 @@ function plot(gd, subplot, cdata) {
         }
     }
 
+    // TODO Remove selectMode variable eventually
     var selectMode = dragmode === 'lasso' || dragmode === 'select';
     scene.selectBatch = null;
     scene.unselectBatch = null;
@@ -548,6 +549,7 @@ function plot(gd, subplot, cdata) {
             (height - vpSize.t) - (1 - yaxis.domain[1]) * vpSize.h
         ];
 
+        // TODO click-to-select: maybe attach selectedpoints to scene here to then use it in scene.draw to detect non-selection mode
         if(trace.selectedpoints || selectMode) {
             if(!selectMode) selectMode = true;
 
@@ -590,32 +592,30 @@ function plot(gd, subplot, cdata) {
             null;
     });
 
-    if(selectMode) {
-        // create select2d
-        if(!scene.select2d) {
-            // create scatter instance by cloning scatter2d
-            scene.select2d = createScatter(fullLayout._glcanvas.data()[1].regl);
-        }
+    // create select2d
+    if(!scene.select2d) {
+        // create scatter instance by cloning scatter2d
+        scene.select2d = createScatter(fullLayout._glcanvas.data()[1].regl);
+    }
 
-        if(scene.scatter2d && scene.selectBatch && scene.selectBatch.length) {
-            // update only traces with selection
-            scene.scatter2d.update(scene.markerUnselectedOptions.map(function(opts, i) {
-                return scene.selectBatch[i] ? opts : null;
-            }));
-        }
+    if(scene.scatter2d && scene.selectBatch && scene.selectBatch.length) {
+        // update only traces with selection
+        scene.scatter2d.update(scene.markerUnselectedOptions.map(function(opts, i) {
+            return scene.selectBatch[i] ? opts : null;
+        }));
+    }
 
-        if(scene.select2d) {
-            scene.select2d.update(scene.markerOptions);
-            scene.select2d.update(scene.markerSelectedOptions);
-        }
+    if(scene.select2d) {
+        scene.select2d.update(scene.markerOptions);
+        scene.select2d.update(scene.markerSelectedOptions);
+    }
 
-        if(scene.glText) {
-            cdata.forEach(function(cdscatter) {
-                if(cdscatter && cdscatter[0] && cdscatter[0].trace) {
-                    styleTextSelection(cdscatter);
-                }
-            });
-        }
+    if(scene.glText) {
+        cdata.forEach(function(cdscatter) {
+            if(cdscatter && cdscatter[0] && cdscatter[0].trace) {
+                styleTextSelection(cdscatter);
+            }
+        });
     }
 
     // upload viewport/range data to GPU
@@ -823,7 +823,7 @@ function calcHover(pointData, x, y, trace) {
     return pointData;
 }
 
-
+// TODO Remove eventually
 function selectPoints(searchInfo, polygon) {
     var cd = searchInfo.cd;
     var selection = [];
@@ -894,6 +894,102 @@ function selectPoints(searchInfo, polygon) {
     return selection;
 }
 
+function getPointsIn(searchInfo, polygon) {
+    var pointsIn = [];
+
+    var calcData = searchInfo.cd;
+    var trace = calcData[0].trace;
+    var stash = calcData[0].t;
+    var scene = stash._scene;
+    var i;
+
+    if(!scene) return [];
+
+    var hasOnlyLines = (!subTypes.hasMarkers(trace) && !subTypes.hasText(trace));
+    if(trace.visible !== true || hasOnlyLines) return [];
+
+    if(polygon !== false && !polygon.degenerate) {
+        for(i = 0; i < stash.count; i++) {
+            if(polygon.contains([stash.xpx[i], stash.ypx[i]])) {
+                pointsIn.push(i);
+            }
+        }
+    }
+
+    return pointsIn;
+}
+
+function toggleSelected(searchInfo, selected, pointIds) {
+    var clearSelection = selected === false && !Array.isArray(pointIds);
+    var selection = [];
+    var calcData = searchInfo.cd;
+    var stash = calcData[0].t;
+    var scene = stash._scene;
+    var oldEls;
+    var oldUnels;
+    var newEls;
+    var newUnels;
+    var i;
+
+    if(!Array.isArray(pointIds)) {
+        pointIds = arrayRange(stash.count);
+    }
+
+    // Mutate state
+    coerceSelectBatches(scene, stash);
+    oldEls = scene.selectBatch[stash.index];
+    oldUnels = scene.unselectBatch[stash.index];
+
+    if(selected) {
+        newEls = oldEls.concat(Lib.difference(pointIds, oldEls));
+        newUnels = Lib.difference(oldUnels, pointIds);
+    } else {
+        newEls = Lib.difference(oldEls, pointIds);
+        newUnels = oldUnels.concat(Lib.difference(pointIds, oldUnels));
+    }
+
+    scene.selectBatch[stash.index] = clearSelection ? null : newEls;
+    scene.unselectBatch[stash.index] = clearSelection ? arrayRange(stash.count) : newUnels;
+
+    // Compute and return selection array from internal state
+    for(i = 0; i < newEls.length; i++) {
+        selection.push({
+            pointNumber: newEls[i],
+            x: stash.x[newEls[i]],
+            y: stash.y[newEls[i]]
+        });
+    }
+
+    // TODO remove eventually
+    // console.log('els  : ' + JSON.stringify(scene.selectBatch[stash.index]));
+    // console.log('unels: ' + JSON.stringify(scene.unselectBatch[stash.index]));
+    // console.log('selection: ' + JSON.stringify(selection));
+    return selection;
+}
+
+function coerceSelectBatches(scene, stash) {
+    var i;
+
+    if(!scene.selectBatch) {
+        scene.selectBatch = [];
+        scene.unselectBatch = [];
+    }
+
+    if(!scene.selectBatch[stash.index]) {
+        // enter every trace select mode
+        for(i = 0; i < scene.count; i++) {
+            scene.selectBatch[i] = [];
+            scene.unselectBatch[i] = [];
+        }
+
+        scene.unselectBatch[stash.index] = arrayRange(stash.count);
+
+        // TODO what does that do?
+        // we should turn scatter2d into unselected once we have any points selected
+        scene.scatter2d.update(scene.markerUnselectedOptions);
+    }
+}
+
 function style(gd, cds) {
     if(!cds) return;
 
@@ -959,7 +1055,8 @@ module.exports = {
     plot: plot,
     hoverPoints: hoverPoints,
     style: style,
-    selectPoints: selectPoints,
+    getPointsIn: getPointsIn,
+    toggleSelected: toggleSelected,
 
     sceneOptions: sceneOptions,
     sceneUpdate: sceneUpdate,

From 4f0a05d93ebc8acf03527ef6193f3e7cd8fc1037 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Tue, 3 Jul 2018 11:34:52 +0200
Subject: [PATCH 18/45] Adapt cartesian's selectOnClick to support histogram
 [1852]

- Account for difference in hoverData having no pointNumber but
  binNumber and pointNumbers in case of histogram.
- Created a messy entireSelectionToBeCleared function
---
 src/plots/cartesian/select.js | 63 +++++++++++++++++++++++++----------
 src/traces/histogram/index.js |  3 +-
 2 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 59bc07a9c36..d755518b594 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -336,7 +336,7 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
     var searchTraces;
     var searchInfo;
     var trace;
-    var multiPtsSelected;
+    var clearEntireSelection;
     var clickedPts;
     var clickedPt;
     var shouldSelect;
@@ -350,7 +350,7 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
         allSelectionItems = [];
 
         searchTraces = determineSearchTraces(gd, xAxes, yAxes);
-        multiPtsSelected = areMultiplePointsSelected(searchTraces);
+        clearEntireSelection = entireSelectionToBeCleared(searchTraces, hoverData);
 
         for(i = 0; i < searchTraces.length; i++) {
             searchInfo = searchTraces[i];
@@ -370,8 +370,8 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
                 for(j = 0; j < clickedPts.length; j++) {
                     clickedPt = clickedPts[j];
                     var ptSelected = isPointSelected(trace, clickedPt);
-                    shouldSelect = !ptSelected || (ptSelected && multiPtsSelected && !retainSelection);
-                    traceSelection = searchInfo._module.toggleSelected(searchInfo, shouldSelect, [clickedPt]);
+                    shouldSelect = !ptSelected || (ptSelected && !clearEntireSelection && !retainSelection);
+                    traceSelection = searchInfo._module.toggleSelected(searchInfo, shouldSelect, [clickedPt.pointNumber]);
                 }
             } else {
                 // If current trace has no pts clicked, we at least call toggleSelected
@@ -384,6 +384,7 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
             allSelectionItems = allSelectionItems.concat(fillSelectionItem(traceSelection, searchInfo));
         }
 
+        // TODO Use the clearEntireSelection flag now
         // Hack to achieve regl traces to set selectBatch to null in case no point is selected anymore
         // TODO check in advance if a click clear the entire selection, because in this
         // case just call toggleSelected(searchInfo, false) on all traces and be done. The `shouldSelect` above might
@@ -408,7 +409,16 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
         for(var i = 0; i < hoverData.length; i++) {
             var hoverDatum = hoverData[i];
             if(hoverDatum.fullData._expandedIndex === searchInfo.cd[0].trace._expandedIndex) {
-                clickedPts.push(hoverDatum.pointNumber);
+                if(hoverDatum.pointNumber !== undefined) {
+                    clickedPts.push({
+                        pointNumber: hoverDatum.pointNumber
+                    });
+                } else if(hoverDatum.binNumber !== undefined) {
+                    clickedPts.push({
+                        pointNumber: hoverDatum.binNumber,
+                        pointNumbers: hoverDatum.pointNumbers
+                    });
+                }
             }
         }
 
@@ -476,23 +486,40 @@ function shouldRetainSelection(evt) {
     return evt.shiftKey;
 }
 
-function isPointSelected(trace, pointNumber) {
-    if(!trace.selectedpoints && !Array.isArray(trace.selectedpoints)) return false;
-    return trace.selectedpoints.indexOf(pointNumber) > -1;
-}
-
-function areMultiplePointsSelected(searchTraces) {
-    var ptsSelected = 0;
+// TODO Clean up
+function entireSelectionToBeCleared(searchTraces, hoverData) {
+    var somePointsSelected = false;
     for(var i = 0; i < searchTraces.length; i++) {
-        var trace = searchTraces[i].cd[0].trace;
-        if(Array.isArray(trace.selectedpoints)) {
-            ptsSelected += trace.selectedpoints.length;
-        }
+        var searchInfo = searchTraces[i];
+        var trace = searchInfo.cd[0].trace;
+        if(trace.selectedpoints && trace.selectedpoints.length > 0) {
+            somePointsSelected = true;
+            var selectedPtsCopy = trace.selectedpoints.slice();
+
+            for(var j = 0; j < hoverData.length; j++) {
+                var hoverDatum = hoverData[j];
+                if(hoverDatum.fullData._expandedIndex === trace._expandedIndex) {
+                    selectedPtsCopy = difference(selectedPtsCopy, hoverDatum.pointNumbers || [hoverDatum.pointNumber]);
+                }
+            }
 
-        if(ptsSelected > 1) return true;
+            if(selectedPtsCopy.length > 0) {
+                return false;
+            }
+        }
     }
+    return somePointsSelected ? true : false;
+}
 
-    return ptsSelected > 1;
+function isPointSelected(trace, point) {
+    if(!trace.selectedpoints && !Array.isArray(trace.selectedpoints)) return false;
+    if(point.pointNumbers) {
+        for(var i = 0; i < point.pointNumbers.length; i++) {
+            if(trace.selectedpoints.indexOf(point.pointNumbers[i]) < 0) return false;
+        }
+        return true;
+    }
+    return trace.selectedpoints.indexOf(point.pointNumber) > -1;
 }
 
 /**
diff --git a/src/traces/histogram/index.js b/src/traces/histogram/index.js
index c20e31d6e55..6098eef280c 100644
--- a/src/traces/histogram/index.js
+++ b/src/traces/histogram/index.js
@@ -37,7 +37,8 @@ Histogram.style = require('../bar/style').style;
 Histogram.styleOnSelect = require('../bar/style').styleOnSelect;
 Histogram.colorbar = require('../scatter/marker_colorbar');
 Histogram.hoverPoints = require('./hover');
-Histogram.selectPoints = require('../bar/select');
+Histogram.getPointsIn = require('../bar/select').getPointsIn;
+Histogram.toggleSelected = require('../bar/select').toggleSelected;
 Histogram.eventData = require('./event_data');
 
 Histogram.moduleType = 'trace';

From 512598167c16fd433a64831e849e50fcbdd1faed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Tue, 3 Jul 2018 13:30:35 +0200
Subject: [PATCH 19/45] Use clearEntireSelection flag in cartesian/select
 [1852]

- Reason: more efficient than the extra hack for scattergl for the case
  when no point is selected at all.
---
 src/plots/cartesian/select.js | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index d755518b594..5eee32d838c 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -356,10 +356,12 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
             searchInfo = searchTraces[i];
             trace = searchInfo.cd[0].trace;
 
-            // Start new selection if needed
-            if(!retainSelection) {
+            // Clear old selection if needed
+            if(!retainSelection || clearEntireSelection) {
                 searchInfo._module.toggleSelected(searchInfo, false);
                 if(outlines) outlines.remove();
+
+                if(clearEntireSelection) continue;
             }
 
             // Determine clicked points,
@@ -384,23 +386,9 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
             allSelectionItems = allSelectionItems.concat(fillSelectionItem(traceSelection, searchInfo));
         }
 
-        // TODO Use the clearEntireSelection flag now
-        // Hack to achieve regl traces to set selectBatch to null in case no point is selected anymore
-        // TODO check in advance if a click clear the entire selection, because in this
-        // case just call toggleSelected(searchInfo, false) on all traces and be done. The `shouldSelect` above might
-        // become obsolete.
-        if(allSelectionItems.length === 0) {
-            for(i = 0; i < searchTraces.length; i++) {
-                searchTraces[i]._module.toggleSelected(searchTraces[i], false);
-            }
-        }
-
         // Grand selection state update needs to be done once for the entire plot
         eventData = {points: allSelectionItems};
         updateSelectedState(gd, searchTraces, eventData);
-
-        // Remove outlines if no point is selected anymore
-        if(allSelectionItems.length === 0 && outlines) outlines.remove();
     }
 
     function clickedPtsFor(searchInfo, hoverData) {

From a79dea75428505157cbe04a203e127649117631b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Tue, 3 Jul 2018 14:47:22 +0200
Subject: [PATCH 20/45] Fix bug when clearing entire selection [1852]

- When drawing lasso bar and scatter trace weren't showing
  their points as deselected.
---
 src/plots/cartesian/select.js | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 5eee32d838c..50dced89f69 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -387,8 +387,12 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
         }
 
         // Grand selection state update needs to be done once for the entire plot
-        eventData = {points: allSelectionItems};
-        updateSelectedState(gd, searchTraces, eventData);
+        if(clearEntireSelection) {
+            updateSelectedState(gd, searchTraces);
+        } else {
+            eventData = {points: allSelectionItems};
+            updateSelectedState(gd, searchTraces, eventData);
+        }
     }
 
     function clickedPtsFor(searchInfo, hoverData) {
@@ -524,11 +528,7 @@ function isPointSelected(trace, point) {
 function updateSelectedState(gd, searchTraces, eventData) {
     var i, j, searchInfo, trace;
 
-    // TODO previously eventData without a point would still set a selection
-    // and all points would appear as non-selected. Moving to another drag mode like
-    // zoom would leave this state. Discuss if the new behavior is better.
-    var selectionNonEmpty = eventData && eventData.points && eventData.points.length > 0;
-    if(selectionNonEmpty) {
+    if(eventData) {
         // var pts = eventData.points || []; TODO remove eventually
         var pts = eventData.points;
 

From 4d8b274937d9e046822e99b4c572ee2c4f34460b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 4 Jul 2018 09:59:41 +0200
Subject: [PATCH 21/45] Implement new selection interface for box trace type
 [1852]

- Needs cleanup in select.js
- Controlling code in cartesian/select.js needs a change due to a false assumption about hoverData: box produces a multi-element hoverData array whereas only the first element is really the clicked point
---
 src/traces/box/index.js  |  3 +-
 src/traces/box/select.js | 84 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/src/traces/box/index.js b/src/traces/box/index.js
index ef67721f8eb..67ce5560a7a 100644
--- a/src/traces/box/index.js
+++ b/src/traces/box/index.js
@@ -20,7 +20,8 @@ Box.plot = require('./plot').plot;
 Box.style = require('./style').style;
 Box.styleOnSelect = require('./style').styleOnSelect;
 Box.hoverPoints = require('./hover').hoverPoints;
-Box.selectPoints = require('./select');
+Box.getPointsIn = require('./select').getPointsIn;
+Box.toggleSelected = require('./select').toggleSelected;
 
 Box.moduleType = 'trace';
 Box.name = 'box';
diff --git a/src/traces/box/select.js b/src/traces/box/select.js
index 9ec9ed03e3f..4e5ff61e133 100644
--- a/src/traces/box/select.js
+++ b/src/traces/box/select.js
@@ -8,7 +8,8 @@
 
 'use strict';
 
-module.exports = function selectPoints(searchInfo, polygon) {
+// TODO Remove eventually
+function selectPoints(searchInfo, polygon) {
     var cd = searchInfo.cd;
     var xa = searchInfo.xaxis;
     var ya = searchInfo.yaxis;
@@ -43,5 +44,86 @@ module.exports = function selectPoints(searchInfo, polygon) {
         }
     }
 
+    return selection;
+}
+
+exports.getPointsIn = function(searchInfo, polygon) {
+    var pointsIn = [];
+    var cd = searchInfo.cd;
+    var xa = searchInfo.xaxis;
+    var ya = searchInfo.yaxis;
+    var i;
+    var j;
+    var pt;
+    var x;
+    var y;
+
+    for(i = 0; i < cd.length; i++) {
+        for(j = 0; j < (cd[i].pts || []).length; j++) {
+            pt = cd[i].pts[j];
+            x = xa.c2p(pt.x);
+            y = ya.c2p(pt.y);
+
+            if(polygon.contains([x, y])) {
+                pointsIn.push(pt.i);
+            }
+        }
+    }
+
+    return pointsIn;
+};
+
+exports.toggleSelected = function(searchInfo, selected, pointIds) {
+    var selection = [];
+    var modifyAll = !Array.isArray(pointIds);
+    var cd = searchInfo.cd;
+    var xa = searchInfo.xaxis;
+    var ya = searchInfo.yaxis;
+    var pointId;
+    var pt;
+    var ptPos;
+    var i;
+    var j;
+
+    if(!modifyAll) {
+        // console.log(pointIds);
+    }
+
+    // Mutate state
+    // if(!modifyAll) {
+    //     for(i = 0; i < pointIds.length; i++) {
+    //         pointId = pointIds[i];
+    //         for(j = 0; j < cd.length; j++) {
+    //             ptPos = cd[j].pts.indexOf(pointId);
+    //             if(ptPos > -1) {
+    //                 pt = cd[j].pts[ptPos];
+    //                 pt.selected = selected ? 1 : 0;
+    //             }
+    //         }
+    //     }
+    // }
+
+
+    for(i = 0; i < cd.length; i++) {
+        for(j = 0; j < (cd[i].pts || []).length; j++) {
+            pt = cd[i].pts[j];
+
+            if(modifyAll) pt.selected = selected ? 1 : 0;
+            else {
+                if(pointIds.indexOf(pt.i) > -1) {
+                    pt.selected = selected ? 1 : 0;
+                }
+            }
+
+            if(pt.selected) {
+                selection.push({
+                    pointNumber: pt.i,
+                    x: xa.c2d(pt.x),
+                    y: ya.c2d(pt.y)
+                });
+            }
+        }
+    }
+
     return selection;
 };

From 231fa6d1933597bebaa271b5434f9f0cf56df8ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 4 Jul 2018 10:21:16 +0200
Subject: [PATCH 22/45] Consider first eventData element only as clicked point
 [1852]

- Reason: box trace type stuffing not even points but also hoverData
  about boxes which appears on mouseover into gd._hoverdata.
- So based on analysis of fx/hover.js, which sorts the hoverdata array
  so that the closest point comes first, it is assumed now that only
  the first element in hoverData represents a clicked point.
- For box trace type particulary if a box is clicked the first
  hoverdata element is not a point but a box with pointnumber being
  the index of the point group that's represented as a box plot
  element visually.
---
 src/plots/cartesian/select.js | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 50dced89f69..ac2caf999c2 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -397,10 +397,15 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
 
     function clickedPtsFor(searchInfo, hoverData) {
         var clickedPts = [];
+        var hoverDatum;
 
-        for(var i = 0; i < hoverData.length; i++) {
-            var hoverDatum = hoverData[i];
+        if(hoverData.length > 0) {
+            hoverDatum = hoverData[0];
             if(hoverDatum.fullData._expandedIndex === searchInfo.cd[0].trace._expandedIndex) {
+                // TODO hoverDatum not having a pointNumber but a binNumber seems to be an oddity of histogram only
+                // Not deleting .pointNumber in histogram/event_data.js would simplify code here and in addition
+                // would not break the hover event structure officially
+                // documented at https://plot.ly/javascript/hover-events/
                 if(hoverDatum.pointNumber !== undefined) {
                     clickedPts.push({
                         pointNumber: hoverDatum.pointNumber

From 6bd46bf4a129a066f283488547de17c5385bb652 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 4 Jul 2018 12:07:06 +0200
Subject: [PATCH 23/45] Clean up code in box/select.js [1852]

---
 src/traces/box/select.js | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/src/traces/box/select.js b/src/traces/box/select.js
index 4e5ff61e133..720b669d943 100644
--- a/src/traces/box/select.js
+++ b/src/traces/box/select.js
@@ -79,40 +79,17 @@ exports.toggleSelected = function(searchInfo, selected, pointIds) {
     var cd = searchInfo.cd;
     var xa = searchInfo.xaxis;
     var ya = searchInfo.yaxis;
-    var pointId;
     var pt;
-    var ptPos;
     var i;
     var j;
 
-    if(!modifyAll) {
-        // console.log(pointIds);
-    }
-
-    // Mutate state
-    // if(!modifyAll) {
-    //     for(i = 0; i < pointIds.length; i++) {
-    //         pointId = pointIds[i];
-    //         for(j = 0; j < cd.length; j++) {
-    //             ptPos = cd[j].pts.indexOf(pointId);
-    //             if(ptPos > -1) {
-    //                 pt = cd[j].pts[ptPos];
-    //                 pt.selected = selected ? 1 : 0;
-    //             }
-    //         }
-    //     }
-    // }
-
-
     for(i = 0; i < cd.length; i++) {
         for(j = 0; j < (cd[i].pts || []).length; j++) {
             pt = cd[i].pts[j];
 
             if(modifyAll) pt.selected = selected ? 1 : 0;
-            else {
-                if(pointIds.indexOf(pt.i) > -1) {
-                    pt.selected = selected ? 1 : 0;
-                }
+            else if(pointIds.indexOf(pt.i) > -1) {
+                pt.selected = selected ? 1 : 0;
             }
 
             if(pt.selected) {

From 427b4b6a2e9fdcb61007db22cca830005f24bc24 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 5 Jul 2018 15:57:19 +0200
Subject: [PATCH 24/45] Not select a clicked box in box trace type [1852]

- By introducing a hoverOnBox property on hoverData.
- Still needs cleaning things up.
- Left over misbehaviour: clicking a box deselects
  all points and instead should not change the
  current selection at all.
---
 src/plots/cartesian/select.js | 25 ++++++++++++++++++++++++-
 src/traces/box/event_data.js  | 26 ++++++++++++++++++++++++++
 src/traces/box/hover.js       |  4 ++++
 src/traces/box/index.js       |  1 +
 4 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 src/traces/box/event_data.js

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index ac2caf999c2..654b1703316 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -395,6 +395,26 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
         }
     }
 
+    /**
+     * Function to determine the clicked points for the given searchInfo (trace)
+     * based on the passed hoverData.
+     *
+     * Function assumes the following about hoverData:
+     * - when hoverData has more than one element (e.g. box trace),
+     *   if a point is hovered upon, the clicked point is the first
+     *   element in the array. It is assumed that fx/hover.js and satellite
+     *   modules are doing that correctly.
+     * - at the moment only one point at a time is considered to be selected
+     *   upon one click.
+     *
+     * Function also encapsulates special knowledge about the slight
+     * inconsistencies in what hoverData can look like for different
+     * trace types. As hoverData will become more homogeneous, this
+     * logic will become cleaner.
+     *
+     * See https://github.com/plotly/plotly.js/issues/1852 for the
+     * respective discussion.
+     */
     function clickedPtsFor(searchInfo, hoverData) {
         var clickedPts = [];
         var hoverDatum;
@@ -402,9 +422,12 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
         if(hoverData.length > 0) {
             hoverDatum = hoverData[0];
             if(hoverDatum.fullData._expandedIndex === searchInfo.cd[0].trace._expandedIndex) {
+                // Special case for box (and violin)
+                if(hoverDatum.hoverOnBox === true) return clickedPts;
+
                 // TODO hoverDatum not having a pointNumber but a binNumber seems to be an oddity of histogram only
                 // Not deleting .pointNumber in histogram/event_data.js would simplify code here and in addition
-                // would not break the hover event structure officially
+                // would not break the hover event structure
                 // documented at https://plot.ly/javascript/hover-events/
                 if(hoverDatum.pointNumber !== undefined) {
                     clickedPts.push({
diff --git a/src/traces/box/event_data.js b/src/traces/box/event_data.js
new file mode 100644
index 00000000000..bdc1cc88817
--- /dev/null
+++ b/src/traces/box/event_data.js
@@ -0,0 +1,26 @@
+/**
+* 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';
+
+module.exports = function eventData(out, pt) {
+    if(pt.hoverOnBox) out.hoverOnBox = pt.hoverOnBox;
+
+    // TODO Clean up
+    if('xVal' in pt) out.x = pt.xVal;
+    else if('x' in pt) out.x = pt.x;
+
+    if('yVal' in pt) out.y = pt.yVal;
+    else if('y' in pt) out.y = pt.y;
+
+    if(pt.xa) out.xaxis = pt.xa;
+    if(pt.ya) out.yaxis = pt.ya;
+    if(pt.zLabelVal !== undefined) out.z = pt.zLabelVal;
+
+    return out;
+};
diff --git a/src/traces/box/hover.js b/src/traces/box/hover.js
index 79e24509360..b4729279e1c 100644
--- a/src/traces/box/hover.js
+++ b/src/traces/box/hover.js
@@ -169,6 +169,10 @@ function hoverOnBoxes(pointData, xval, yval, hovermode) {
         pointData2[vLetter + 'LabelVal'] = val;
         pointData2[vLetter + 'Label'] = (t.labels ? t.labels[attr] + ' ' : '') + Axes.hoverLabelText(vAxis, val);
 
+        // Note: introduced to be able to distinguish a
+        // clicked point from a box during click-to-select
+        pointData2.hoverOnBox = true;
+
         if(attr === 'mean' && ('sd' in di) && trace.boxmean === 'sd') {
             pointData2[vLetter + 'err'] = di.sd;
         }
diff --git a/src/traces/box/index.js b/src/traces/box/index.js
index 67ce5560a7a..4c588b9843e 100644
--- a/src/traces/box/index.js
+++ b/src/traces/box/index.js
@@ -20,6 +20,7 @@ Box.plot = require('./plot').plot;
 Box.style = require('./style').style;
 Box.styleOnSelect = require('./style').styleOnSelect;
 Box.hoverPoints = require('./hover').hoverPoints;
+Box.eventData = require('./event_data');
 Box.getPointsIn = require('./select').getPointsIn;
 Box.toggleSelected = require('./select').toggleSelected;
 

From 3d2185636156901d79b3f6fcbc2e1dca0b6de8c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 5 Jul 2018 16:13:36 +0200
Subject: [PATCH 25/45] Not change selection state when a box is clicked in box
 trace [1852]

---
 src/plots/cartesian/select.js | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 654b1703316..602071f98e0 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -329,9 +329,9 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 // Missing features
 // ----------------
 // TODO handle clearing selection when no point is clicked (based on hoverData)
+// TODO remove console.log statements
 function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
     var hoverData = gd._hoverdata;
-    var isHoverDataSet = hoverData && Array.isArray(hoverData);
     var retainSelection = shouldRetainSelection(evt);
     var searchTraces;
     var searchInfo;
@@ -346,7 +346,7 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
     var i;
     var j;
 
-    if(isHoverDataSet && numClicks === 1) {
+    if(numClicks === 1 && isHoverDataSet(hoverData)) {
         allSelectionItems = [];
 
         searchTraces = determineSearchTraces(gd, xAxes, yAxes);
@@ -387,6 +387,7 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
         }
 
         // Grand selection state update needs to be done once for the entire plot
+        // console.log('allSelItems '  + allSelectionItems.map(asi => asi.pointNumber));
         if(clearEntireSelection) {
             updateSelectedState(gd, searchTraces);
         } else {
@@ -395,6 +396,12 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
         }
     }
 
+    function isHoverDataSet(hoverData) {
+        return hoverData &&
+          Array.isArray(hoverData) &&
+          hoverData[0].hoverOnBox !== true;
+    }
+
     /**
      * Function to determine the clicked points for the given searchInfo (trace)
      * based on the passed hoverData.

From b0c32c1ab92643e3da03fadccfeac466182f1516 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 18 Jul 2018 11:04:39 +0200
Subject: [PATCH 26/45] Remove unused testPoly variable in cartesian select
 module [1852]

- Reason: multiPolygon tester no longer needed due to changes
  how selection code works.
---
 src/plots/cartesian/select.js | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 602071f98e0..16cb5717370 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -27,7 +27,6 @@ var MINSELECT = constants.MINSELECT;
 
 var filteredPolygon = polygon.filter;
 var polygonTester = polygon.tester;
-var multipolygonTester = polygon.multitester;
 
 function getAxId(ax) { return ax._id; }
 
@@ -49,7 +48,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
     var allAxes = dragOptions.xaxes.concat(dragOptions.yaxes);
     var subtract = e.altKey;
 
-    var filterPoly, testPoly, mergedPolygons, currentPolygon;
+    var filterPoly, mergedPolygons, currentPolygon;
     var pointsInPolygon = [];
     var i, searchInfo, eventData;
 
@@ -214,12 +213,9 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
         if(dragOptions.polygons && dragOptions.polygons.length) {
             mergedPolygons = mergePolygons(dragOptions.mergedPolygons, currentPolygon, subtract);
             currentPolygon.subtract = subtract;
-            // TODO Probably isn't needed anymore
-            testPoly = multipolygonTester(dragOptions.polygons.concat([currentPolygon]));
         }
         else {
             mergedPolygons = [currentPolygon];
-            testPoly = polygonTester(currentPolygon);
         }
 
         // draw selection

From a630ef88c6d316c5b47f3f6b3111820e4ed5952c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 18 Jul 2018 11:05:26 +0200
Subject: [PATCH 27/45] Remove obsolete `selectPoints` from box trace's select
 module [1852]

---
 src/traces/box/select.js | 39 ---------------------------------------
 1 file changed, 39 deletions(-)

diff --git a/src/traces/box/select.js b/src/traces/box/select.js
index 720b669d943..a753d5ef460 100644
--- a/src/traces/box/select.js
+++ b/src/traces/box/select.js
@@ -8,45 +8,6 @@
 
 'use strict';
 
-// TODO Remove eventually
-function selectPoints(searchInfo, polygon) {
-    var cd = searchInfo.cd;
-    var xa = searchInfo.xaxis;
-    var ya = searchInfo.yaxis;
-    var selection = [];
-    var i, j;
-
-    if(polygon === false) {
-        for(i = 0; i < cd.length; i++) {
-            for(j = 0; j < (cd[i].pts || []).length; j++) {
-                // clear selection
-                cd[i].pts[j].selected = 0;
-            }
-        }
-    } else {
-        for(i = 0; i < cd.length; i++) {
-            for(j = 0; j < (cd[i].pts || []).length; j++) {
-                var pt = cd[i].pts[j];
-                var x = xa.c2p(pt.x);
-                var y = ya.c2p(pt.y);
-
-                if(polygon.contains([x, y])) {
-                    selection.push({
-                        pointNumber: pt.i,
-                        x: xa.c2d(pt.x),
-                        y: ya.c2d(pt.y)
-                    });
-                    pt.selected = 1;
-                } else {
-                    pt.selected = 0;
-                }
-            }
-        }
-    }
-
-    return selection;
-}
-
 exports.getPointsIn = function(searchInfo, polygon) {
     var pointsIn = [];
     var cd = searchInfo.cd;

From e07210cde698d8609bf34e6a23c49c6ce07484fe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 18 Jul 2018 11:08:07 +0200
Subject: [PATCH 28/45] Quick fix in dragbox module to satisfy ESLint [1852]

- Until clickMode attribute is not introduced in public API
  this hack needs to be there to satisfy ESLint.
---
 src/plots/cartesian/dragbox.js | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js
index b9e286c1396..ded7940a9c4 100644
--- a/src/plots/cartesian/dragbox.js
+++ b/src/plots/cartesian/dragbox.js
@@ -634,8 +634,13 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
         // Thought: we may end up returning thin wrapping functions that
         // call the real functions based on the `clickmode` attr
 
-        // return Fx.click;
-        return selectOnClick;
+        // FIXME Dummy code to satisfy ESLint
+        var clickMode = 'select';
+        if(clickMode === 'select') {
+            return selectOnClick;
+        } else {
+            return Fx.click;
+        }
     }
 
     function doubleClick() {

From e082e58abbd14a3ae6efeae219ea3293296253fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 18 Jul 2018 11:06:42 +0200
Subject: [PATCH 29/45] Remove obsolete `selectPoints` from scattergl's select
 module [1852]

- Also adapt the new `toggleSelected` function to style
  text data points properly.
---
 src/traces/scattergl/index.js | 95 +++++++----------------------------
 1 file changed, 17 insertions(+), 78 deletions(-)

diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js
index 26d1e529b14..f22d05f94a2 100644
--- a/src/traces/scattergl/index.js
+++ b/src/traces/scattergl/index.js
@@ -823,77 +823,6 @@ function calcHover(pointData, x, y, trace) {
     return pointData;
 }
 
-// TODO Remove eventually
-function selectPoints(searchInfo, polygon) {
-    var cd = searchInfo.cd;
-    var selection = [];
-    var trace = cd[0].trace;
-    var stash = cd[0].t;
-    var x = stash.x;
-    var y = stash.y;
-    var scene = stash._scene;
-
-    if(!scene) return selection;
-
-    var hasText = subTypes.hasText(trace);
-    var hasMarkers = subTypes.hasMarkers(trace);
-    var hasOnlyLines = !hasMarkers && !hasText;
-    if(trace.visible !== true || hasOnlyLines) return selection;
-
-    // degenerate polygon does not enable selection
-    // filter out points by visible scatter ones
-    var els = null;
-    var unels = null;
-    // FIXME: clearing selection does not work here
-    var i;
-    if(polygon !== false && !polygon.degenerate) {
-        els = [], unels = [];
-        for(i = 0; i < stash.count; i++) {
-            if(polygon.contains([stash.xpx[i], stash.ypx[i]])) {
-                els.push(i);
-                selection.push({
-                    pointNumber: i,
-                    x: x[i],
-                    y: y[i]
-                });
-            }
-            else {
-                unels.push(i);
-            }
-        }
-    } else {
-        unels = arrayRange(stash.count);
-    }
-
-    // make sure selectBatch is created
-    if(!scene.selectBatch) {
-        scene.selectBatch = [];
-        scene.unselectBatch = [];
-    }
-
-    if(!scene.selectBatch[stash.index]) {
-        // enter every trace select mode
-        for(i = 0; i < scene.count; i++) {
-            scene.selectBatch[i] = [];
-            scene.unselectBatch[i] = [];
-        }
-        // we should turn scatter2d into unselected once we have any points selected
-        if(hasMarkers) {
-            scene.scatter2d.update(scene.markerUnselectedOptions);
-        }
-    }
-
-    scene.selectBatch[stash.index] = els;
-    scene.unselectBatch[stash.index] = unels;
-
-    // update text options
-    if(hasText) {
-        styleTextSelection(cd);
-    }
-
-    return selection;
-}
-
 function getPointsIn(searchInfo, polygon) {
     var pointsIn = [];
 
@@ -905,7 +834,9 @@ function getPointsIn(searchInfo, polygon) {
 
     if(!scene) return [];
 
-    var hasOnlyLines = (!subTypes.hasMarkers(trace) && !subTypes.hasText(trace));
+    var hasText = subTypes.hasText(trace);
+    var hasMarkers = subTypes.hasMarkers(trace);
+    var hasOnlyLines = !hasMarkers && !hasText;
     if(trace.visible !== true || hasOnlyLines) return [];
 
     if(polygon !== false && !polygon.degenerate) {
@@ -925,6 +856,9 @@ function toggleSelected(searchInfo, selected, pointIds) {
     var calcData = searchInfo.cd;
     var stash = calcData[0].t;
     var scene = stash._scene;
+    var trace = calcData[0].trace;
+    var hasText = subTypes.hasText(trace);
+    var hasMarkers = subTypes.hasMarkers(trace);
     var oldEls;
     var oldUnels;
     var newEls;
@@ -936,7 +870,7 @@ function toggleSelected(searchInfo, selected, pointIds) {
     }
 
     // Mutate state
-    coerceSelectBatches(scene, stash);
+    coerceSelectBatches(scene, stash, hasMarkers);
     oldEls = scene.selectBatch[stash.index];
     oldUnels = scene.unselectBatch[stash.index];
 
@@ -960,6 +894,10 @@ function toggleSelected(searchInfo, selected, pointIds) {
         });
     }
 
+    if(hasText) {
+        styleTextSelection(calcData);
+    }
+
     // TODO remove eventually
     // console.log('els  : ' + JSON.stringify(scene.selectBatch[stash.index]));
     // console.log('unels: ' + JSON.stringify(scene.unselectBatch[stash.index]));
@@ -967,7 +905,7 @@ function toggleSelected(searchInfo, selected, pointIds) {
     return selection;
 }
 
-function coerceSelectBatches(scene, stash) {
+function coerceSelectBatches(scene, stash, hasMarkers) {
     var i;
 
     if(!scene.selectBatch) {
@@ -984,9 +922,10 @@ function coerceSelectBatches(scene, stash) {
 
         scene.unselectBatch[stash.index] = arrayRange(stash.count);
 
-        // TODO what does that do?
         // we should turn scatter2d into unselected once we have any points selected
-        scene.scatter2d.update(scene.markerUnselectedOptions);
+        if(hasMarkers) {
+            scene.scatter2d.update(scene.markerUnselectedOptions);
+        }
     }
 }
 
@@ -1010,8 +949,8 @@ function styleTextSelection(cd) {
     var stash = cd0.t;
     var scene = stash._scene;
     var index = stash.index;
-    var els = scene.selectBatch[index];
-    var unels = scene.unselectBatch[index];
+    var els = scene.selectBatch ? scene.selectBatch[index] : null;
+    var unels = scene.unselectBatch ? scene.unselectBatch[index] : null;
     var baseOpts = scene.textOptions[index];
     var selOpts = scene.textSelectedOptions[index] || {};
     var unselOpts = scene.textUnselectedOptions[index] || {};

From a97af32c4c4a70f3f347ad1a9b150b52e8c4e912 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 18 Jul 2018 13:30:33 +0200
Subject: [PATCH 30/45] Transition violin trace type to new selection interface
 [1852]

- Violin uses box trace type select implementation.
---
 src/traces/violin/index.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/traces/violin/index.js b/src/traces/violin/index.js
index 9c058ef998d..ff563e18b92 100644
--- a/src/traces/violin/index.js
+++ b/src/traces/violin/index.js
@@ -19,7 +19,8 @@ module.exports = {
     style: require('./style'),
     styleOnSelect: require('../scatter/style').styleOnSelect,
     hoverPoints: require('./hover'),
-    selectPoints: require('../box/select'),
+    getPointsIn: require('../box/select').getPointsIn,
+    toggleSelected: require('../box/select').toggleSelected,
 
     moduleType: 'trace',
     name: 'violin',

From aac89cefc644a9335077659dda5adc0e0a924c17 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 18 Jul 2018 14:24:18 +0200
Subject: [PATCH 31/45] Reactivate emitting 'plotly_click' events [1852]

---
 src/plots/cartesian/dragbox.js | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js
index ded7940a9c4..27fcc569643 100644
--- a/src/plots/cartesian/dragbox.js
+++ b/src/plots/cartesian/dragbox.js
@@ -160,7 +160,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
                 // to pan (or to zoom if it already is pan) on shift
                 if(e.shiftKey) {
                     if(dragModeNow === 'pan') dragModeNow = 'zoom';
-                    else if(!isSelectOrLasso(dragModeNow)) dragModeNow = 'pan';
+                    else if(!isBoxOrLassoSelect(dragModeNow)) dragModeNow = 'pan';
                 }
                 else if(e.ctrlKey) {
                     dragModeNow = 'pan';
@@ -173,8 +173,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
         if(dragModeNow === 'lasso') dragOptions.minDrag = 1;
         else dragOptions.minDrag = undefined;
 
-        // TODO Maybe rename into isBoxOrLassoSelect
-        if(isSelectOrLasso(dragModeNow)) {
+        if(isBoxOrLassoSelect(dragModeNow)) {
             dragOptions.xaxes = xaxes;
             dragOptions.yaxes = yaxes;
             // this attaches moveFn, clickFn, doneFn on dragOptions
@@ -182,7 +181,6 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
             prepSelect(e, startX, startY, dragOptions, dragModeNow);
         } else {
             dragOptions.clickFn = clickFn;
-            // TODO Is this good? prepFn will always clear and reset the selection when not in lasso or select mode, but now we've a select mode on click, a selectOnClick behavior so to speak
             clearAndResetSelect();
 
             if(!allFixedRanges) {
@@ -217,9 +215,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
         if(numClicks === 2 && !singleEnd) doubleClick();
 
         if(isMainDrag) {
-            var clickHandler = obtainClickHandler();
-            // TODO not sure a uniform interface of click handlers applies, probably better be explicit
-            clickHandler(gd, numClicks, evt, xaxes, yaxes);
+            handleClickInMainDrag(gd, numClicks, evt, xaxes, yaxes, plotinfo.id);
         }
         // Allow manual editing of range bounds through an input field
         // TODO consider extracting that to a method for clarity
@@ -629,18 +625,10 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
         redrawObjs(gd._fullLayout.images || [], Registry.getComponentMethod('images', 'draw'), true);
     }
 
-    function obtainClickHandler() {
-        // TODO differentiate based on `clickmode` attr here
-        // Thought: we may end up returning thin wrapping functions that
-        // call the real functions based on the `clickmode` attr
-
-        // FIXME Dummy code to satisfy ESLint
-        var clickMode = 'select';
-        if(clickMode === 'select') {
-            return selectOnClick;
-        } else {
-            return Fx.click;
-        }
+    function handleClickInMainDrag(gd, numClicks, evt, xaxes, yaxes, subplot) {
+        // TODO differentiate based on `clickmode` attr here as soon as it is available
+        selectOnClick(gd, numClicks, evt, xaxes, yaxes);
+        Fx.click(gd, evt, subplot);
     }
 
     function doubleClick() {
@@ -1036,7 +1024,7 @@ function showDoubleClickNotifier(gd) {
     }
 }
 
-function isSelectOrLasso(dragmode) {
+function isBoxOrLassoSelect(dragmode) {
     return dragmode === 'lasso' || dragmode === 'select';
 }
 

From 4b41e244f33bad633b4b256a0a2cbe81eaa1072e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 18 Jul 2018 15:17:26 +0200
Subject: [PATCH 32/45] Emit 'plotly_deselect' after outlines have been removed
 [1852]

- Reason: Jasmine tests in select_test.js are relying on outlines
  being removed when the event has been emitted.
---
 src/plots/cartesian/select.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 16cb5717370..9e52152bb07 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -284,10 +284,11 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
                 }
 
                 updateSelectedState(gd, searchTraces);
-                gd.emit('plotly_deselect', null);
 
                 // clear visual boundaries of selection area if displayed
                 outlines.remove();
+
+                gd.emit('plotly_deselect', null);
             }
             else {
                 // TODO What to do with the code below because we now have behavior for a single click

From f0822d5df59aee74c603d45c75dba22adfe17d73 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 18 Jul 2018 18:01:43 +0200
Subject: [PATCH 33/45] Fix testing if double-click cleans selection in
 box/lasso mode [1852]

- Additionally add `hoverMode` to `closest` because with click-to-select
  a click would actually trigger a selection when hoverData is not
  empty. This would be the case when `hoverMode` is set to `x`.
---
 test/jasmine/tests/select_test.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js
index 7709e0fc0bd..1f8196046a3 100644
--- a/test/jasmine/tests/select_test.js
+++ b/test/jasmine/tests/select_test.js
@@ -635,6 +635,7 @@ describe('@flaky Test select box and lasso in general:', function() {
         fig.layout.xaxis.range = [2, 8];
         fig.layout.yaxis.autorange = false;
         fig.layout.yaxis.range = [0, 3];
+        fig.layout.hovermode = 'closest';
 
         function _assert(msg, exp) {
             expect(gd.layout.xaxis.range)

From 8428bbcdbc3dbf328a3375f301fe67cbb9d0cc75 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 18 Jul 2018 18:04:08 +0200
Subject: [PATCH 34/45] Fix testing selection subtraction in box/lasso mode
 [1852]

- The Shift key wasn't specified in the drag event.
---
 test/jasmine/tests/select_test.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js
index 1f8196046a3..96202657388 100644
--- a/test/jasmine/tests/select_test.js
+++ b/test/jasmine/tests/select_test.js
@@ -259,7 +259,7 @@ describe('@flaky Test select box and lasso in general:', function() {
             })
             .then(function() {
                 // sub selection
-                drag([[219, 143], [219, 183]], {altKey: true});
+                drag([[219, 143], [219, 183]], {shiftKey: true, altKey: true});
             }).then(function() {
                 assertEventData(selectingData.points, [{
                     curveNumber: 0,

From f4384deee75617ef0cdcad5d73e664e3f99fe37b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 19 Jul 2018 08:03:52 +0200
Subject: [PATCH 35/45] Transition scattergeo trace type to new selection
 interface [1852]

- Thereby fix failing geo_point-selection.json image test.
---
 src/traces/scattergeo/index.js  |  3 +-
 src/traces/scattergeo/select.js | 76 ++++++++++++++++++++++-----------
 2 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/src/traces/scattergeo/index.js b/src/traces/scattergeo/index.js
index 1f2976735c5..c30fc125605 100644
--- a/src/traces/scattergeo/index.js
+++ b/src/traces/scattergeo/index.js
@@ -20,7 +20,8 @@ ScatterGeo.style = require('./style');
 ScatterGeo.styleOnSelect = require('../scatter/style').styleOnSelect;
 ScatterGeo.hoverPoints = require('./hover');
 ScatterGeo.eventData = require('./event_data');
-ScatterGeo.selectPoints = require('./select');
+ScatterGeo.getPointsIn = require('./select').getPointsIn;
+ScatterGeo.toggleSelected = require('./select').toggleSelected;
 
 ScatterGeo.moduleType = 'trace';
 ScatterGeo.name = 'scattergeo';
diff --git a/src/traces/scattergeo/select.js b/src/traces/scattergeo/select.js
index 6be13b21e5f..628ffb9f159 100644
--- a/src/traces/scattergeo/select.js
+++ b/src/traces/scattergeo/select.js
@@ -9,40 +9,66 @@
 'use strict';
 
 var subtypes = require('../scatter/subtypes');
+var arrayRange = require('array-range');
 
-module.exports = function selectPoints(searchInfo, polygon) {
+exports.getPointsIn = function(searchInfo, polygon) {
+    var pointsIn = [];
     var cd = searchInfo.cd;
+    var trace = cd[0].trace;
+    var hasOnlyLines = (!subtypes.hasMarkers(trace) && !subtypes.hasText(trace));
     var xa = searchInfo.xaxis;
     var ya = searchInfo.yaxis;
-    var selection = [];
-    var trace = cd[0].trace;
-
-    var di, lonlat, x, y, i;
+    var di;
+    var lonlat;
+    var x;
+    var y;
+    var i;
 
-    var hasOnlyLines = (!subtypes.hasMarkers(trace) && !subtypes.hasText(trace));
     if(hasOnlyLines) return [];
 
-    if(polygon === false) {
-        for(i = 0; i < cd.length; i++) {
-            cd[i].selected = 0;
+    for(i = 0; i < cd.length; i++) {
+        di = cd[i];
+        lonlat = di.lonlat;
+        x = xa.c2p(lonlat);
+        y = ya.c2p(lonlat);
+
+        if(polygon.contains([x, y])) {
+            pointsIn.push(i);
         }
-    } else {
-        for(i = 0; i < cd.length; i++) {
-            di = cd[i];
+    }
+
+    return pointsIn;
+};
+
+exports.toggleSelected = function(searchInfo, selected, pointIds) {
+    var selection = [];
+    var cd = searchInfo.cd;
+    var modifyAll = !Array.isArray(pointIds);
+    var di;
+    var pointId;
+    var lonlat;
+    var i;
+
+    if(modifyAll) {
+        pointIds = arrayRange(cd.length);
+    }
+
+    // Mutate state
+    for(i = 0; i < pointIds.length; i++) {
+        pointId = pointIds[i];
+        cd[pointId].selected = selected ? 1 : 0;
+    }
+
+    // Compute selection array from internal state
+    for(i = 0; i < cd.length; i++) {
+        di = cd[i];
+        if(di.selected === 1) {
             lonlat = di.lonlat;
-            x = xa.c2p(lonlat);
-            y = ya.c2p(lonlat);
-
-            if(polygon.contains([x, y])) {
-                selection.push({
-                    pointNumber: i,
-                    lon: lonlat[0],
-                    lat: lonlat[1]
-                });
-                di.selected = 1;
-            } else {
-                di.selected = 0;
-            }
+            selection.push({
+                pointNumber: i,
+                lon: lonlat[0],
+                lat: lonlat[1]
+            });
         }
     }
 

From bbd63d4060563e534f71f86496de6b67c12638a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Thu, 19 Jul 2018 08:22:48 +0200
Subject: [PATCH 36/45] Transition scattercarpet trace type to new selection
 interface [1852]

- Only the proper mapping to scatter's select interface was needed.
---
 src/traces/scattercarpet/index.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/traces/scattercarpet/index.js b/src/traces/scattercarpet/index.js
index eba9c0f0b66..e41a5969a59 100644
--- a/src/traces/scattercarpet/index.js
+++ b/src/traces/scattercarpet/index.js
@@ -18,7 +18,8 @@ ScatterCarpet.plot = require('./plot');
 ScatterCarpet.style = require('../scatter/style').style;
 ScatterCarpet.styleOnSelect = require('../scatter/style').styleOnSelect;
 ScatterCarpet.hoverPoints = require('./hover');
-ScatterCarpet.selectPoints = require('../scatter/select');
+ScatterCarpet.getPointsIn = require('../scatter/select').getPointsIn;
+ScatterCarpet.toggleSelected = require('../scatter/select').toggleSelected;
 ScatterCarpet.eventData = require('./event_data');
 
 ScatterCarpet.moduleType = 'trace';

From d7087a66e059cccf142d79f8ac661f5aa280a840 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= <etienne@plot.ly>
Date: Thu, 19 Jul 2018 17:59:10 -0400
Subject: [PATCH 37/45] speed up Lib.difference

---
 src/lib/set_operations.js | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/src/lib/set_operations.js b/src/lib/set_operations.js
index 26473084d30..131353527e7 100644
--- a/src/lib/set_operations.js
+++ b/src/lib/set_operations.js
@@ -9,25 +9,33 @@
 'use strict';
 
 
-/*
+/**
  * Computes the set difference of two arrays.
  *
  * @param {array} a
  * @param {array} b
- * @returns all elements of a that are not in b.
+ * @returns out all elements of a that are not in b.
  *      If a is not an array, an empty array is returned.
  *      If b is not an array, a is returned.
  */
 function difference(a, b) {
-    if(!Array.isArray(a)) {
-        return [];
+    if(!Array.isArray(a)) return [];
+    if(!Array.isArray(b)) return a;
+
+    var hash = {};
+    var out = [];
+    var i;
+
+    for(i = 0; i < b.length; i++) {
+        hash[b[i]] = 1;
     }
-    if(!Array.isArray(b)) {
-        return a;
+
+    for(i = 0; i < a.length; i++) {
+        var ai = a[i];
+        if(!hash[ai]) out.push(ai);
     }
-    return a.filter(function(e) {
-        return b.indexOf(e) < 0;
-    });
+
+    return out;
 }
 
 module.exports = {

From df351c3ff538ecadb0208883842bdaae66aec432 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Fri, 20 Jul 2018 15:12:47 +0200
Subject: [PATCH 38/45] Introduce _module.selectable flag for trace modules
 [1852]

- Reason: code for checking if a trace supports selection was duplicated
  across the code base.
---
 src/components/modebar/manage.js | 2 +-
 src/plots/cartesian/select.js    | 3 +--
 src/plots/plots.js               | 2 +-
 src/registry.js                  | 2 ++
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/components/modebar/manage.js b/src/components/modebar/manage.js
index f4ee57af541..255a30e001f 100644
--- a/src/components/modebar/manage.js
+++ b/src/components/modebar/manage.js
@@ -193,7 +193,7 @@ function isSelectable(fullData) {
 
         var trace = fullData[i];
 
-        if(!trace._module || !trace._module.getPointsIn || !trace._module.toggleSelected) continue;
+        if(!trace._module || !trace._module.selectable) continue;
 
         if(Registry.traceIs(trace, 'scatter-like')) {
             if(scatterSubTypes.hasMarkers(trace) || scatterSubTypes.hasText(trace)) {
diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 9e52152bb07..bedde40ede4 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -462,8 +462,7 @@ function determineSearchTraces(gd, xAxes, yAxes) {
         cd = gd.calcdata[i];
         trace = cd[0].trace;
 
-        if(trace.visible !== true || !trace._module ||
-          !trace._module.toggleSelected || !trace._module.getPointsIn) continue;
+        if(trace.visible !== true || !trace._module || !trace._module.selectable) continue;
 
         // TODO is dragOptions.subplot is ever set? If not, delete.
         // if(dragOptions.subplot) {
diff --git a/src/plots/plots.js b/src/plots/plots.js
index 1b0774b083d..ccabf27d5a3 100644
--- a/src/plots/plots.js
+++ b/src/plots/plots.js
@@ -1171,7 +1171,7 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac
             traceOut.visible = !!traceOut.visible;
         }
 
-        if(_module && _module.getPointsIn && _module.toggleSelected) {
+        if(_module && _module.selectable) {
             coerce('selectedpoints');
         }
 
diff --git a/src/registry.js b/src/registry.js
index 953e58b285e..3235ca7387a 100644
--- a/src/registry.js
+++ b/src/registry.js
@@ -241,6 +241,8 @@ function registerTraceModule(_module) {
         exports.allCategories[categoriesIn[i]] = true;
     }
 
+    _module.selectable = _module.getPointsIn && _module.toggleSelected;
+
     exports.modules[thisType] = {
         _module: _module,
         categories: categoryObj

From b703a9c20bd9a955179702c40740de1c0fd64e89 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Tue, 7 Aug 2018 15:27:09 +0200
Subject: [PATCH 39/45] Consider subplot when determining traces for
 click-to-select [1852]

---
 src/plots/cartesian/select.js | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index bedde40ede4..9ba091717eb 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -108,7 +108,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
     var throttleID = fullLayout._uid + constants.SELECTID;
 
     // find the traces to search for selection points
-    var searchTraces = determineSearchTraces(gd, dragOptions.xaxes, dragOptions.yaxes);
+    var searchTraces = determineSearchTraces(gd, dragOptions.xaxes, dragOptions.yaxes, dragOptions.subplot);
 
     function axValue(ax) {
         var index = (ax._id.charAt(0) === 'y') ? 1 : 0;
@@ -292,7 +292,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
             }
             else {
                 // TODO What to do with the code below because we now have behavior for a single click
-                selectOnClick(gd, numClicks, evt, dragOptions.xaxes, dragOptions.yaxes, outlines);
+                selectOnClick(gd, numClicks, evt, dragOptions.xaxes, dragOptions.yaxes, outlines, dragOptions.subplot);
 
                 // TODO: remove in v2 - this was probably never intended to work as it does,
                 // but in case anyone depends on it we don't want to break it now.
@@ -327,7 +327,7 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
 // ----------------
 // TODO handle clearing selection when no point is clicked (based on hoverData)
 // TODO remove console.log statements
-function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
+function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines, subplot) {
     var hoverData = gd._hoverdata;
     var retainSelection = shouldRetainSelection(evt);
     var searchTraces;
@@ -346,7 +346,7 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
     if(numClicks === 1 && isHoverDataSet(hoverData)) {
         allSelectionItems = [];
 
-        searchTraces = determineSearchTraces(gd, xAxes, yAxes);
+        searchTraces = determineSearchTraces(gd, xAxes, yAxes, subplot);
         clearEntireSelection = entireSelectionToBeCleared(searchTraces, hoverData);
 
         for(i = 0; i < searchTraces.length; i++) {
@@ -450,7 +450,7 @@ function selectOnClick(gd, numClicks, evt, xAxes, yAxes, outlines) {
     }
 }
 
-function determineSearchTraces(gd, xAxes, yAxes) {
+function determineSearchTraces(gd, xAxes, yAxes, subplot) {
     var searchTraces = [];
     var xAxisIds = xAxes.map(getAxId);
     var yAxisIds = yAxes.map(getAxId);
@@ -464,21 +464,9 @@ function determineSearchTraces(gd, xAxes, yAxes) {
 
         if(trace.visible !== true || !trace._module || !trace._module.selectable) continue;
 
-        // TODO is dragOptions.subplot is ever set? If not, delete.
-        // if(dragOptions.subplot) {
-        //     if(
-        //       trace.subplot === dragOptions.subplot ||
-        //       trace.geo === dragOptions.subplot
-        //     ) {
-        //         searchTraces.push({
-        //             _module: trace._module,
-        //             cd: cd,
-        //             xaxis: xAxes[0],
-        //             yaxis: yAxes[0]
-        //         });
-        //     }
-        // } else if(
-        if(
+        if(subplot && (trace.subplot === subplot || trace.geo === subplot)) {
+            searchTraces.push(createSearchInfo(trace._module, cd, xAxes[0], yAxes[0]));
+        } else if(
           trace.type === 'splom' &&
           // FIXME: make sure we don't have more than single axis for splom
           trace._xaxes[xAxisIds[0]] && trace._yaxes[yAxisIds[0]]

From d7e64dd949a91263d580dd77ff8d39e0aefd525e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Tue, 7 Aug 2018 17:43:24 +0200
Subject: [PATCH 40/45] Put back call to remove outlines back to original
 location [1852]

- Reason: to prevent confusion when comparing code to previous
  versions.
- See comment in PR #2824 for more info.
---
 src/plots/cartesian/select.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
index 9ba091717eb..6d147cfe921 100644
--- a/src/plots/cartesian/select.js
+++ b/src/plots/cartesian/select.js
@@ -283,11 +283,11 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
                     searchInfo._module.toggleSelected(searchInfo, false);
                 }
 
-                updateSelectedState(gd, searchTraces);
-
                 // clear visual boundaries of selection area if displayed
                 outlines.remove();
 
+                updateSelectedState(gd, searchTraces);
+
                 gd.emit('plotly_deselect', null);
             }
             else {

From c4012e17ecaa3dc3a384e6599c22b37320873591 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Tue, 7 Aug 2018 18:13:42 +0200
Subject: [PATCH 41/45] Reintroduce selectMode guard in scattegl [1852]

- Reason: to spare the creation of an unnecessary regl context when
  selection can't take place.
- TODO: use the not yet introduced clickmode layout attribute to set
  the select mode.
---
 src/traces/scattergl/index.js | 49 +++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js
index ce6f662145d..b2001a0febf 100644
--- a/src/traces/scattergl/index.js
+++ b/src/traces/scattergl/index.js
@@ -522,7 +522,9 @@ function plot(gd, subplot, cdata) {
     scene.selectBatch = null;
     scene.unselectBatch = null;
     var dragmode = fullLayout.dragmode;
-    var selectMode = dragmode === 'lasso' || dragmode === 'select';
+    // TODO get that from layout as soon as clickmode attribute has been added
+    var clickmode = 'select';
+    var selectMode = (dragmode === 'lasso' || dragmode === 'select' || clickmode === 'select');
 
     for(i = 0; i < cdata.length; i++) {
         var cd0 = cdata[i][0];
@@ -571,31 +573,32 @@ function plot(gd, subplot, cdata) {
         }
     }
 
+    if(selectMode) {
+        // create select2d
+        if(!scene.select2d) {
+            // create scatter instance by cloning scatter2d
+            scene.select2d = createScatter(fullLayout._glcanvas.data()[1].regl);
+        }
 
-    // create select2d
-    if(!scene.select2d) {
-        // create scatter instance by cloning scatter2d
-        scene.select2d = createScatter(fullLayout._glcanvas.data()[1].regl);
-    }
-
-    if(scene.scatter2d && scene.selectBatch && scene.selectBatch.length) {
-        // update only traces with selection
-        scene.scatter2d.update(scene.markerUnselectedOptions.map(function(opts, i) {
-            return scene.selectBatch[i] ? opts : null;
-        }));
-    }
+        if(scene.scatter2d && scene.selectBatch && scene.selectBatch.length) {
+            // update only traces with selection
+            scene.scatter2d.update(scene.markerUnselectedOptions.map(function(opts, i) {
+                return scene.selectBatch[i] ? opts : null;
+            }));
+        }
 
-    if(scene.select2d) {
-        scene.select2d.update(scene.markerOptions);
-        scene.select2d.update(scene.markerSelectedOptions);
-    }
+        if(scene.select2d) {
+            scene.select2d.update(scene.markerOptions);
+            scene.select2d.update(scene.markerSelectedOptions);
+        }
 
-    if(scene.glText) {
-        cdata.forEach(function(cdscatter) {
-            if(cdscatter && cdscatter[0] && cdscatter[0].trace) {
-                styleTextSelection(cdscatter);
-            }
-        });
+        if(scene.glText) {
+            cdata.forEach(function(cdscatter) {
+                if(cdscatter && cdscatter[0] && cdscatter[0].trace) {
+                    styleTextSelection(cdscatter);
+                }
+            });
+        }
     }
 
     // provide viewport and range

From f9494b53435c4b738448b4ac82e5b0428c18d57d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 8 Aug 2018 08:07:42 +0200
Subject: [PATCH 42/45] Fix modebar Jasmine tests involving selectable traces
 [1852]

- Reason: mocking of `gd._fullData._module` was missing the `selectable`
  property introduced a few commits earlier to centralize checking if
  a trace supports selection.
---
 test/jasmine/tests/modebar_test.js | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/jasmine/tests/modebar_test.js b/test/jasmine/tests/modebar_test.js
index 31b5c8acfeb..4f2ca3a66c4 100644
--- a/test/jasmine/tests/modebar_test.js
+++ b/test/jasmine/tests/modebar_test.js
@@ -356,7 +356,7 @@ describe('ModeBar', function() {
                 type: 'scatter',
                 visible: true,
                 mode: 'markers',
-                _module: {getPointsIn: true, toggleSelected: true}
+                _module: {selectable: true}
             }];
 
             manageModeBar(gd);
@@ -380,7 +380,7 @@ describe('ModeBar', function() {
                 type: 'box',
                 visible: true,
                 boxpoints: 'all',
-                _module: {getPointsIn: true, toggleSelected: true}
+                _module: {selectable: true}
             }];
 
             manageModeBar(gd);
@@ -452,7 +452,7 @@ describe('ModeBar', function() {
                 type: 'scattergeo',
                 visible: true,
                 mode: 'markers',
-                _module: {getPointsIn: true, toggleSelected: true}
+                _module: {selectable: true}
             }];
 
             manageModeBar(gd);
@@ -492,7 +492,7 @@ describe('ModeBar', function() {
                 type: 'scatter',
                 visible: true,
                 mode: 'markers',
-                _module: {getPointsIn: true, toggleSelected: true}
+                _module: {selectable: true}
             }];
 
             manageModeBar(gd);
@@ -584,7 +584,7 @@ describe('ModeBar', function() {
                 type: 'scatter',
                 visible: true,
                 mode: 'markers',
-                _module: {getPointsIn: true, toggleSelected: true}
+                _module: {selectable: true}
             }];
 
             manageModeBar(gd);
@@ -606,7 +606,7 @@ describe('ModeBar', function() {
                 type: 'scatter',
                 visible: true,
                 mode: 'markers',
-                _module: {getPointsIn: true, toggleSelected: true}
+                _module: {selectable: true}
             }];
             gd._fullLayout.xaxis = {fixedrange: false};
             gd._fullLayout._basePlotModules = [{ name: 'cartesian' }, { name: 'pie' }];
@@ -662,7 +662,7 @@ describe('ModeBar', function() {
                 type: 'scatterternary',
                 visible: true,
                 mode: 'markers',
-                _module: {getPointsIn: true, toggleSelected: true}
+                _module: {selectable: true}
             }];
             gd._fullLayout._basePlotModules = [{ name: 'ternary' }];
 

From e7815e965dac2387eb8c92ec802a32672d0f1ac1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 8 Aug 2018 12:53:49 +0200
Subject: [PATCH 43/45] Transition scattermapbox trace type to new selection
 interface [1852]

---
 src/plots/mapbox/mapbox.js         | 27 +++++++----
 src/traces/scattermapbox/index.js  |  3 +-
 src/traces/scattermapbox/select.js | 73 ++++++++++++++++++++----------
 3 files changed, 67 insertions(+), 36 deletions(-)

diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js
index c5332d05fd8..70e63697317 100644
--- a/src/plots/mapbox/mapbox.js
+++ b/src/plots/mapbox/mapbox.js
@@ -15,6 +15,7 @@ var Fx = require('../../components/fx');
 var Lib = require('../../lib');
 var dragElement = require('../../components/dragelement');
 var prepSelect = require('../cartesian/select').prepSelect;
+var selectOnClick = require('../cartesian/select').selectOnClick;
 var constants = require('./constants');
 var layoutAttributes = require('./layout_attributes');
 var createMapboxLayer = require('./layers');
@@ -176,15 +177,6 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) {
         Fx.hover(gd, evt, self.id);
     });
 
-    map.on('click', function(evt) {
-        // TODO: this does not support right-click. If we want to support it, we
-        // would likely need to change mapbox to use dragElement instead of straight
-        // mapbox event binding. Or perhaps better, make a simple wrapper with the
-        // right mousedown, mousemove, and mouseup handlers just for a left/right click
-        // pie would use this too.
-        Fx.click(gd, evt.originalEvent);
-    });
-
     function unhover() {
         Fx.loneUnhover(fullLayout._toppaper);
     }
@@ -221,11 +213,17 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) {
         gd.emit('plotly_relayout', evtData);
     }
 
-    // define clear select on map creation, to keep one ref per map,
+    // define event handlers on map creation, to keep one ref per map,
     // so that map.on / map.off in updateFx works as expected
     self.clearSelect = function() {
         gd._fullLayout._zoomlayer.selectAll('.select-outline').remove();
     };
+
+    self.onClickInPan = function(evt) {
+        // TODO Add condition evaluating clickmode attr when it has been introduced
+        selectOnClick(gd, 1, evt.originalEvent, [self.xaxis], [self.yaxis], undefined, self.id);
+        Fx.click(gd, evt.originalEvent);
+    };
 };
 
 proto.updateMap = function(calcData, fullLayout, resolve, reject) {
@@ -404,10 +402,19 @@ proto.updateFx = function(fullLayout) {
         };
 
         dragElement.init(dragOptions);
+
+        map.off('click', self.onClickInPan);
     } else {
         map.dragPan.enable();
         map.off('zoomstart', self.clearSelect);
         self.div.onmousedown = null;
+
+        // TODO: this does not support right-click. If we want to support it, we
+        // would likely need to change mapbox to use dragElement instead of straight
+        // mapbox event binding. Or perhaps better, make a simple wrapper with the
+        // right mousedown, mousemove, and mouseup handlers just for a left/right click
+        // pie would use this too.
+        map.on('click', self.onClickInPan);
     }
 };
 
diff --git a/src/traces/scattermapbox/index.js b/src/traces/scattermapbox/index.js
index ea58c0936b8..55f402b1fec 100644
--- a/src/traces/scattermapbox/index.js
+++ b/src/traces/scattermapbox/index.js
@@ -18,7 +18,8 @@ ScatterMapbox.calc = require('../scattergeo/calc');
 ScatterMapbox.plot = require('./plot');
 ScatterMapbox.hoverPoints = require('./hover');
 ScatterMapbox.eventData = require('./event_data');
-ScatterMapbox.selectPoints = require('./select');
+ScatterMapbox.getPointsIn = require('./select').getPointsIn;
+ScatterMapbox.toggleSelected = require('./select').toggleSelected;
 
 ScatterMapbox.style = function(_, cd) {
     if(cd) {
diff --git a/src/traces/scattermapbox/select.js b/src/traces/scattermapbox/select.js
index ade2ea5ceeb..fb842c2f0f5 100644
--- a/src/traces/scattermapbox/select.js
+++ b/src/traces/scattermapbox/select.js
@@ -11,43 +11,66 @@
 var Lib = require('../../lib');
 var subtypes = require('../scatter/subtypes');
 var BADNUM = require('../../constants/numerical').BADNUM;
+var arrayRange = require('array-range');
 
-module.exports = function selectPoints(searchInfo, polygon) {
+exports.getPointsIn = function(searchInfo, polygon) {
+    var pointsIn = [];
     var cd = searchInfo.cd;
     var xa = searchInfo.xaxis;
     var ya = searchInfo.yaxis;
-    var selection = [];
     var trace = cd[0].trace;
     var i;
 
     if(!subtypes.hasMarkers(trace)) return [];
 
-    if(polygon === false) {
-        for(i = 0; i < cd.length; i++) {
-            cd[i].selected = 0;
-        }
-    } else {
-        for(i = 0; i < cd.length; i++) {
-            var di = cd[i];
-            var lonlat = di.lonlat;
-
-            if(lonlat[0] !== BADNUM) {
-                var lonlat2 = [Lib.wrap180(lonlat[0]), lonlat[1]];
-                var xy = [xa.c2p(lonlat2), ya.c2p(lonlat2)];
-
-                if(polygon.contains(xy)) {
-                    selection.push({
-                        pointNumber: i,
-                        lon: lonlat[0],
-                        lat: lonlat[1]
-                    });
-                    di.selected = 1;
-                } else {
-                    di.selected = 0;
-                }
+    for(i = 0; i < cd.length; i++) {
+        var di = cd[i];
+        var lonlat = di.lonlat;
+
+        if(lonlat[0] !== BADNUM) {
+            var lonlat2 = [Lib.wrap180(lonlat[0]), lonlat[1]];
+            var xy = [xa.c2p(lonlat2), ya.c2p(lonlat2)];
+
+            if(polygon.contains(xy)) {
+                pointsIn.push(i);
             }
         }
     }
 
+    return pointsIn;
+};
+
+exports.toggleSelected = function(searchInfo, selected, pointIds) {
+    var selection = [];
+    var cd = searchInfo.cd;
+    var modifyAll = !Array.isArray(pointIds);
+    var di;
+    var pointId;
+    var lonlat;
+    var i;
+
+    if(modifyAll) {
+        pointIds = arrayRange(cd.length);
+    }
+
+    // Mutate state
+    for(i = 0; i < pointIds.length; i++) {
+        pointId = pointIds[i];
+        cd[pointId].selected = selected ? 1 : 0;
+    }
+
+    // Compute selection array from internal state
+    for(i = 0; i < cd.length; i++) {
+        di = cd[i];
+        if(di.selected === 1) {
+            lonlat = di.lonlat;
+            selection.push({
+                pointNumber: i,
+                lon: lonlat[0],
+                lat: lonlat[1]
+            });
+        }
+    }
+
     return selection;
 };

From c88aee0d80e0a319f069a5783eccf58d0eb880ac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 8 Aug 2018 15:10:23 +0200
Subject: [PATCH 44/45] Prevent mapbox removing select outlines on every drag
 operation [1852]

- Reason: `cartesian/select.js.prepSelect` expects
  `dragOptions.plotinfo` to have an `id` property set. Mapbox base plot
  wasn't setting this id property and thus cartesian select always
  cleared existing outlines (user holding Shift key)
---
 src/plots/mapbox/mapbox.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js
index 70e63697317..44fdbc250a7 100644
--- a/src/plots/mapbox/mapbox.js
+++ b/src/plots/mapbox/mapbox.js
@@ -388,6 +388,7 @@ proto.updateFx = function(fullLayout) {
             element: self.div,
             gd: gd,
             plotinfo: {
+                id: self.id,
                 xaxis: self.xaxis,
                 yaxis: self.yaxis,
                 fillRangeItems: fillRangeItems

From bd04fd8cfcf3a12a485b4d7d48dc69b8f9b5a3d9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=B6stl?= <robert.moestl@scalingcurve.at>
Date: Wed, 8 Aug 2018 21:15:26 +0200
Subject: [PATCH 45/45] Disable click-to-select in pan/zoom mode temporarily
 [1852]

- Reason: until clickmode attribute is not introduced, Jasmine spies in
  gl2d_plot_interact_test are expecting different default behavior.
---
 src/traces/scattergl/index.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js
index b2001a0febf..c237b0ea5f6 100644
--- a/src/traces/scattergl/index.js
+++ b/src/traces/scattergl/index.js
@@ -523,7 +523,7 @@ function plot(gd, subplot, cdata) {
     scene.unselectBatch = null;
     var dragmode = fullLayout.dragmode;
     // TODO get that from layout as soon as clickmode attribute has been added
-    var clickmode = 'select';
+    var clickmode = 'click';
     var selectMode = (dragmode === 'lasso' || dragmode === 'select' || clickmode === 'select');
 
     for(i = 0; i < cdata.length; i++) {