make supplyDefault (and Plotly.validate) more strict w/ polar?.bar*

- coerce polar?.bargap and polar?.barmode only for subplots
  with visible barpolar traces on them.

Author: etpinard

src/traces/barpolar/layout_defaults.js

@@ -11,17 +11,23 @@
 var Lib = require('../../lib');
 var attrs = require('./layout_attributes');
 
-module.exports = function(layoutIn, layoutOut) {
-    var subplotIds = layoutOut._subplots.polar;
+module.exports = function(layoutIn, layoutOut, fullData) {
+    var subplotsDone = {};
     var sp;
 
     function coerce(attr, dflt) {
         return Lib.coerce(layoutIn[sp] || {}, layoutOut[sp], attrs, attr, dflt);
     }
 
-    for(var i = 0; i < subplotIds.length; i++) {
-        sp = subplotIds[i];
-        coerce('barmode');
-        coerce('bargap');
+    for(var i = 0; i < fullData.length; i++) {
+        var trace = fullData[i];
+        if(trace.type === 'barpolar' && trace.visible === true) {
+            sp = trace.subplot;
+            if(!subplotsDone[sp]) {
+                coerce('barmode');
+                coerce('bargap');
+                subplotsDone[sp] = 1;
+            }
+        }
     }
 };

test/jasmine/tests/barpolar_test.js

@@ -1,10 +1,43 @@
 var Plotly = require('@lib');
 var Lib = require('@src/lib');
 
+var supplyAllDefaults = require('../assets/supply_defaults');
 var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
 var failTest = require('../assets/fail_test');
 
+describe('Test barpolar defaults:', function() {
+    var gd;
+
+    function _supply(opts, layout) {
+        gd = {};
+        opts = Array.isArray(opts) ? opts : [opts];
+
+        gd.data = opts.map(function(o) {
+            return Lib.extendFlat({type: 'barpolar'}, o || {});
+        });
+        gd.layout = layout || {};
+
+        supplyAllDefaults(gd);
+    }
+
+    it('should not coerce polar.bar* attributes on subplot w/o visible barpolar', function() {
+        _supply([
+            {visible: false, subplot: 'polar'},
+            {r: [1], subplot: 'polar2'},
+            {type: 'scatterpolar', r: [1], subplot: 'polar3'}
+        ]);
+
+        var fullLayout = gd._fullLayout;
+        expect(fullLayout.polar.barmode).toBeUndefined();
+        expect(fullLayout.polar.bargap).toBeUndefined();
+        expect(fullLayout.polar2.barmode).toBe('stack');
+        expect(fullLayout.polar2.bargap).toBe(0.1);
+        expect(fullLayout.polar3.barmode).toBeUndefined();
+        expect(fullLayout.polar3.bargap).toBeUndefined();
+    });
+});
+
 describe('Test barpolar hover:', function() {
     var gd;
 

test/jasmine/tests/validate_test.js

@@ -558,17 +558,14 @@ describe('Plotly.validate', function() {
             polar3: {bargap: 0.4}
         });
 
-        expect(out.length).toBe(1);
+        expect(out.length).toBe(2);
         assertErrorContent(
-            out[0], 'unused', 'layout', null, ['polar3'], 'polar3',
+            out[0], 'unused', 'layout', null, ['polar2', 'bargap'], 'polar2.bargap',
+            'In layout, key polar2.bargap did not get coerced'
+        );
+        assertErrorContent(
+            out[1], 'unused', 'layout', null, ['polar3'], 'polar3',
             'In layout, container polar3 did not get coerced'
         );
-
-        // Plotly.validate should be more strict here.
-        //
-        // It should log an 'unused' warning for `polar2.bargap`,
-        // but trace layout attribute set in subplot containers are considered
-        // valid as long as one trace on that graph has those trace layout
-        // attributes.
     });
 });