Merge pull request #3021 from plotly/polar-fail-gracefully-on-non-sense-inputs

Make polar be more graceful on non sense inputs

Author: etpinard

Diff for: src/lib/angles.js

@@ -23,11 +23,13 @@ function rad2deg(rad) { return rad / PI * 180; }
  * is sector a full circle?
  * ... this comes up a lot in SVG path-drawing routines
  *
+ * N.B. we consider all sectors that span more that 2pi 'full' circles
+ *
  * @param {2-item array} aBnds : angular bounds in *radians*
  * @return {boolean}
  */
 function isFullCircle(aBnds) {
-    return Math.abs(Math.abs(aBnds[1] - aBnds[0]) - twoPI) < 1e-15;
+    return Math.abs(aBnds[1] - aBnds[0]) > twoPI - 1e-15;
 }
 
 /**

Diff for: src/plots/polar/polar.js

@@ -380,6 +380,7 @@ proto.updateRadialAxis = function(fullLayout, polarLayout) {
     var radialLayout = polarLayout.radialaxis;
     var a0 = mod(polarLayout.sector[0], 360);
     var ax = _this.radialAxis;
+    var hasRoomForIt = innerRadius < radius;
 
     _this.fillViewInitialKey('radialaxis.angle', radialLayout.angle);
     _this.fillViewInitialKey('radialaxis.range', ax.range.slice());
@@ -410,8 +411,10 @@ proto.updateRadialAxis = function(fullLayout, polarLayout) {
         _this.radialTickLayout = newTickLayout;
     }
 
-    ax.setScale();
-    doTicksSingle(gd, ax, true);
+    if(hasRoomForIt) {
+        ax.setScale();
+        doTicksSingle(gd, ax, true);
+    }
 
     // stash 'actual' radial axis angle for drag handlers (in degrees)
     var angle = _this.radialAxisAngle = _this.vangles ?
@@ -420,24 +423,31 @@ proto.updateRadialAxis = function(fullLayout, polarLayout) {
 
     var trans = strTranslate(cx, cy) + strRotate(-angle);
 
-    updateElement(layers['radial-axis'], radialLayout.showticklabels || radialLayout.ticks, {
-        transform: trans
-    });
+    updateElement(
+        layers['radial-axis'],
+        hasRoomForIt && (radialLayout.showticklabels || radialLayout.ticks),
+        {transform: trans}
+    );
 
     // move all grid paths to about circle center,
     // undo individual grid lines translations
-    updateElement(layers['radial-grid'], radialLayout.showgrid, {
-        transform: strTranslate(cx, cy)
-    })
-    .selectAll('path').attr('transform', null);
-
-    updateElement(layers['radial-line'].select('line'), radialLayout.showline, {
-        x1: innerRadius,
-        y1: 0,
-        x2: radius,
-        y2: 0,
-        transform: trans
-    })
+    updateElement(
+        layers['radial-grid'],
+        hasRoomForIt && radialLayout.showgrid,
+        {transform: strTranslate(cx, cy)}
+    ).selectAll('path').attr('transform', null);
+
+    updateElement(
+        layers['radial-line'].select('line'),
+        hasRoomForIt && radialLayout.showline,
+        {
+            x1: innerRadius,
+            y1: 0,
+            x2: radius,
+            y2: 0,
+            transform: trans
+        }
+    )
     .attr('stroke-width', radialLayout.linewidth)
     .call(Color.stroke, radialLayout.linecolor);
 };
@@ -802,7 +812,8 @@ proto.updateMainDrag = function(fullLayout) {
         var cpath;
 
         if(clampAndSetR0R1(rr0, rr1)) {
-            path1 = path0 + _this.pathSector(r1) + _this.pathSector(r0);
+            path1 = path0 + _this.pathSector(r1);
+            if(r0) path1 += _this.pathSector(r0);
             // keep 'starting' angle
             cpath = pathCorner(r0, a0) + pathCorner(r1, a0);
         }
@@ -827,7 +838,8 @@ proto.updateMainDrag = function(fullLayout) {
         var cpath;
 
         if(clampAndSetR0R1(rr0, rr1)) {
-            path1 = path0 + _this.pathSector(r1) + _this.pathSector(r0);
+            path1 = path0 + _this.pathSector(r1);
+            if(r0) path1 += _this.pathSector(r0);
             // keep 'starting' angle here too
             cpath = [
                 pathCornerForPolygons(r0, vangles0[0], vangles0[1]),
@@ -963,7 +975,10 @@ proto.updateRadialDrag = function(fullLayout, polarLayout, rngIndex) {
 
     var radialDrag = dragBox.makeRectDragger(layers, className, 'crosshair', -bl2, -bl2, bl, bl);
     var dragOpts = {element: radialDrag, gd: gd};
-    d3.select(radialDrag).attr('transform', strTranslate(tx, ty));
+
+    updateElement(d3.select(radialDrag), radialAxis.visible && innerRadius < radius, {
+        transform: strTranslate(tx, ty)
+    });
 
     // move function (either rotate or re-range flavor)
     var moveFn2;

Diff for: src/plots/polar/set_convert.js

@@ -166,7 +166,7 @@ function setConvertAngular(ax, polarLayout) {
                 // Set the angular range in degrees to make auto-tick computation cleaner,
                 // changing rotation/direction should not affect the angular tick value.
                 ax.range = Lib.isFullCircle(sectorInRad) ?
-                    sector.slice() :
+                    [sector[0], sector[0] + 360] :
                     sectorInRad.map(g2rad).map(rad2deg);
                 break;
 

Diff for: src/traces/splom/defaults.js

@@ -89,7 +89,7 @@ function handleAxisDefaults(traceIn, traceOut, layout, coerce) {
     // fill in splom subplot keys
     for(i = 0; i < axLength; i++) {
         for(j = 0; j < axLength; j++) {
-            var id = [xaxes[i] + yaxes[j]];
+            var id = xaxes[i] + yaxes[j];
 
             if(i > j && showUpper) {
                 layout._splomSubplots[id] = 1;

Diff for: test/image/baselines/polar_blank.png

@@ -21,6 +21,11 @@
     "text": "<b>N.B.</b><br>radial<br>auotrange<br>for (0,0)<br>gives [0,1]",
     "textposition": "left",
     "subplot": "polar4"
+  }, {
+    "type": "scatterpolar",
+    "r": [1, 2, 3],
+    "theta": [0, 90, 200],
+    "subplot": "polar5"
   }],
   "layout": {
     "polar": {
@@ -69,6 +74,14 @@
       },
       "radialaxis": {"visible": false}
     },
+    "polar5": {
+      "domain": {
+        "x": [0.12, 0.34],
+        "y": [0.11, 0.34]
+      },
+      "hole": 1
+    },
+    "showlegend": false,
     "width": 600,
     "height": 500
   }

Diff for: test/image/mocks/polar_blank.json

@@ -1143,7 +1143,7 @@
     "polar20": {
       "sector": [
         -180,
-        180
+        200
       ],
       "bgcolor": "#d3d3d3",
       "domain": {

Diff for: test/image/mocks/polar_sector.json

@@ -215,7 +215,7 @@ describe('Test gl plot side effects', function() {
         .then(done);
     });
 
-    it('@gl should fire *plotly_webglcontextlost* when on webgl context lost', function(done) {
+    it('@noCI @gl should fire *plotly_webglcontextlost* when on webgl context lost', function(done) {
         var _mock = Lib.extendDeep({}, require('@mocks/gl2d_12.json'));
 
         function _trigger(name) {

Diff for: test/jasmine/tests/gl2d_plot_interact_test.js

@@ -13,6 +13,9 @@ var doubleClick = require('../assets/double_click');
 var drag = require('../assets/drag');
 var delay = require('../assets/delay');
 
+var customAssertions = require('../assets/custom_assertions');
+var assertNodeDisplay = customAssertions.assertNodeDisplay;
+
 describe('Test legacy polar plots logs:', function() {
     var gd;
 
@@ -628,6 +631,35 @@ describe('Test relayout on polar subplots:', function() {
         .catch(failTest)
         .then(done);
     });
+
+    it('should not attempt to draw radial axis when *polar.hole* is set to 1', function(done) {
+        var gd = createGraphDiv();
+
+        var queries = [
+            '.radial-axis', '.radial-grid', '.radial-line > line',
+            '.radialdrag', '.radialdrag-inner'
+        ];
+
+        function _assert(msg, showing) {
+            var exp = showing ? null : 'none';
+            var sp3 = d3.select(gd).select('.polarlayer > .polar');
+            queries.forEach(function(q) {
+                assertNodeDisplay(sp3.selectAll(q), [exp], msg + ' ' + q);
+            });
+        }
+
+        Plotly.plot(gd, [{
+            type: 'scatterpolar',
+            r: ['a', 'b', 'c']
+        }])
+        .then(function() { _assert('base', true); })
+        .then(function() { return Plotly.relayout(gd, 'polar.hole', 1); })
+        .then(function() { _assert('hole=1', false); })
+        .then(function() { return Plotly.relayout(gd, 'polar.hole', 0.2); })
+        .then(function() { _assert('hole=0.2', true); })
+        .catch(failTest)
+        .then(done);
+    });
 });
 
 describe('Test polar interactions:', function() {

Diff for: test/jasmine/tests/polar_test.js

@@ -1296,7 +1296,7 @@ describe('Test select box and lasso in general:', function() {
         .then(done);
     });
 
-    it('should cleanly clear and restart selections on double click when add/subtract mode on', function(done) {
+    it('@flaky should cleanly clear and restart selections on double click when add/subtract mode on', function(done) {
         var gd = createGraphDiv();
         var fig = Lib.extendDeep({}, require('@mocks/0.json'));