Skip to content

Commit

Permalink
fix #2271 - ghost fill tonext when either trace is emptied out
Browse files Browse the repository at this point in the history
  • Loading branch information
alexcjohnson committed Jan 23, 2018
1 parent e701b5e commit fb8357a
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 21 deletions.
7 changes: 3 additions & 4 deletions src/traces/scatter/link_traces.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@
'use strict';

module.exports = function linkTraces(gd, plotinfo, cdscatter) {
var cd, trace;
var trace, i;
var prevtrace = null;

for(var i = 0; i < cdscatter.length; ++i) {
cd = cdscatter[i];
trace = cd[0].trace;
for(i = 0; i < cdscatter.length; ++i) {
trace = cdscatter[i][0].trace;

// Note: The check which ensures all cdscatter here are for the same axis and
// are either cartesian or scatterternary has been removed. This code assumes
Expand Down
49 changes: 32 additions & 17 deletions src/traces/scatter/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,10 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition

Drawing.setClipUrl(lineJoin, plotinfo.layerClipId);

function clearFill(selection) {
transition(selection).attr('d', 'M0,0Z');
}

if(segments.length) {
if(ownFillEl3) {
if(pt0 && pt1) {
Expand All @@ -348,30 +352,41 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
}
}
}
else if(trace.fill.substr(0, 6) === 'tonext' && fullpath && prevRevpath) {
// fill to next: full trace path, plus the previous path reversed
if(trace.fill === 'tonext') {
// tonext: for use by concentric shapes, like manually constructed
// contours, we just add the two paths closed on themselves.
// This makes strange results if one path is *not* entirely
// inside the other, but then that is a strange usage.
transition(tonext).attr('d', fullpath + 'Z' + prevRevpath + 'Z')
.call(Drawing.singleFillStyle);
else if(tonext) {
if(trace.fill.substr(0, 6) === 'tonext' && fullpath && prevRevpath) {
// fill to next: full trace path, plus the previous path reversed
if(trace.fill === 'tonext') {
// tonext: for use by concentric shapes, like manually constructed
// contours, we just add the two paths closed on themselves.
// This makes strange results if one path is *not* entirely
// inside the other, but then that is a strange usage.
transition(tonext).attr('d', fullpath + 'Z' + prevRevpath + 'Z')
.call(Drawing.singleFillStyle);
}
else {
// tonextx/y: for now just connect endpoints with lines. This is
// the correct behavior if the endpoints are at the same value of
// y/x, but if they *aren't*, we should ideally do more complicated
// things depending on whether the new endpoint projects onto the
// existing curve or off the end of it
transition(tonext).attr('d', fullpath + 'L' + prevRevpath.substr(1) + 'Z')
.call(Drawing.singleFillStyle);
}
trace._polygons = trace._polygons.concat(prevPolygons);
}
else {
// tonextx/y: for now just connect endpoints with lines. This is
// the correct behavior if the endpoints are at the same value of
// y/x, but if they *aren't*, we should ideally do more complicated
// things depending on whether the new endpoint projects onto the
// existing curve or off the end of it
transition(tonext).attr('d', fullpath + 'L' + prevRevpath.substr(1) + 'Z')
.call(Drawing.singleFillStyle);
clearFill(tonext);
trace._polygons = null;
}
trace._polygons = trace._polygons.concat(prevPolygons);
}
trace._prevRevpath = revpath;
trace._prevPolygons = thisPolygons;
}
else {
if(ownFillEl3) clearFill(ownFillEl3);
else if(tonext) clearFill(tonext);
trace._polygons = trace._prevRevpath = trace._prevPolygons = null;
}


function visFilter(d) {
Expand Down
44 changes: 44 additions & 0 deletions test/jasmine/tests/scatter_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,50 @@ describe('end-to-end scatter tests', function() {
expect(fill()).toEqual('rgb(0, 0, 255)');
}).catch(fail).then(done);
});

it('clears fills tonext when either trace is emptied out', function(done) {
var trace0 = {y: [1, 3, 5, 7]};
var trace1 = {y: [1, 2, 3, 4], line: {width: 0}, mode: 'lines'};
var trace2 = {y: [3, 4, 5, 6], fill: 'tonexty', mode: 'none'};

function checkFill(visible, msg) {
var fillSelection = d3.select(gd).selectAll('.scatterlayer .js-fill');
expect(fillSelection.size()).toBe(1, msg);
expect(fillSelection.attr('d')).negateIf(visible).toBe('M0,0Z', msg);
}

Plotly.newPlot(gd, [trace0, trace1, trace2], {}, {scrollZoom: true})
.then(function() {
checkFill(true, 'initial');

return Plotly.restyle(gd, {y: [[null, null, null, null]]}, [1]);
})
.then(function() {
checkFill(false, 'null out trace 1');

return Plotly.restyle(gd, {y: [[1, 2, 3, 4]]}, [1]);
})
.then(function() {
checkFill(true, 'restore trace 1');

return Plotly.restyle(gd, {y: [[null, null, null, null]]}, [2]);
})
.then(function() {
checkFill(false, 'null out trace 2');

return Plotly.restyle(gd, {y: [[1, 2, 3, 4]]}, [2]);
})
.then(function() {
checkFill(true, 'restore trace 2');

return Plotly.restyle(gd, {y: [[null, null, null, null], [null, null, null, null]]}, [1, 2]);
})
.then(function() {
checkFill(false, 'null out both traces');
})
.catch(fail)
.then(done);
});
});

describe('scatter hoverPoints', function() {
Expand Down

0 comments on commit fb8357a

Please sign in to comment.