Merge pull request #1709 from plotly/scatter-id-selection-order

Selection re-ordering for scatter traces with `ids`

Author: etpinard

src/components/drawing/index.js

@@ -47,7 +47,7 @@ drawing.setRect = function(s, x, y, w, h) {
     s.call(drawing.setPosition, x, y).call(drawing.setSize, w, h);
 };
 
-/** Translate / remove node
+/** Translate node
  *
  * @param {object} d : calcdata point item
  * @param {sel} sel : d3 selction of node to translate
@@ -56,7 +56,7 @@ drawing.setRect = function(s, x, y, w, h) {
  *
  * @return {boolean} :
  *  true if selection got translated
- *  false if selection got removed
+ *  false if selection could not get translated
  */
 drawing.translatePoint = function(d, sel, xa, ya) {
     // put xp and yp into d if pixel scaling is already done
@@ -71,7 +71,6 @@ drawing.translatePoint = function(d, sel, xa, ya) {
             sel.attr('transform', 'translate(' + x + ',' + y + ')');
         }
     } else {
-        sel.remove();
         return false;
     }
 

src/traces/scatter/plot.js

@@ -419,17 +419,20 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
         var enter = join.enter().append('path')
             .classed('point', true);
 
-        enter.call(Drawing.pointStyle, trace)
-            .call(Drawing.translatePoints, xa, ya, trace);
-
         if(hasTransition) {
-            enter.style('opacity', 0).transition()
+            enter
+                .call(Drawing.pointStyle, trace)
+                .call(Drawing.translatePoints, xa, ya, trace)
+                .style('opacity', 0)
+                .transition()
                 .style('opacity', 1);
         }
 
         var markerScale = showMarkers && Drawing.tryColorscale(trace.marker, '');
         var lineScale = showMarkers && Drawing.tryColorscale(trace.marker, 'line');
 
+        join.order();
+
         join.each(function(d) {
             var el = d3.select(this);
             var sel = transition(el);
@@ -441,6 +444,8 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
                 if(trace.customdata) {
                     el.classed('plotly-customdata', d.data !== null && d.data !== undefined);
                 }
+            } else {
+                sel.remove();
             }
         });
 
@@ -460,6 +465,8 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
         // it gets converted to mathjax
         join.enter().append('g').classed('textpoint', true).append('text');
 
+        join.order();
+
         join.each(function(d) {
             var g = d3.select(this);
             var sel = transition(g.select('text'));

test/jasmine/tests/scatter_test.js

@@ -437,6 +437,144 @@ describe('end-to-end scatter tests', function() {
         .catch(fail)
         .then(done);
     });
+
+    function _assertNodes(ptStyle, txContent) {
+        var pts = d3.selectAll('.point');
+        var txs = d3.selectAll('.textpoint');
+
+        expect(pts.size()).toEqual(ptStyle.length);
+        expect(txs.size()).toEqual(txContent.length);
+
+        pts.each(function(_, i) {
+            expect(d3.select(this).style('fill')).toEqual(ptStyle[i], 'pt ' + i);
+        });
+
+        txs.each(function(_, i) {
+            expect(d3.select(this).select('text').text()).toEqual(txContent[i], 'tx ' + i);
+        });
+    }
+
+    it('should reorder point and text nodes even when linked to ids (shuffle case)', function(done) {
+        Plotly.plot(gd, [{
+            x: [150, 350, 650],
+            y: [100, 300, 600],
+            text: ['apple', 'banana', 'clementine'],
+            ids: ['A', 'B', 'C'],
+            mode: 'markers+text',
+            marker: {
+                color: ['rgb(255, 0, 0)', 'rgb(0, 255, 0)', 'rgb(0, 0, 255)']
+            },
+            transforms: [{
+                type: 'sort',
+                enabled: false,
+                target: [0, 1, 0]
+            }]
+        }])
+        .then(function() {
+            _assertNodes(
+                ['rgb(255, 0, 0)', 'rgb(0, 255, 0)', 'rgb(0, 0, 255)'],
+                ['apple', 'banana', 'clementine']
+            );
+
+            return Plotly.restyle(gd, 'transforms[0].enabled', true);
+        })
+        .then(function() {
+            _assertNodes(
+                ['rgb(255, 0, 0)', 'rgb(0, 0, 255)', 'rgb(0, 255, 0)'],
+                ['apple', 'clementine', 'banana']
+            );
+
+            return Plotly.restyle(gd, 'transforms[0].enabled', false);
+        })
+        .then(function() {
+            _assertNodes(
+                ['rgb(255, 0, 0)', 'rgb(0, 255, 0)', 'rgb(0, 0, 255)'],
+                ['apple', 'banana', 'clementine']
+            );
+        })
+        .catch(fail)
+        .then(done);
+    });
+
+    it('should reorder point and text nodes even when linked to ids (add/remove case)', function(done) {
+        Plotly.plot(gd, [{
+            x: [150, 350, null, 600],
+            y: [100, 300, null, 700],
+            text: ['apple', 'banana', null, 'clementine'],
+            ids: ['A', 'B', null, 'C'],
+            mode: 'markers+text',
+            marker: {
+                color: ['rgb(255, 0, 0)', 'rgb(0, 255, 0)', null, 'rgb(0, 0, 255)']
+            },
+            transforms: [{
+                type: 'filter',
+                enabled: false,
+                target: [1, 0, 0, 1],
+                operation: '=',
+                value: 1
+            }]
+        }])
+        .then(function() {
+            _assertNodes(
+                ['rgb(255, 0, 0)', 'rgb(0, 255, 0)', 'rgb(0, 0, 255)'],
+                ['apple', 'banana', 'clementine']
+            );
+
+            return Plotly.restyle(gd, 'transforms[0].enabled', true);
+        })
+        .then(function() {
+            _assertNodes(
+                ['rgb(255, 0, 0)', 'rgb(0, 0, 255)'],
+                ['apple', 'clementine']
+            );
+
+            return Plotly.restyle(gd, 'transforms[0].enabled', false);
+        })
+        .then(function() {
+            _assertNodes(
+                ['rgb(255, 0, 0)', 'rgb(0, 255, 0)', 'rgb(0, 0, 255)'],
+                ['apple', 'banana', 'clementine']
+            );
+        })
+        .catch(fail)
+        .then(done);
+    });
+
+    it('should smoothly add/remove nodes tags with *ids* during animations', function(done) {
+        Plotly.plot(gd, {
+            data: [{
+                mode: 'markers+text',
+                y: [1, 2, 1],
+                text: ['apple', 'banana', 'clementine'],
+                ids: ['A', 'B', 'C'],
+                marker: { color: ['red', 'green', 'blue'] }
+            }],
+            frames: [{
+                data: [{
+                    y: [2, 1, 2],
+                    text: ['apple', 'banana', 'dragon fruit'],
+                    ids: ['A', 'C', 'D'],
+                    marker: { color: ['red', 'blue', 'yellow'] }
+                }]
+            }]
+        })
+        .then(function() {
+            _assertNodes(
+                ['rgb(255, 0, 0)', 'rgb(0, 128, 0)', 'rgb(0, 0, 255)'],
+                ['apple', 'banana', 'clementine']
+            );
+
+            return Plotly.animate(gd, null, {frame: {redraw: false}});
+        })
+        .then(function() {
+            _assertNodes(
+                ['rgb(255, 0, 0)', 'rgb(0, 0, 255)', 'rgb(255, 255, 0)'],
+                ['apple', 'banana', 'dragon fruit']
+            );
+        })
+        .catch(fail)
+        .then(done);
+    });
 });
 
 describe('scatter hoverPoints', function() {