Merge remote-tracking branch 'upstream/master'

samelawrence · samelawrence · commit fae0803392fc · 2016-04-07T15:44:07.000-04:00
diff --git a/package.json b/package.json
@@ -35,6 +35,7 @@
     "test-syntax": "node test/syntax_test.js",
     "test-bundle": "node tasks/test_bundle.js",
     "test": "npm run citest-jasmine && npm run test-image && npm run test-syntax && npm run test-bundle",
+    "start": "npm run start-test_dashboard",
     "start-test_dashboard": "node devtools/test_dashboard/server.js",
     "start-image_viewer": "node devtools/image_viewer/server.js",
     "baseline": "./tasks/baseline.sh",
diff --git a/src/components/shapes/index.js b/src/components/shapes/index.js
@@ -89,7 +89,6 @@ function linearToData(ax) { return ax.type === 'category' ? ax.l2c : ax.l2d; }
 
 shapes.drawAll = function(gd) {
     var fullLayout = gd._fullLayout;
-    fullLayout._shapelayer.selectAll('path').remove();
     for(var i = 0; i < fullLayout.shapes.length; i++) {
         shapes.draw(gd, i);
     }
@@ -115,86 +114,106 @@ shapes.add = function(gd) {
 // if opt is blank, val can be 'add' or a full options object to add a new
 //  annotation at that point in the array, or 'remove' to delete this one
 shapes.draw = function(gd, index, opt, value) {
-    var layout = gd.layout,
-        fullLayout = gd._fullLayout,
-        i;
-
-    // TODO: abstract out these drawAll, add, and remove blocks for shapes and annotations
     if(!isNumeric(index) || index===-1) {
         // no index provided - we're operating on ALL shapes
         if(!index && Array.isArray(value)) {
-            // a whole annotation array is passed in
-            // (as in, redo of delete all)
-            layout.shapes = value;
-            shapes.supplyLayoutDefaults(layout, fullLayout);
-            shapes.drawAll(gd);
+            replaceAllShapes(gd, value);
             return;
         }
         else if(value==='remove') {
-            // delete all
-            delete layout.shapes;
-            fullLayout.shapes = [];
-            shapes.drawAll(gd);
+            deleteAllShapes(gd);
             return;
         }
         else if(opt && value!=='add') {
-            // make the same change to all shapes
-            for(i = 0; i < fullLayout.shapes.length; i++) {
-                shapes.draw(gd, i, opt, value);
-            }
+            updateAllShapes(gd, opt, value);
             return;
         }
         else {
             // add a new empty annotation
-            index = fullLayout.shapes.length;
-            fullLayout.shapes.push({});
+            index = gd._fullLayout.shapes.length;
+            gd._fullLayout.shapes.push({});
         }
     }
 
     if(!opt && value) {
         if(value==='remove') {
-            fullLayout._shapelayer.selectAll('[data-index="'+index+'"]')
-                .remove();
-            fullLayout.shapes.splice(index,1);
-            layout.shapes.splice(index,1);
-            for(i=index; i<fullLayout.shapes.length; i++) {
-                fullLayout._shapelayer
-                    .selectAll('[data-index="'+(i+1)+'"]')
-                    .attr('data-index',String(i));
-
-                // redraw all shapes past the removed one,
-                // so they bind to the right events
-                shapes.draw(gd,i);
-            }
+            deleteShape(gd, index);
             return;
         }
         else if(value==='add' || Plotly.Lib.isPlainObject(value)) {
-            fullLayout.shapes.splice(index,0,{});
+            insertShape(gd, index, value);
+        }
+    }
 
-            var rule = Plotly.Lib.isPlainObject(value) ?
-                    Plotly.Lib.extendFlat({}, value) :
-                    {text: 'New text'};
+    updateShape(gd, index, opt, value);
+};
 
-            if(layout.shapes) {
-                layout.shapes.splice(index, 0, rule);
-            } else {
-                layout.shapes = [rule];
-            }
+function replaceAllShapes(gd, newShapes) {
+    gd.layout.shapes = newShapes;
+    shapes.supplyLayoutDefaults(gd.layout, gd._fullLayout);
+    shapes.drawAll(gd);
+}
 
-            for(i=fullLayout.shapes.length-1; i>index; i--) {
-                fullLayout._shapelayer
-                    .selectAll('[data-index="'+(i-1)+'"]')
-                    .attr('data-index',String(i));
-                shapes.draw(gd,i);
-            }
-        }
+function deleteAllShapes(gd) {
+    delete gd.layout.shapes;
+    gd._fullLayout.shapes = [];
+    shapes.drawAll(gd);
+}
+
+function updateAllShapes(gd, opt, value) {
+    for(var i = 0; i < gd._fullLayout.shapes.length; i++) {
+        shapes.draw(gd, i, opt, value);
+    }
+}
+
+function deleteShape(gd, index) {
+    gd._fullLayout._shapelayer.selectAll('[data-index="' + index + '"]')
+        .remove();
+
+    gd._fullLayout.shapes.splice(index, 1);
+
+    gd.layout.shapes.splice(index, 1);
+
+    for(var i = index; i < gd._fullLayout.shapes.length; i++) {
+        // redraw all shapes past the removed one,
+        // so they bind to the right events
+        gd._fullLayout._shapelayer
+            .selectAll('[data-index="' + (i+1) + '"]')
+            .attr('data-index', String(i));
+        shapes.draw(gd, i);
+    }
+}
+
+function insertShape(gd, index, newShape) {
+    gd._fullLayout.shapes.splice(index, 0, {});
+
+    var rule = Plotly.Lib.isPlainObject(newShape) ?
+        Plotly.Lib.extendFlat({}, newShape) :
+        {text: 'New text'};
+
+    if(gd.layout.shapes) {
+        gd.layout.shapes.splice(index, 0, rule);
+    } else {
+        gd.layout.shapes = [rule];
     }
 
+    for(var i = gd._fullLayout.shapes.length - 1; i > index; i--) {
+        gd._fullLayout._shapelayer
+            .selectAll('[data-index="' + (i - 1) + '"]')
+            .attr('data-index', String(i));
+        shapes.draw(gd, i);
+    }
+}
+
+function updateShape(gd, index, opt, value) {
+    var i;
+
     // remove the existing shape if there is one
-    fullLayout._shapelayer.selectAll('[data-index="'+index+'"]').remove();
+    gd._fullLayout._shapelayer.selectAll('[data-index="' + index + '"]')
+        .remove();
 
     // remember a few things about what was already there,
-    var optionsIn = layout.shapes[index];
+    var optionsIn = gd.layout.shapes[index];
 
     // (from annos...) not sure how we're getting here... but C12 is seeing a bug
     // where we fail here when they add/remove annotations
@@ -261,8 +280,8 @@ shapes.draw = function(gd, index, opt, value) {
         optionsIn[posAttr] = position;
     }
 
-    var options = handleShapeDefaults(optionsIn, fullLayout);
-    fullLayout.shapes[index] = options;
+    var options = handleShapeDefaults(optionsIn, gd._fullLayout);
+    gd._fullLayout.shapes[index] = options;
 
     var attrs = {
             'data-index': String(index),
@@ -273,15 +292,18 @@ shapes.draw = function(gd, index, opt, value) {
 
     var lineColor = options.line.width ? options.line.color : 'rgba(0,0,0,0)';
 
-    var path = fullLayout._shapelayer.append('path')
+    var path = gd._fullLayout._shapelayer.append('path')
         .attr(attrs)
         .style('opacity', options.opacity)
         .call(Plotly.Color.stroke, lineColor)
         .call(Plotly.Color.fill, options.fillcolor)
         .call(Plotly.Drawing.dashLine, options.line.dash, options.line.width);
 
-    if(clipAxes) path.call(Plotly.Drawing.setClipUrl, 'clip' + fullLayout._uid + clipAxes);
-};
+    if(clipAxes) {
+        path.call(Plotly.Drawing.setClipUrl,
+            'clip' + gd._fullLayout._uid + clipAxes);
+    }
+}
 
 function decodeDate(convertToPx) {
     return function(v) { return convertToPx(v.replace('_', ' ')); };
diff --git a/src/traces/scatter3d/convert.js b/src/traces/scatter3d/convert.js
@@ -432,15 +432,15 @@ proto.dispose = function() {
         this.scatterPlot.dispose();
     }
     if(this.errorBars) {
-        this.scene.remove(this.errorBars);
+        this.scene.glplot.remove(this.errorBars);
         this.errorBars.dispose();
     }
     if(this.textMarkers) {
         this.scene.glplot.remove(this.textMarkers);
         this.textMarkers.dispose();
     }
     if(this.delaunayMesh) {
-        this.scene.glplot.remove(this.textMarkers);
+        this.scene.glplot.remove(this.delaunayMesh);
         this.delaunayMesh.dispose();
     }
 };
diff --git a/test/jasmine/tests/gl_plot_interact_test.js b/test/jasmine/tests/gl_plot_interact_test.js
@@ -73,7 +73,17 @@ describe('Test gl plot interactions', function() {
 
         beforeEach(function(done) {
             gd = createGraphDiv();
-            Plotly.plot(gd, mock.data, mock.layout).then(function() {
+
+            var mockCopy = Lib.extendDeep({}, mock);
+
+            // lines, markers, text, error bars and surfaces each
+            // correspond to one glplot object
+            mockCopy.data[0].mode = 'lines+markers+text';
+            mockCopy.data[0].error_z = { value: 10 };
+            mockCopy.data[0].surfaceaxis = 2;
+            mockCopy.layout.showlegend = true;
+
+            Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() {
                 delay(done);
             });
         });
@@ -190,6 +200,22 @@ describe('Test gl plot interactions', function() {
             });
         });
 
+        it('should be able to toggle visibility', function(done) {
+            var objects = gd._fullLayout.scene._scene.glplot.objects;
+
+            expect(objects.length).toEqual(5);
+
+            Plotly.restyle(gd, 'visible', 'legendonly').then(function() {
+                expect(objects.length).toEqual(0);
+
+                return Plotly.restyle(gd, 'visible', true);
+            }).then(function() {
+                expect(objects.length).toEqual(5);
+
+                done();
+            });
+        });
+
     });
 
     describe('gl2d plots', function() {
diff --git a/test/jasmine/tests/shapes_test.js b/test/jasmine/tests/shapes_test.js
@@ -7,7 +7,7 @@ var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
 
 
-describe('Test shapes nodes', function() {
+describe('Test shapes:', function() {
     'use strict';
 
     var mock = require('@mocks/shapes.json');
@@ -28,26 +28,82 @@ describe('Test shapes nodes', function() {
         return d3.selectAll('.shapelayer').size();
     }
 
-    function countPaths() {
+    function countShapePaths() {
         return d3.selectAll('.shapelayer > path').size();
     }
 
-    it('has one *shapelayer* node', function() {
-        expect(countShapeLayers()).toEqual(1);
-    });
+    describe('DOM', function() {
+        it('has one *shapelayer* node', function() {
+            expect(countShapeLayers()).toEqual(1);
+        });
+
+        it('has as many *path* nodes as there are shapes', function() {
+            expect(countShapePaths()).toEqual(mock.layout.shapes.length);
+        });
 
-    it('has as many *path* nodes as there are shapes', function() {
-        expect(countPaths()).toEqual(mock.layout.shapes.length);
+        it('should be able to get relayout', function(done) {
+            expect(countShapeLayers()).toEqual(1);
+            expect(countShapePaths()).toEqual(mock.layout.shapes.length);
+
+            Plotly.relayout(gd, {height: 200, width: 400}).then(function() {
+                expect(countShapeLayers()).toEqual(1);
+                expect(countShapePaths()).toEqual(mock.layout.shapes.length);
+            }).then(done);
+        });
     });
 
-    it('should be able to get relayout', function(done) {
-        expect(countShapeLayers()).toEqual(1);
-        expect(countPaths()).toEqual(mock.layout.shapes.length);
+    function countShapes(gd) {
+        return gd.layout.shapes ?
+            gd.layout.shapes.length :
+            0;
+    }
+
+    function getLastShape(gd) {
+        return gd.layout.shapes ?
+            gd.layout.shapes[gd.layout.shapes.length - 1] :
+            null;
+    }
 
-        Plotly.relayout(gd, {height: 200, width: 400}).then(function() {
-            expect(countShapeLayers()).toEqual(1);
-            expect(countPaths()).toEqual(mock.layout.shapes.length);
-            done();
+    function getRandomShape() {
+        return {
+            x0: Math.random(),
+            y0: Math.random(),
+            x1: Math.random(),
+            y1: Math.random()
+        };
+    }
+
+    describe('Plotly.relayout', function() {
+        it('should be able to add a shape', function(done) {
+            var pathCount = countShapePaths();
+            var index = countShapes(gd);
+            var shape = getRandomShape();
+
+            Plotly.relayout(gd, 'shapes[' + index + ']', shape).then(function() {
+                expect(countShapeLayers()).toEqual(1);
+                expect(countShapePaths()).toEqual(pathCount + 1);
+                expect(getLastShape(gd)).toEqual(shape);
+                expect(countShapes(gd)).toEqual(index + 1);
+            }).then(done);
+        });
+
+        it('should be able to remove a shape', function(done) {
+            var pathCount = countShapePaths();
+            var index = countShapes(gd);
+            var shape = getRandomShape();
+
+            Plotly.relayout(gd, 'shapes[' + index + ']', shape).then(function() {
+                expect(countShapeLayers()).toEqual(1);
+                expect(countShapePaths()).toEqual(pathCount + 1);
+                expect(getLastShape(gd)).toEqual(shape);
+                expect(countShapes(gd)).toEqual(index + 1);
+            }).then(function() {
+                Plotly.relayout(gd, 'shapes[' + index + ']', 'remove');
+            }).then(function() {
+                expect(countShapeLayers()).toEqual(1);
+                expect(countShapePaths()).toEqual(pathCount);
+                expect(countShapes(gd)).toEqual(index);
+            }).then(done);
         });
     });
 });

Original file line number	Diff line number	Diff line change
`@@ -432,15 +432,15 @@ proto.dispose = function() {`
`432`	`432`	`this.scatterPlot.dispose();`
`433`	`433`	`}`
`434`	`434`	`if(this.errorBars) {`
`435`		`- this.scene.remove(this.errorBars);`
	`435`	`+ this.scene.glplot.remove(this.errorBars);`
`436`	`436`	`this.errorBars.dispose();`
`437`	`437`	`}`
`438`	`438`	`if(this.textMarkers) {`
`439`	`439`	`this.scene.glplot.remove(this.textMarkers);`
`440`	`440`	`this.textMarkers.dispose();`
`441`	`441`	`}`
`442`	`442`	`if(this.delaunayMesh) {`
`443`		`- this.scene.glplot.remove(this.textMarkers);`
	`443`	`+ this.scene.glplot.remove(this.delaunayMesh);`
`444`	`444`	`this.delaunayMesh.dispose();`
`445`	`445`	`}`
`446`	`446`	`};`