Skip to content

Commit 3a5a4c2

Browse files
committed
treat trace w/ _length===0 as if they were visible:false
... after transform _module.calc loop - that way filter transforms that remove all data coordinates don't result in errors - of all the mocks listed in mock_lists.js, only 'scattercarpet' and 'world-cals' still error out (wip)
1 parent 1e57928 commit 3a5a4c2

File tree

9 files changed

+83
-10
lines changed

9 files changed

+83
-10
lines changed

src/components/fx/hover.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -370,10 +370,12 @@ function _hover(gd, evt, subplot, noHoverEvent) {
370370
cd = searchData[curvenum];
371371

372372
// filter out invisible or broken data
373-
if(!cd || !cd[0] || !cd[0].trace || cd[0].trace.visible !== true) continue;
373+
if(!cd || !cd[0] || !cd[0].trace) continue;
374374

375375
trace = cd[0].trace;
376376

377+
if(trace.visible !== true || trace._length === 0) continue;
378+
377379
// Explicitly bail out for these two. I don't know how to otherwise prevent
378380
// the rest of this function from running and failing
379381
if(['carpet', 'contourcarpet'].indexOf(trace._module.name) !== -1) continue;

src/lib/filter_visible.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ function baseFilter(item) {
3232
}
3333

3434
function calcDataFilter(item) {
35-
return item[0].trace.visible === true;
35+
var trace = item[0].trace;
36+
return trace.visible === true && trace._length !== 0;
3637
}
3738

3839
function isCalcData(cont) {

src/plot_api/subroutines.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -674,7 +674,7 @@ exports.redrawReglTraces = function(gd) {
674674
for(i = 0; i < fullData.length; i++) {
675675
var trace = fullData[i];
676676

677-
if(trace.visible === true) {
677+
if(trace.visible === true && trace._length !== 0) {
678678
if(trace.type === 'splom') {
679679
fullLayout._splomScenes[trace.uid].draw();
680680
} else if(trace.type === 'scattergl') {

src/plots/get_data.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,10 @@ exports.getModuleCalcData = function(calcdata, arg1) {
6969
for(var i = 0; i < calcdata.length; i++) {
7070
var cd = calcdata[i];
7171
var trace = cd[0].trace;
72-
// N.B. 'legendonly' traces do not make it past here
73-
if(trace.visible !== true) continue;
72+
// N.B.
73+
// - 'legendonly' traces do not make it past here
74+
// - skip over 'visible' traces that got trimmed completely during calc transforms
75+
if(trace.visible !== true || trace._length === 0) continue;
7476

7577
// group calcdata trace not by 'module' (as the name of this function
7678
// would suggest), but by 'module plot method' so that if some traces

src/plots/gl3d/scene.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ proto.plot = function(sceneData, fullLayout, layout) {
502502

503503
for(i = 0; i < sceneData.length; ++i) {
504504
data = sceneData[i];
505-
if(data.visible !== true) continue;
505+
if(data.visible !== true || data._length === 0) continue;
506506

507507
computeTraceBounds(this, data, dataBounds);
508508
}
@@ -526,7 +526,7 @@ proto.plot = function(sceneData, fullLayout, layout) {
526526
// Update traces
527527
for(i = 0; i < sceneData.length; ++i) {
528528
data = sceneData[i];
529-
if(data.visible !== true) {
529+
if(data.visible !== true || data._length === 0) {
530530
continue;
531531
}
532532
trace = this.traces[data.uid];
@@ -551,7 +551,8 @@ proto.plot = function(sceneData, fullLayout, layout) {
551551
traceIdLoop:
552552
for(i = 0; i < traceIds.length; ++i) {
553553
for(j = 0; j < sceneData.length; ++j) {
554-
if(sceneData[j].uid === traceIds[i] && sceneData[j].visible === true) {
554+
if(sceneData[j].uid === traceIds[i] &&
555+
(sceneData[j].visible === true && sceneData[j]._length !== 0)) {
555556
continue traceIdLoop;
556557
}
557558
}

src/plots/plots.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -2778,7 +2778,7 @@ plots.doCalcdata = function(gd, traces) {
27782778

27792779
var cd = [];
27802780

2781-
if(trace.visible === true) {
2781+
if(trace.visible === true && trace._length !== 0) {
27822782
// clear existing ref in case it got relinked
27832783
delete trace._indexToPoints;
27842784
// keep ref of index-to-points map object of the *last* enabled transform,

src/plots/polar/set_convert.js

+3
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,9 @@ function setConvertAngular(ax, polarLayout) {
174174
var catLen = ax._categories.length;
175175
var _period = ax.period ? Math.max(ax.period, catLen) : catLen;
176176

177+
// fallback in case all categories have been filtered out
178+
if(_period === 0) _period = 1;
179+
177180
c2rad = t2rad = function(v) { return v * 2 * Math.PI / _period; };
178181
rad2c = rad2t = function(v) { return v * _period / Math.PI / 2; };
179182

src/traces/scattermapbox/convert.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ var convertTextOpts = require('../../plots/mapbox/convert_text_opts');
2323
module.exports = function convert(calcTrace) {
2424
var trace = calcTrace[0].trace;
2525

26-
var isVisible = (trace.visible === true);
26+
var isVisible = (trace.visible === true && trace._length !== 0);
2727
var hasFill = (trace.fill !== 'none');
2828
var hasLines = subTypes.hasLines(trace);
2929
var hasMarkers = subTypes.hasMarkers(trace);

test/jasmine/tests/transform_filter_test.js

+64
Original file line numberDiff line numberDiff line change
@@ -1319,3 +1319,67 @@ describe('filter transforms interactions', function() {
13191319
.then(done);
13201320
});
13211321
});
1322+
1323+
describe('filter resulting in empty coordinate arrays', function() {
1324+
afterEach(destroyGraphDiv);
1325+
1326+
function filter2empty(mock) {
1327+
var fig = Lib.extendDeep({}, mock);
1328+
var data = fig.data || [];
1329+
1330+
data.forEach(function(trace) {
1331+
trace.transforms = [{
1332+
type: 'filter',
1333+
target: [null]
1334+
}];
1335+
});
1336+
1337+
return fig;
1338+
}
1339+
1340+
describe('svg mocks', function() {
1341+
var mockList = require('../assets/mock_lists').svg;
1342+
1343+
mockList.forEach(function(d) {
1344+
if(d[0] === 'scattercarpet' || d[0] === 'world-cals') {
1345+
// scattercarpet don't work with transforms
1346+
// world-cals mock complains during a Lib.cleanDate()
1347+
return;
1348+
}
1349+
1350+
it(d[0], function(done) {
1351+
var gd = createGraphDiv();
1352+
var fig = filter2empty(d[1]);
1353+
Plotly.newPlot(gd, fig).catch(failTest).then(done);
1354+
});
1355+
});
1356+
});
1357+
1358+
describe('gl mocks', function() {
1359+
var mockList = require('../assets/mock_lists').gl;
1360+
1361+
mockList.forEach(function(d) {
1362+
it('@gl ' + d[0], function(done) {
1363+
var gd = createGraphDiv();
1364+
var fig = filter2empty(d[1]);
1365+
Plotly.newPlot(gd, fig).catch(failTest).then(done);
1366+
});
1367+
});
1368+
});
1369+
1370+
describe('mapbox mocks', function() {
1371+
var mockList = require('../assets/mock_lists').mapbox;
1372+
1373+
Plotly.setPlotConfig({
1374+
mapboxAccessToken: require('@build/credentials.json').MAPBOX_ACCESS_TOKEN
1375+
});
1376+
1377+
mockList.forEach(function(d) {
1378+
it('@noCI ' + d[0], function(done) {
1379+
var gd = createGraphDiv();
1380+
var fig = filter2empty(d[1]);
1381+
Plotly.newPlot(gd, fig).catch(failTest).then(done);
1382+
});
1383+
});
1384+
});
1385+
});

0 commit comments

Comments
 (0)