Skip to content
Merged
51 changes: 51 additions & 0 deletions src/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,57 @@ lib.extractOption = function(calcPt, trace, calcKey, traceKey) {
if(!Array.isArray(traceVal)) return traceVal;
};

/** Tag selected calcdata items
*
* N.B. note that point 'index' corresponds to input data array index
* whereas 'number' is its post-transform version.
*
* @param {array} calcTrace
* @param {object} trace
* - selectedpoints {array}
* - _indexToPoints {object}
* @param {ptNumber2cdIndex} ptNumber2cdIndex (optional)
* optional map object for trace types that do not have 1-to-1 point number to
* calcdata item index correspondence (e.g. histogram)
*/
lib.tagSelected = function(calcTrace, trace, ptNumber2cdIndex) {
var selectedpoints = trace.selectedpoints;
var indexToPoints = trace._indexToPoints;
var ptIndex2ptNumber;

// make pt index-to-number map object, which takes care of transformed traces
if(indexToPoints) {
ptIndex2ptNumber = {};
for(var k in indexToPoints) {
var pts = indexToPoints[k];
for(var j = 0; j < pts.length; j++) {
ptIndex2ptNumber[pts[j]] = k;
}
}
}

function isPtIndexValid(v) {
return lib.validate(v, {valType: 'integer', min: 0});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha, that's super 🌴 but might be a bit slow for this 🌶 path - non-negative integers are just
typeof v === 'number' && v >= 0 && v % 1 === 0 right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

🐎 > 🌴 in 🌶️ code paths

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh hmm I guess this is looking at user input so it should be isNumeric(v) instead of typeof v === 'number' ie allow index '1' as well as 1

Copy link
Contributor Author

@etpinard etpinard Nov 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in c8d715b 451778b

}

function isCdIndexValid(v) {
return v !== undefined && v < calcTrace.length;
}

for(var i = 0; i < selectedpoints.length; i++) {
var ptIndex = selectedpoints[i];

if(isPtIndexValid(ptIndex)) {
var ptNumber = ptIndex2ptNumber ? ptIndex2ptNumber[ptIndex] : ptIndex;
var cdIndex = ptNumber2cdIndex ? ptNumber2cdIndex[ptNumber] : ptNumber;

if(isCdIndexValid(cdIndex)) {
calcTrace[cdIndex].selected = 1;
}
}
}
};

/** Returns target as set by 'target' transform attribute
*
* @param {object} trace : full trace object
Expand Down
25 changes: 12 additions & 13 deletions src/plots/cartesian/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,7 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) {
traceSelection = searchInfo.selectPoints(searchInfo, testPoly);
traceSelections.push(traceSelection);

var thisSelection = fillSelectionItem(
traceSelection, searchInfo
);
var thisSelection = fillSelectionItem(traceSelection, searchInfo);
if(selection.length) {
for(var j = 0; j < thisSelection.length; j++) {
selection.push(thisSelection[j]);
Expand Down Expand Up @@ -294,30 +292,31 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) {
};

function updateSelectedState(gd, searchTraces, eventData) {
var i, searchInfo;
var i, searchInfo, trace;

if(eventData) {
var pts = eventData.points || [];

for(i = 0; i < searchTraces.length; i++) {
searchInfo = searchTraces[i];
searchInfo.cd[0].trace.selectedpoints = [];
searchInfo.cd[0].trace._input.selectedpoints = [];
trace = searchTraces[i].cd[0].trace;
trace.selectedpoints = [];
trace._input.selectedpoints = [];
}

for(i = 0; i < pts.length; i++) {
var pt = pts[i];
var ptNumber = pt.pointNumber;
var data = pt.data;
var fullData = pt.fullData;

pt.data.selectedpoints.push(ptNumber);
pt.fullData.selectedpoints.push(ptNumber);
data.selectedpoints.push(pt.pointIndex);
fullData.selectedpoints.push(pt.pointIndex);
}
}
else {
for(i = 0; i < searchTraces.length; i++) {
searchInfo = searchTraces[i];
delete searchInfo.cd[0].trace.selectedpoints;
delete searchInfo.cd[0].trace._input.selectedpoints;
trace = searchTraces[i].cd[0].trace;
delete trace.selectedpoints;
delete trace._input.selectedpoints;
}
}

Expand Down
16 changes: 15 additions & 1 deletion src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -2241,7 +2241,21 @@ plots.doCalcdata = function(gd, traces) {

if(trace.visible === true) {
_module = trace._module;
if(_module && _module.calc) cd = _module.calc(gd, trace);

// keep ref of index-to-points map object of the *last* enabled transform,
// this index-to-points map object is required to determine the calcdata indices
// that correspond to input indices (e.g. from 'selectedpoints')
var transforms = trace.transforms || [];
for(j = transforms.length - 1; j >= 0; j--) {
if(transforms[j].enabled) {
trace._indexToPoints = transforms[j]._indexToPoints;
break;
}
}

if(_module && _module.calc) {
cd = _module.calc(gd, trace);
}
}

// Make sure there is a first point.
Expand Down
13 changes: 7 additions & 6 deletions src/traces/box/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ module.exports = function calc(gd, trace) {
}
}

calcSelection(cd, trace);
Axes.expand(valAxis, val, {padded: true});

if(cd.length > 0) {
Expand Down Expand Up @@ -193,13 +194,13 @@ function arraysToCalcdata(pt, trace, i) {
pt[trace2calc[k]] = trace[k][i];
}
}
}

var selectedpoints = trace.selectedpoints;

// TODO this is slow
if(Array.isArray(selectedpoints)) {
if(selectedpoints.indexOf(pt.i) !== -1) {
pt.selected = 1;
function calcSelection(cd, trace) {
if(Array.isArray(trace.selectedpoints)) {
for(var i = 0; i < cd.length; i++) {
var pts = cd[i].pts || [];
Lib.tagSelected(pts, trace);
}
}
}
Expand Down
18 changes: 16 additions & 2 deletions src/traces/histogram/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@ var Lib = require('../../lib');
var Axes = require('../../plots/cartesian/axes');

var arraysToCalcdata = require('../bar/arrays_to_calcdata');
var calcSelection = require('../scatter/calc_selection');
var binFunctions = require('./bin_functions');
var normFunctions = require('./norm_functions');
var doAvg = require('./average');
var cleanBins = require('./clean_bins');
var oneMonth = require('../../constants/numerical').ONEAVGMONTH;
var getBinSpanLabelRound = require('./bin_label_vals');


module.exports = function calc(gd, trace) {
// ignore as much processing as possible (and including in autorange) if bar is not visible
if(trace.visible !== true) return;
Expand Down Expand Up @@ -512,3 +510,19 @@ function cdf(size, direction, currentBin) {
}
}
}

function calcSelection(cd, trace) {
if(Array.isArray(trace.selectedpoints)) {
var ptNumber2cdIndex = {};

// make histogram-specific pt-number-to-cd-index map object
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we want this available for use later, ie without a recalc? I would have imagined creating this in the main binning loop and stashing it in the full trace, also possibly creating it as an array rather than a map, seems like that would be more efficient and would work just as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we want this available for use later, ie without a recalc?

I don't think so. Calling restyle with selectedpoints has to trigger a recalc. On user selections, the calcdata items are mutated directly w/o needing to refer back to input data-array indices. Am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have imagined creating this in the main binning loop

Good call, done in -> 7543dc6

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling restyle with selectedpoints has to trigger a recalc.

hmm ok, I was hoping we could avoid that but we can discuss optimizing that later.

for(var i = 0; i < cd.length; i++) {
var pts = cd[i].pts || [];
for(var j = 0; j < pts.length; j++) {
ptNumber2cdIndex[pts[j]] = i;
}
}

Lib.tagSelected(cd, trace, ptNumber2cdIndex);
}
}
18 changes: 5 additions & 13 deletions src/traces/scatter/calc_selection.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,12 @@

'use strict';

var isNumeric = require('fast-isnumeric');
var Lib = require('../../lib');

module.exports = function calcSelection(cd, trace) {
var selectedpoints = trace.selectedpoints;

// TODO ids vs points??
// TODO ids vs points??

if(Array.isArray(selectedpoints)) {
for(var i = 0; i < selectedpoints.length; i++) {
var ptNumber = selectedpoints[i];

if(isNumeric(ptNumber)) {
cd[+ptNumber].selected = 1;
}
}
module.exports = function calcSelection(cd, trace) {
if(Array.isArray(trace.selectedpoints)) {
Lib.tagSelected(cd, trace);
}
};