From 310e67a56c35bd559ee5088c54a2d4b78600d24b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 11 May 2017 10:55:47 -0400 Subject: [PATCH 1/4] add test case for point / text node removal on blank data --- test/jasmine/tests/scatter_test.js | 54 ++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index 735f8ca6114..5c9aa1161eb 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/test/jasmine/tests/scatter_test.js @@ -383,6 +383,60 @@ describe('end-to-end scatter tests', function() { expect(Plotly.d3.selectAll('.textpoint').size()).toBe(3); }).catch(fail).then(done); }); + + it('should remove all point and text nodes on blank data', function(done) { + function assertNodeCnt(ptCnt, txCnt) { + expect(d3.selectAll('.point').size()).toEqual(ptCnt); + expect(d3.selectAll('.textpoint').size()).toEqual(txCnt); + } + + function assertText(content) { + d3.selectAll('.textpoint').each(function(_, i) { + var tx = d3.select(this).select('text'); + expect(tx.text()).toEqual(content[i]); + }); + } + + Plotly.plot(gd, [{ + x: [150, 350, 650], + y: [100, 300, 600], + text: ['A', 'B', 'C'], + mode: 'markers+text', + marker: { + size: [100, 200, 300], + line: { width: [10, 20, 30] }, + color: 'yellow', + sizeref: 3, + gradient: { + type: 'radial', + color: 'white' + } + } + }]) + .then(function() { + assertNodeCnt(3, 3); + assertText(['A', 'B', 'C']); + + return Plotly.restyle(gd, { + x: [[null, undefined, NaN]], + y: [[NaN, null, undefined]] + }); + }) + .then(function() { + assertNodeCnt(0, 0); + + return Plotly.restyle(gd, { + x: [[150, 350, 650]], + y: [[100, 300, 600]] + }); + }) + .then(function() { + assertNodeCnt(3, 3); + assertText(['A', 'B', 'C']); + }) + .catch(fail) + .then(done); + }); }); describe('scatter hoverPoints', function() { From 68959b3b24cc168e15d0410e661a304b19efedae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 11 May 2017 10:57:15 -0400 Subject: [PATCH 2/4] add return boolean to translatePoint - return false when sel is removed i.e. when its x/y coords are non-numeric - this is helpful for clearing associated - return true otherwise --- src/components/drawing/index.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index a1e4eb011be..462abb15428 100644 --- a/src/components/drawing/index.js +++ b/src/components/drawing/index.js @@ -47,6 +47,17 @@ drawing.setRect = function(s, x, y, w, h) { s.call(drawing.setPosition, x, y).call(drawing.setSize, w, h); }; +/** Translate / remove node + * + * @param {object} d : calcdata point item + * @param {sel} sel : d3 selction of node to translate + * @param {object} xa : corresponding full xaxis object + * @param {object} ya : corresponding full yaxis object + * + * @return {boolean} : + * true if selection got translated + * false if selection got removed + */ drawing.translatePoint = function(d, sel, xa, ya) { // put xp and yp into d if pixel scaling is already done var x = d.xp || xa.c2p(d.x), @@ -59,8 +70,12 @@ drawing.translatePoint = function(d, sel, xa, ya) { } else { sel.attr('transform', 'translate(' + x + ',' + y + ')'); } + } else { + sel.remove(); + return false; } - else sel.remove(); + + return true; }; drawing.translatePoints = function(s, xa, ya, trace) { From 49e5da549506fe9eca2367652a7285da285f5a12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 11 May 2017 10:57:48 -0400 Subject: [PATCH 3/4] use new Drawing.translatePoint return boolean to clear --- src/traces/scatter/plot.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js index aeeb56711ac..4896807a65d 100644 --- a/src/traces/scatter/plot.js +++ b/src/traces/scatter/plot.js @@ -458,8 +458,10 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition join.enter().append('g').classed('textpoint', true).append('text'); join.each(function(d) { - var sel = transition(d3.select(this).select('text')); - Drawing.translatePoint(d, sel, xa, ya); + var g = d3.select(this); + var sel = transition(g.select('text')); + var hasTextPt = Drawing.translatePoint(d, sel, xa, ya); + if(!hasTextPt) g.remove(); }); join.selectAll('text') From 3f7601c66bec6e8a2ef963cdb433938a315392c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 16 May 2017 13:37:19 -0400 Subject: [PATCH 4/4] bypass marker node styling if removed during translatePoint --- src/traces/scatter/plot.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js index 4896807a65d..22ddcfd8a66 100644 --- a/src/traces/scatter/plot.js +++ b/src/traces/scatter/plot.js @@ -391,7 +391,7 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition } function makePoints(d) { - var join, selection; + var join, selection, hasNode; var trace = d[0].trace, s = d3.select(this), @@ -433,11 +433,14 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition join.each(function(d) { var el = d3.select(this); var sel = transition(el); - Drawing.translatePoint(d, sel, xa, ya); - Drawing.singlePointStyle(d, sel, trace, markerScale, lineScale, gd); + hasNode = Drawing.translatePoint(d, sel, xa, ya); - if(trace.customdata) { - el.classed('plotly-customdata', d.data !== null && d.data !== undefined); + if(hasNode) { + Drawing.singlePointStyle(d, sel, trace, markerScale, lineScale, gd); + + if(trace.customdata) { + el.classed('plotly-customdata', d.data !== null && d.data !== undefined); + } } }); @@ -460,8 +463,8 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition join.each(function(d) { var g = d3.select(this); var sel = transition(g.select('text')); - var hasTextPt = Drawing.translatePoint(d, sel, xa, ya); - if(!hasTextPt) g.remove(); + hasNode = Drawing.translatePoint(d, sel, xa, ya); + if(!hasNode) g.remove(); }); join.selectAll('text')