Skip to content
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.

Sign up for GitHub

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

Add multiselect #2140

Merged
merged 9 commits into from
Nov 13, 2017
Merged

Add multiselect #2140

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b800e55
Add multiselect
dy Nov 2, 2017
05b0dca
Fix failing cases
dy Nov 2, 2017
40fbca9
Avoid overcalculation
dy Nov 2, 2017
473d34d
Replace poly-bool
dy Nov 2, 2017
02db56f
Enhance readability
dy Nov 2, 2017
e8c18f0
Add subtraction mode
dy Nov 3, 2017
ca4aaae
Fix initial-shift selection
dy Nov 3, 2017
39a4dc4
Add multiselection test
dy Nov 6, 2017
ab92fd2
Fix review comments
dy Nov 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
"ndarray-fill": "^1.0.2",
"ndarray-homography": "^1.0.0",
"ndarray-ops": "^1.2.2",
"polybooljs": "^1.2.0",
"regl": "^1.3.0",
"right-now": "^1.0.0",
"robust-orientation": "^1.1.3",
Expand Down
58 changes: 55 additions & 3 deletions src/lib/polygon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,17 @@ var polygon = module.exports = {};
* returns boolean: is pt inside the polygon (including on its edges)
*/
polygon.tester = function tester(ptsIn) {
if(Array.isArray(ptsIn[0][0])) return polygon.multitester(ptsIn);

var pts = ptsIn.slice(),
xmin = pts[0][0],
xmax = xmin,
ymin = pts[0][1],
ymax = ymin;
ymax = ymin,
i;

pts.push(pts[0]);
for(var i = 1; i < pts.length; i++) {
for(i = 1; i < pts.length; i++) {
xmin = Math.min(xmin, pts[i][0]);
xmax = Math.max(xmax, pts[i][0]);
ymin = Math.min(ymin, pts[i][1]);
Expand Down Expand Up @@ -149,14 +152,63 @@ polygon.tester = function tester(ptsIn) {
return crossings % 2 === 1;
}

// detect if poly is degenerate
var degenerate = true;
var lastPt = pts[0];
for(i = 1; i < pts.length; i++) {
if(lastPt[0] !== pts[i][0] || lastPt[1] !== pts[i][1]) {
degenerate = false;
break;
}
}

return {
xmin: xmin,
xmax: xmax,
ymin: ymin,
ymax: ymax,
pts: pts,
contains: isRect ? rectContains : contains,
isRect: isRect
isRect: isRect,
degenerate: degenerate
};
};

/**
* Test multiple polygons
*/
polygon.multitester = function multitester(list) {
var testers = [],
xmin = list[0][0][0],
xmax = xmin,
ymin = list[0][0][1],
ymax = ymin;

for(var i = 0; i < list.length; i++) {
var tester = polygon.tester(list[i]);
testers.push(tester);
xmin = Math.min(xmin, tester.xmin);
xmax = Math.max(xmax, tester.xmax);
ymin = Math.min(ymin, tester.ymin);
ymax = Math.max(ymax, tester.ymax);
}

function contains(pt, arg) {
for(var i = 0; i < testers.length; i++) {
if(testers[i].contains(pt, arg)) return true;
}
return false;
}

return {
xmin: xmin,
xmax: xmax,
ymin: ymin,
ymax: ymax,
pts: [],
contains: contains,
isRect: false,
degenerate: false
};
};

Expand Down
55 changes: 51 additions & 4 deletions src/plots/cartesian/dragbox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,10 @@ module.exports = function dragBox(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 dragModeNow = 'pan';
else if(!isSelectOrLasso(dragModeNow)) dragModeNow = 'pan';
}
else if(e.ctrlKey) {
dragModeNow = 'pan';
}
}
// all other draggers just pan
Expand Down Expand Up @@ -168,13 +171,31 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) {
else if(isSelectOrLasso(dragModeNow)) {
dragOptions.xaxes = xa;
dragOptions.yaxes = ya;

// take over selection polygons from prev mode, if any
if(e.shiftKey && plotinfo.selection.polygons && !dragOptions.polygons) {
dragOptions.polygons = plotinfo.selection.polygons;
dragOptions.mergedPolygons = plotinfo.selection.mergedPolygons;
}
// create new polygons, if shift mode
else if(!e.shiftKey || (e.shiftKey && !plotinfo.selection.polygons)) {
plotinfo.selection = {};
plotinfo.selection.polygons = dragOptions.polygons = [];
dragOptions.mergedPolygons = plotinfo.selection.mergedPolygons = [];
}

prepSelect(e, startX, startY, dragOptions, dragModeNow);
}
}
};

dragElement.init(dragOptions);

// FIXME: this hack highlights selection once we enter select/lasso mode
if(isSelectOrLasso(gd._fullLayout.dragmode) && plotinfo.selection) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a big fan of this behavior. @dfcreative is there a particular reason why you added this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not relevant since #2135

showSelect(zoomlayer, dragOptions);
}

var x0,
y0,
box,
Expand Down Expand Up @@ -526,6 +547,9 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) {
}

updateSubplots([x0, y0, pw - dx, ph - dy]);

if(plotinfo.ondrag) plotinfo.ondrag.call([x0, y0, pw - dx, ph - dy]);
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed


ticksAndAnnotations(yActive, xActive);
}

Expand Down Expand Up @@ -902,6 +926,31 @@ function clearSelect(zoomlayer) {
zoomlayer.selectAll('.select-outline').remove();
}

function showSelect(zoomlayer, dragOptions) {
var outlines = zoomlayer.selectAll('path.select-outline').data([1, 2]),
plotinfo = dragOptions.plotinfo,
xaxis = plotinfo.xaxis,
yaxis = plotinfo.yaxis,
selection = plotinfo.selection,
polygons = selection.mergedPolygons,
xs = xaxis._offset,
ys = yaxis._offset,
paths = [];

for(var i = 0; i < polygons.length; i++) {
var ppts = polygons[i];
paths.push(ppts.join('L') + 'L' + ppts[0]);
}

if(paths.length) {
outlines.enter()
.append('path')
.attr('class', function(d) { return 'select-outline select-outline-' + d; })
.attr('transform', 'translate(' + xs + ', ' + ys + ')')
.attr('d', 'M' + paths.join('M') + 'Z');
}
}

function updateZoombox(zb, corners, box, path0, dimmed, lum) {
zb.attr('d',
path0 + 'M' + (box.l) + ',' + (box.t) + 'v' + (box.h) +
Expand All @@ -924,9 +973,7 @@ function removeZoombox(gd) {
}

function isSelectOrLasso(dragmode) {
var modes = ['lasso', 'select'];

return modes.indexOf(dragmode) !== -1;
return dragmode === 'lasso' || dragmode === 'select';
Copy link
Contributor

Choose a reason for hiding this comment

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

Way better 🐎 . Thanks!

}

function xCorners(box, y0) {
Expand Down
96 changes: 75 additions & 21 deletions src/plots/cartesian/select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

'use strict';

var polybool = require('polybooljs');
var polygon = require('../../lib/polygon');
var throttle = require('../../lib/throttle');
var color = require('../../components/color');
Expand All @@ -19,6 +20,7 @@ var constants = require('./constants');

var filteredPolygon = polygon.filter;
var polygonTester = polygon.tester;
var multipolygonTester = polygon.multitester;
var MINSELECT = constants.MINSELECT;

function getAxId(ax) { return ax._id; }
Expand All @@ -39,10 +41,10 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) {
xAxisIds = dragOptions.xaxes.map(getAxId),
yAxisIds = dragOptions.yaxes.map(getAxId),
allAxes = dragOptions.xaxes.concat(dragOptions.yaxes),
pts;
filterPoly, testPoly, mergedPolygons, currentPolygon;

if(mode === 'lasso') {
pts = filteredPolygon([[x0, y0]], constants.BENDPX);
filterPoly = filteredPolygon([[x0, y0]], constants.BENDPX);
}

var outlines = zoomLayer.selectAll('path.select-outline').data([1, 2]);
Expand Down Expand Up @@ -129,20 +131,18 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) {
}
};
} else {
fillRangeItems = function(eventData, poly, pts) {
fillRangeItems = function(eventData, currentPolygon, filterPoly) {
var dataPts = eventData.lassoPoints = {};

for(i = 0; i < allAxes.length; i++) {
var ax = allAxes[i];
dataPts[ax._id] = pts.filtered.map(axValue(ax));
dataPts[ax._id] = filterPoly.filtered.map(axValue(ax));
}
};
}
}

dragOptions.moveFn = function(dx0, dy0) {
var poly;

x1 = Math.max(0, Math.min(pw, dx0 + x0));
y1 = Math.max(0, Math.min(ph, dy0 + y0));

Expand All @@ -152,46 +152,78 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) {
if(mode === 'select') {
if(dy < Math.min(dx * 0.6, MINSELECT)) {
// horizontal motion: make a vertical box
poly = polygonTester([[x0, 0], [x0, ph], [x1, ph], [x1, 0]]);
currentPolygon = [[x0, 0], [x0, ph], [x1, ph], [x1, 0]];
currentPolygon.xmin = Math.min(x0, x1);
currentPolygon.xmax = Math.max(x0, x1);
currentPolygon.ymin = Math.min(0, ph);
currentPolygon.ymax = Math.max(0, ph);
// extras to guide users in keeping a straight selection
corners.attr('d', 'M' + poly.xmin + ',' + (y0 - MINSELECT) +
corners.attr('d', 'M' + currentPolygon.xmin + ',' + (y0 - MINSELECT) +
'h-4v' + (2 * MINSELECT) + 'h4Z' +
'M' + (poly.xmax - 1) + ',' + (y0 - MINSELECT) +
'M' + (currentPolygon.xmax - 1) + ',' + (y0 - MINSELECT) +
'h4v' + (2 * MINSELECT) + 'h-4Z');

}
else if(dx < Math.min(dy * 0.6, MINSELECT)) {
// vertical motion: make a horizontal box
poly = polygonTester([[0, y0], [0, y1], [pw, y1], [pw, y0]]);
corners.attr('d', 'M' + (x0 - MINSELECT) + ',' + poly.ymin +
currentPolygon = [[0, y0], [0, y1], [pw, y1], [pw, y0]];
currentPolygon.xmin = Math.min(0, pw);
currentPolygon.xmax = Math.max(0, pw);
currentPolygon.ymin = Math.min(y0, y1);
currentPolygon.ymax = Math.max(y0, y1);
corners.attr('d', 'M' + (x0 - MINSELECT) + ',' + currentPolygon.ymin +
'v-4h' + (2 * MINSELECT) + 'v4Z' +
'M' + (x0 - MINSELECT) + ',' + (poly.ymax - 1) +
'M' + (x0 - MINSELECT) + ',' + (currentPolygon.ymax - 1) +
'v4h' + (2 * MINSELECT) + 'v-4Z');
}
else {
// diagonal motion
poly = polygonTester([[x0, y0], [x0, y1], [x1, y1], [x1, y0]]);
currentPolygon = [[x0, y0], [x0, y1], [x1, y1], [x1, y0]];
currentPolygon.xmin = Math.min(x0, x1);
currentPolygon.xmax = Math.max(x0, x1);
currentPolygon.ymin = Math.min(y0, y1);
currentPolygon.ymax = Math.max(y0, y1);
corners.attr('d', 'M0,0Z');
}
outlines.attr('d', 'M' + poly.xmin + ',' + poly.ymin +
'H' + (poly.xmax - 1) + 'V' + (poly.ymax - 1) +
'H' + poly.xmin + 'Z');
}
else if(mode === 'lasso') {
pts.addPt([x1, y1]);
poly = polygonTester(pts.filtered);
outlines.attr('d', 'M' + pts.filtered.join('L') + 'Z');
filterPoly.addPt([x1, y1]);
currentPolygon = filterPoly.filtered;
}

// create outline & tester
if(dragOptions.polygons && dragOptions.polygons.length) {
mergedPolygons = joinPolygons(dragOptions.mergedPolygons, currentPolygon);
testPoly = multipolygonTester(dragOptions.polygons.concat([currentPolygon]));
}
else {
mergedPolygons = [currentPolygon];
testPoly = polygonTester(currentPolygon);
}

// draw selection
var paths = [];
for(i = 0; i < mergedPolygons.length; i++) {
var ppts = mergedPolygons[i];
paths.push(ppts.join('L') + 'L' + ppts[0]);
}
outlines.attr('d', 'M' + paths.join('M') + 'Z');

throttle.throttle(
throttleID,
constants.SELECTDELAY,
function() {
selection = [];

var traceSelections = [], traceSelection;
for(i = 0; i < searchTraces.length; i++) {
searchInfo = searchTraces[i];

traceSelection = searchInfo.selectPoints(searchInfo, testPoly);
traceSelections.push(traceSelection);

var thisSelection = fillSelectionItem(
searchInfo.selectPoints(searchInfo, poly), searchInfo
traceSelection, searchInfo
);
if(selection.length) {
for(var j = 0; j < thisSelection.length; j++) {
Expand All @@ -202,14 +234,15 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) {
}

eventData = {points: selection};
fillRangeItems(eventData, poly, pts);
fillRangeItems(eventData, currentPolygon, filterPoly);
dragOptions.gd.emit('plotly_selecting', eventData);
}
);
};

dragOptions.doneFn = function(dragged, numclicks) {
corners.remove();

throttle.done(throttleID).then(function() {
throttle.clear(throttleID);

Expand All @@ -226,10 +259,31 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) {
else {
dragOptions.gd.emit('plotly_selected', eventData);
}

if(currentPolygon && dragOptions.polygons) {
// save last polygons
dragOptions.polygons.push(currentPolygon);

// we have to keep reference to arrays container
dragOptions.mergedPolygons.length = 0;
[].push.apply(dragOptions.mergedPolygons, mergedPolygons);
}
});
};
};

function joinPolygons(list, poly) {
Copy link
Contributor

@etpinard etpinard Nov 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Joining the polygons sounds very reasonable to me. But, some users might expect that selections within selections would remove the inner polygons. Perhaps we'll need to add layout options (e.g. layout.multiselectionmode: 'join' | 'cut').

Any thoughts about this @alexcjohnson @chriddyp @cldougl @monfera ?

Copy link
Member

@cldougl cldougl Nov 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

so 'join' mode would always add regions to the selection (and do nothing if a selected region is selected a second time) and 'cut' mode would always remove regions from the selection (and do nothing if the region was not previously selected)?

Copy link
Member

Choose a reason for hiding this comment

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

+1 for a cut mode (after seeing the join in action)

Copy link
Collaborator

Choose a reason for hiding this comment

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

My gut reaction: if you start a new selection inside an existing one, it's purely subtractive, if you start outside the existing ones it's purely additive. Would have to play with it implemented to see if this really feels intuitive though...

Copy link
Contributor Author

@dy dy Nov 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Ok, added subtract selection by holding Alt in e8c18f0

Copy link
Contributor

Choose a reason for hiding this comment

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

My gut reaction: if you start a new selection inside an existing one, it's purely subtractive, if you start outside the existing ones it's purely additive.

+1

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, added subtract selection by holding Alt in e8c18f0

That is more powerful (as well as easier to code 😅 ), my only concern is whether people will be able to find it. Though I guess if they can find add they can find subtract...

(and I never saw <kbd> before, nice! 🎉 )

Copy link
Contributor

@etpinard etpinard Nov 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Nice solution @dfcreative . The only (potential) problem I can think of is mobile and tablets users complaining no having access to Shift and Alt keys and by consequent multi-selections.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@etpinard good point, we can use @alexcjohnson strategy for that. But I'd wait for persistent selections PR merged first

var res = polybool.union({
regions: list,
inverted: false
}, {
regions: [poly],
inverted: false
});

return res.regions;
}

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