Skip to content

Persistent point selection with transforms compatibility #2163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
46 changes: 46 additions & 0 deletions src/components/fx/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,52 @@ exports.quadrature = function quadrature(dx, dy) {
};
};

/** Fill event data point object for hover and selection.
* Invokes _module.eventData if present.
*
* N.B. note that point 'index' corresponds to input data array index
* whereas 'number' is its post-transform version.
*
* If the hovered/selected pt corresponds to an multiple input points
* (e.g. for histogram and transformed traces), 'pointNumbers` and 'pointIndices'
* are include in the event data.
*
* @param {object} pt
* @param {object} trace
* @param {object} cd
* @return {object}
*/
exports.makeEventData = function makeEventData(pt, trace, cd) {
// hover uses 'index', select uses 'pointNumber'
var pointNumber = 'index' in pt ? pt.index : pt.pointNumber;

var out = {
data: trace._input,
fullData: trace,
curveNumber: trace.index,
pointNumber: pointNumber
};


if(trace._module.eventData) {
out = trace._module.eventData(out, pt, trace, cd, pointNumber);
} else {
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;
}

exports.appendArrayPointValue(out, trace, pointNumber);

return out;
};

/** Appends values inside array attributes corresponding to given point number
*
* @param {object} pointData : point data object (gets mutated here)
Expand Down
21 changes: 1 addition & 20 deletions src/components/fx/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,26 +417,7 @@ function _hover(gd, evt, subplot, noHoverEvent) {
// other people and send it to the event
for(itemnum = 0; itemnum < hoverData.length; itemnum++) {
var pt = hoverData[itemnum];

var out = {
data: pt.trace._input,
fullData: pt.trace,
curveNumber: pt.trace.index,
pointNumber: pt.index
};

if(pt.trace._module.eventData) out = pt.trace._module.eventData(out, pt);
else {
out.x = pt.xVal;
out.y = pt.yVal;
out.xaxis = pt.xa;
out.yaxis = pt.ya;

if(pt.zLabelVal !== undefined) out.z = pt.zLabelVal;
}

helpers.appendArrayPointValue(out, pt.trace, pt.index);
newhoverdata.push(out);
newhoverdata.push(helpers.makeEventData(pt, pt.trace, pt.cd));
}

gd._hoverdata = newhoverdata;
Expand Down
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
35 changes: 15 additions & 20 deletions src/plots/cartesian/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ var polybool = require('polybooljs');
var polygon = require('../../lib/polygon');
var throttle = require('../../lib/throttle');
var color = require('../../components/color');
var appendArrayPointValue = require('../../components/fx/helpers').appendArrayPointValue;
var makeEventData = require('../../components/fx/helpers').makeEventData;

var axes = require('./axes');
var constants = require('./constants');
Expand Down 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 Expand Up @@ -355,15 +354,11 @@ function mergePolygons(list, poly, subtract) {

function fillSelectionItem(selection, searchInfo) {
if(Array.isArray(selection)) {
var cd = searchInfo.cd;
var trace = searchInfo.cd[0].trace;

for(var i = 0; i < selection.length; i++) {
var sel = selection[i];

sel.curveNumber = trace.index;
sel.data = trace._input;
sel.fullData = trace;
appendArrayPointValue(sel, trace, sel.pointNumber);
selection[i] = makeEventData(selection[i], trace, cd);
}
}

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);
}
}
24 changes: 13 additions & 11 deletions src/traces/histogram/event_data.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,23 @@
* LICENSE file in the root directory of this source tree.
*/


'use strict';


module.exports = function eventData(out, pt) {
module.exports = function eventData(out, pt, trace, cd, pointNumber) {
// standard cartesian event data
out.x = pt.xVal;
out.y = pt.yVal;
out.xaxis = pt.xa;
out.yaxis = pt.ya;
out.x = 'xVal' in pt ? pt.xVal : pt.x;
out.y = 'yVal' in pt ? pt.yVal : pt.y;

if(pt.xa) out.xaxis = pt.xa;
if(pt.ya) out.yaxis = pt.ya;

// specific to histogram - CDFs do not have pts (yet?)
if(!(trace.cumulative || {}).enabled) {
var pts = Array.isArray(pointNumber) ?
cd[0].pts[pointNumber[0]][pointNumber[1]] :
cd[pointNumber].pts;

// specific to histogram
// CDFs do not have pts (yet?)
if(pt.pts) {
out.pointNumbers = pt.pts;
out.pointNumbers = pts;
out.binNumber = out.pointNumber;
delete out.pointNumber;
}
Expand Down
1 change: 0 additions & 1 deletion src/traces/histogram/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
var posLetter = trace.orientation === 'h' ? 'y' : 'x';

pointData[posLetter + 'Label'] = hoverLabelText(pointData[posLetter + 'a'], di.p0, di.p1);
pointData.pts = di.pts;
}

return pts;
Expand Down
1 change: 0 additions & 1 deletion src/traces/histogram2d/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, hoverLay

pointData.xLabel = hoverLabelText(pointData.xa, xRange[0], xRange[1]);
pointData.yLabel = hoverLabelText(pointData.ya, yRange[0], yRange[1]);
pointData.pts = cd0.pts[ny][nx];

return pts;
};
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);
}
};
Loading