Merge pull request #1672 from plotly/scatter-text-join-fixup

Scatter <g textpoint> join fixup
plotly · May 16, 2017 · e58dcb0 · e58dcb0
2 parents ad65c23 + 3f7601c
commit e58dcb0
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 8 deletions.
diff --git 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) {

diff --git 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);
+                }
             }
         });
 
@@ -458,8 +461,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'));
+            hasNode = Drawing.translatePoint(d, sel, xa, ya);
+            if(!hasNode) g.remove();
         });
 
         join.selectAll('text')

diff --git 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() {