From 6d7803685e49de62c359703a0820ebd65c8dcbf6 Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Wed, 18 Oct 2017 16:42:28 +0200 Subject: [PATCH 1/5] handle trace with no points, resulting in invisible trace and node3 undefined --- src/traces/choropleth/select.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/traces/choropleth/select.js b/src/traces/choropleth/select.js index 209b1ba35a7..cffc3933380 100644 --- a/src/traces/choropleth/select.js +++ b/src/traces/choropleth/select.js @@ -46,9 +46,11 @@ module.exports = function selectPoints(searchInfo, polygon) { } } - node3.selectAll('path').style('opacity', function(d) { - return d.dim ? DESELECTDIM : 1; - }); + if(node3) { + node3.selectAll('path').style('opacity', function (d) { + return d.dim ? DESELECTDIM : 1; + }); + } return selection; }; From 314fbe1b8d6ed8e44c09c8036ac74070806283e6 Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Wed, 18 Oct 2017 18:23:41 +0200 Subject: [PATCH 2/5] test: handle trace with no points, resulting in invisible trace and node3 undefined --- src/traces/choropleth/select.js | 2 +- test/jasmine/tests/select_test.js | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/traces/choropleth/select.js b/src/traces/choropleth/select.js index cffc3933380..661e2c79549 100644 --- a/src/traces/choropleth/select.js +++ b/src/traces/choropleth/select.js @@ -47,7 +47,7 @@ module.exports = function selectPoints(searchInfo, polygon) { } if(node3) { - node3.selectAll('path').style('opacity', function (d) { + node3.selectAll('path').style('opacity', function(d) { return d.dim ? DESELECTDIM : 1; }); } diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index 00301826243..6affd0b3741 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -687,6 +687,12 @@ describe('Test select box and lasso per trace:', function() { fig.layout.dragmode = 'select'; fig.layout.geo.scope = 'europe'; + // add a trace with no locations which will then make trace invisible, lacking DOM elements + fig.data.push(Lib.extendDeep({}, fig.data[0])); + fig.data[1].text = []; + fig.data[1].locations = []; + fig.data[1].z = []; + Plotly.plot(gd, fig) .then(function() { return _run( From 8cb0c1adcf15f89bb01221097c0ec99f5120c321 Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Wed, 18 Oct 2017 21:01:06 +0200 Subject: [PATCH 3/5] more upstream fix for avoiding invisible traces --- src/plots/cartesian/select.js | 7 +++++++ src/traces/choropleth/select.js | 8 +++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js index 70952c023dc..4c5f12b8faf 100644 --- a/src/plots/cartesian/select.js +++ b/src/plots/cartesian/select.js @@ -23,6 +23,11 @@ var MINSELECT = constants.MINSELECT; function getAxId(ax) { return ax._id; } +function visible(searchInfo) { + var cd0 = searchInfo.cd[0]; + return cd0 && cd0.trace && cd0.trace.visible === true; +} + module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { var zoomLayer = dragOptions.gd._fullLayout._zoomlayer, dragBBox = dragOptions.element.getBoundingClientRect(), @@ -190,6 +195,7 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { selection = []; for(i = 0; i < searchTraces.length; i++) { searchInfo = searchTraces[i]; + if(!visible(searchInfo)) continue; var thisSelection = fillSelectionItem( searchInfo.selectPoints(searchInfo, poly), searchInfo ); @@ -218,6 +224,7 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { outlines.remove(); for(i = 0; i < searchTraces.length; i++) { searchInfo = searchTraces[i]; + if(!visible(searchInfo)) continue; searchInfo.selectPoints(searchInfo, false); } diff --git a/src/traces/choropleth/select.js b/src/traces/choropleth/select.js index 661e2c79549..209b1ba35a7 100644 --- a/src/traces/choropleth/select.js +++ b/src/traces/choropleth/select.js @@ -46,11 +46,9 @@ module.exports = function selectPoints(searchInfo, polygon) { } } - if(node3) { - node3.selectAll('path').style('opacity', function(d) { - return d.dim ? DESELECTDIM : 1; - }); - } + node3.selectAll('path').style('opacity', function(d) { + return d.dim ? DESELECTDIM : 1; + }); return selection; }; From b9817ad1cb4a6e26f3baac129423d09c24c0ec57 Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Thu, 19 Oct 2017 00:42:38 +0200 Subject: [PATCH 4/5] discontinuing per-trace type visibility check --- src/traces/bar/select.js | 3 --- src/traces/scatter/select.js | 2 +- src/traces/scattergeo/select.js | 2 +- src/traces/scattergl/select.js | 2 +- src/traces/scattermapbox/select.js | 2 +- 5 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/traces/bar/select.js b/src/traces/bar/select.js index 309016c72be..37f434138dc 100644 --- a/src/traces/bar/select.js +++ b/src/traces/bar/select.js @@ -13,12 +13,9 @@ var DESELECTDIM = require('../../constants/interactions').DESELECTDIM; module.exports = function selectPoints(searchInfo, polygon) { var cd = searchInfo.cd; var selection = []; - var trace = cd[0].trace; var node3 = cd[0].node3; var i; - if(trace.visible !== true) return []; - if(polygon === false) { // clear selection for(i = 0; i < cd.length; i++) { diff --git a/src/traces/scatter/select.js b/src/traces/scatter/select.js index a529a4c21e6..96cd76fe2d8 100644 --- a/src/traces/scatter/select.js +++ b/src/traces/scatter/select.js @@ -26,7 +26,7 @@ module.exports = function selectPoints(searchInfo, polygon) { // TODO: include lines? that would require per-segment line properties var hasOnlyLines = (!subtypes.hasMarkers(trace) && !subtypes.hasText(trace)); - if(trace.visible !== true || hasOnlyLines) return []; + if(hasOnlyLines) return []; var opacity = Array.isArray(marker.opacity) ? 1 : marker.opacity; diff --git a/src/traces/scattergeo/select.js b/src/traces/scattergeo/select.js index 8cdfc187ef7..aeacd646423 100644 --- a/src/traces/scattergeo/select.js +++ b/src/traces/scattergeo/select.js @@ -22,7 +22,7 @@ module.exports = function selectPoints(searchInfo, polygon) { var di, lonlat, x, y, i; var hasOnlyLines = (!subtypes.hasMarkers(trace) && !subtypes.hasText(trace)); - if(trace.visible !== true || hasOnlyLines) return []; + if(hasOnlyLines) return []; var marker = trace.marker; var opacity = Array.isArray(marker.opacity) ? 1 : marker.opacity; diff --git a/src/traces/scattergl/select.js b/src/traces/scattergl/select.js index e4187fb9eda..498a99ba287 100644 --- a/src/traces/scattergl/select.js +++ b/src/traces/scattergl/select.js @@ -26,7 +26,7 @@ module.exports = function selectPoints(searchInfo, polygon) { var scene = glTrace.scene; var hasOnlyLines = (!subtypes.hasMarkers(trace) && !subtypes.hasText(trace)); - if(trace.visible !== true || hasOnlyLines) return []; + if(hasOnlyLines) return []; // filter out points by visible scatter ones if(polygon === false) { diff --git a/src/traces/scattermapbox/select.js b/src/traces/scattermapbox/select.js index 7e464ae17fd..35eaf523e9a 100644 --- a/src/traces/scattermapbox/select.js +++ b/src/traces/scattermapbox/select.js @@ -24,7 +24,7 @@ module.exports = function selectPoints(searchInfo, polygon) { // to not insert data-driven 'circle-opacity' when we don't need to trace._hasDimmedPts = false; - if(trace.visible !== true || !subtypes.hasMarkers(trace)) return []; + if(!subtypes.hasMarkers(trace)) return []; if(polygon === false) { for(i = 0; i < cd.length; i++) { From fc0ad56dc5a8d898b9602830752838e059e7c9b3 Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Thu, 19 Oct 2017 00:58:17 +0200 Subject: [PATCH 5/5] moving check more upstream --- src/plots/cartesian/select.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js index 4c5f12b8faf..98afbcd5e84 100644 --- a/src/plots/cartesian/select.js +++ b/src/plots/cartesian/select.js @@ -23,11 +23,6 @@ var MINSELECT = constants.MINSELECT; function getAxId(ax) { return ax._id; } -function visible(searchInfo) { - var cd0 = searchInfo.cd[0]; - return cd0 && cd0.trace && cd0.trace.visible === true; -} - module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { var zoomLayer = dragOptions.gd._fullLayout._zoomlayer, dragBBox = dragOptions.element.getBoundingClientRect(), @@ -79,7 +74,7 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { for(i = 0; i < gd.calcdata.length; i++) { cd = gd.calcdata[i]; trace = cd[0].trace; - if(!trace._module || !trace._module.selectPoints) continue; + if(trace.visible !== true || !trace._module || !trace._module.selectPoints) continue; if(dragOptions.subplot) { if( @@ -195,7 +190,6 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { selection = []; for(i = 0; i < searchTraces.length; i++) { searchInfo = searchTraces[i]; - if(!visible(searchInfo)) continue; var thisSelection = fillSelectionItem( searchInfo.selectPoints(searchInfo, poly), searchInfo ); @@ -224,7 +218,6 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { outlines.remove(); for(i = 0; i < searchTraces.length; i++) { searchInfo = searchTraces[i]; - if(!visible(searchInfo)) continue; searchInfo.selectPoints(searchInfo, false); }