Contour legend

src/components/legend/defaults.js

@@ -21,18 +21,33 @@ var helpers = require('./helpers');
 module.exports = function legendDefaults(layoutIn, layoutOut, fullData) {
     var containerIn = layoutIn.legend || {};
 
-    var visibleTraces = 0;
+    var legendTraceCount = 0;
+    var legendReallyHasATrace = false;
     var defaultOrder = 'normal';
 
     var defaultX, defaultY, defaultXAnchor, defaultYAnchor;
 
     for(var i = 0; i < fullData.length; i++) {
         var trace = fullData[i];
 
-        if(helpers.legendGetsTrace(trace)) {
-            visibleTraces++;
-            // always show the legend by default if there's a pie
-            if(Registry.traceIs(trace, 'pie')) visibleTraces++;
+        if(!trace.visible) continue;
+
+        // Note that we explicitly count any trace that is either shown or
+        // *would* be shown by default, toward the two traces you need to
+        // ensure the legend is shown by default, because this can still help
+        // disambiguate.
+        if(trace.showlegend || trace._dfltShowLegend) {
+            legendTraceCount++;
+            if(trace.showlegend) {
+                legendReallyHasATrace = true;
+                // Always show the legend by default if there's a pie,
+                // or if there's only one trace but it's explicitly shown
+                if(Registry.traceIs(trace, 'pie') ||
+                    trace._input.showlegend === true
+                ) {
+                    legendTraceCount++;
+                }
+            }
         }
 
         if((Registry.traceIs(trace, 'bar') && layoutOut.barmode === 'stack') ||
@@ -48,7 +63,8 @@ module.exports = function legendDefaults(layoutIn, layoutOut, fullData) {
     }
 
     var showLegend = Lib.coerce(layoutIn, layoutOut,
-        basePlotLayoutAttributes, 'showlegend', visibleTraces > 1);
+        basePlotLayoutAttributes, 'showlegend',
+        legendReallyHasATrace && legendTraceCount > 1);
 
     if(showLegend === false) return;
 

src/components/legend/get_legend_data.js

@@ -41,12 +41,12 @@ module.exports = function getLegendData(calcdata, opts) {
 
     // build an { legendgroup: [cd0, cd0], ... } object
     for(i = 0; i < calcdata.length; i++) {
-        var cd = calcdata[i],
-            cd0 = cd[0],
-            trace = cd0.trace,
-            lgroup = trace.legendgroup;
+        var cd = calcdata[i];
+        var cd0 = cd[0];
+        var trace = cd0.trace;
+        var lgroup = trace.legendgroup;
 
-        if(!helpers.legendGetsTrace(trace) || !trace.showlegend) continue;
+        if(!trace.visible || !trace.showlegend) continue;
 
         if(Registry.traceIs(trace, 'pie')) {
             if(!slicesShown[lgroup]) slicesShown[lgroup] = {};

src/components/legend/helpers.js

@@ -9,16 +9,6 @@
 
 'use strict';
 
-exports.legendGetsTrace = function legendGetsTrace(trace) {
-    // traceIs(trace, 'showLegend') is not sufficient anymore, due to contour(carpet)?
-    // which are legend-eligible only if type: constraint. Otherwise, showlegend gets deleted.
-
-    // Note that we explicitly include showlegend: false, so a trace that *could* be
-    // in the legend but is not shown still counts toward the two traces you need to
-    // ensure the legend is shown by default, because this can still help disambiguate.
-    return trace.visible && (trace.showlegend !== undefined);
-};
-
 exports.isGrouped = function isGrouped(legendLayout) {
     return (legendLayout.traceorder || '').indexOf('grouped') !== -1;
 };

src/plots/layout_attributes.js

@@ -173,7 +173,13 @@ module.exports = {
         valType: 'boolean',
         role: 'info',
         editType: 'legend',
-        description: 'Determines whether or not a legend is drawn.'
+        description: [
+            'Determines whether or not a legend is drawn.',
+            'Default is `true` if there is a trace to show and any of these:',
+            'a) Two or more traces would by default be shown in the legend.',
+            'b) One pie trace is shown in the legend.',
+            'c) One trace is explicitly given with `showlegend: true`.'
+        ].join(' ')
     },
     colorway: {
         valType: 'colorlist',

src/plots/plots.js

@@ -1144,9 +1144,13 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac
         coerce('ids');
 
         if(Registry.traceIs(traceOut, 'showLegend')) {
+            traceOut._dfltShowLegend = true;
             coerce('showlegend');
             coerce('legendgroup');
         }
+        else {
+            traceOut._dfltShowLegend = false;
+        }
 
         Registry.getComponentMethod(
             'fx',

src/traces/contour/style_defaults.js

@@ -30,6 +30,7 @@ module.exports = function handleStyleDefaults(traceIn, traceOut, coerce, layout,
         // plots/plots always coerces showlegend to true, but in this case
         // we default to false and (by default) show a colorbar instead
         if(traceIn.showlegend !== true) traceOut.showlegend = false;
+        traceOut._dfltShowLegend = false;
 
         colorscaleDefaults(
             traceIn, traceOut, layout, coerce, {prefix: '', cLetter: 'z'}

test/image/mocks/colorscale_opacity.json

@@ -567,7 +567,6 @@
         },
         "height": 450,
         "width": 1100,
-        "autosize": true,
-        "showlegend": false
+        "autosize": true
     }
 }

test/image/mocks/contour_scatter.json

@@ -341,8 +341,7 @@
       "line": {
         "color": "black"
       },
-      "type": "scatter",
-      "showlegend": false
+      "type": "scatter"
     }
   ]
 }

test/jasmine/tests/legend_test.js

@@ -33,12 +33,81 @@ describe('legend defaults', function() {
         };
     });
 
+    function allShown(fullData) {
+        return fullData.map(function(trace) {
+            return Lib.extendDeep({
+                visible: true,
+                showlegend: true,
+                _dfltShowLegend: true,
+                _input: {}
+            }, trace);
+        });
+    }
+
+    it('hides by default if there is only one legend item by default', function() {
+        fullData = allShown([
+            {type: 'scatter'},
+            {type: 'scatter', visible: false}, // ignored
+            {type: 'contour', _dfltShowLegend: false, showlegend: false} // hidden by default
+        ]);
+
+        supplyLayoutDefaults({}, layoutOut, fullData);
+        expect(layoutOut.showlegend).toBe(false);
+    });
+
+    it('shows if there are two legend items by default but only one is shown', function() {
+        fullData = allShown([
+            {type: 'scatter'},
+            {type: 'scatter', showlegend: false} // not shown but still triggers legend
+        ]);
+
+        supplyLayoutDefaults({}, layoutOut, fullData);
+        expect(layoutOut.showlegend).toBe(true);
+    });
+
+    it('hides if no items are actually shown', function() {
+        fullData = allShown([
+            {type: 'scatter', showlegend: false},
+            {type: 'scatter', showlegend: false}
+        ]);
+
+        supplyLayoutDefaults({}, layoutOut, fullData);
+        expect(layoutOut.showlegend).toBe(false);
+    });
+
+    it('shows with one visible pie', function() {
+        fullData = allShown([
+            {type: 'pie'}
+        ]);
+
+        supplyLayoutDefaults({}, layoutOut, fullData);
+        expect(layoutOut.showlegend).toBe(true);
+    });
+
+    it('does not show with a hidden pie', function() {
+        fullData = allShown([
+            {type: 'pie', showlegend: false}
+        ]);
+
+        supplyLayoutDefaults({}, layoutOut, fullData);
+        expect(layoutOut.showlegend).toBe(false);
+    });
+
+    it('shows if even a default hidden single item is explicitly shown', function() {
+        fullData = allShown([
+            {type: 'contour', _dfltShowLegend: false, _input: {showlegend: true}}
+        ]);
+
+        supplyLayoutDefaults({}, layoutOut, fullData);
+        expect(layoutOut.showlegend).toBe(true);
+    });
+
     it('should default traceorder to reversed for stack bar charts', function() {
-        fullData = [
-            { type: 'bar' },
-            { type: 'bar' },
-            { type: 'scatter' }
-        ];
+        fullData = allShown([
+            {type: 'bar', visible: 'legendonly'},
+            {type: 'bar', visible: 'legendonly'},
+            {type: 'scatter'}
+        ]);
 
         supplyLayoutDefaults(layoutIn, layoutOut, fullData);
         expect(layoutOut.legend.traceorder).toEqual('normal');
@@ -50,20 +119,20 @@ describe('legend defaults', function() {
     });
 
     it('should default traceorder to reversed for filled tonext scatter charts', function() {
-        fullData = [
-            { type: 'scatter' },
-            { type: 'scatter', fill: 'tonexty' }
-        ];
+        fullData = allShown([
+            {type: 'scatter'},
+            {type: 'scatter', fill: 'tonexty'}
+        ]);
 
         supplyLayoutDefaults(layoutIn, layoutOut, fullData);
         expect(layoutOut.legend.traceorder).toEqual('reversed');
     });
 
     it('should default traceorder to grouped when a group is present', function() {
-        fullData = [
-            { type: 'scatter', legendgroup: 'group' },
-            { type: 'scatter'}
-        ];
+        fullData = allShown([
+            {type: 'scatter', legendgroup: 'group'},
+            {type: 'scatter'}
+        ]);
 
         supplyLayoutDefaults(layoutIn, layoutOut, fullData);
         expect(layoutOut.legend.traceorder).toEqual('grouped');
@@ -74,6 +143,27 @@ describe('legend defaults', function() {
         expect(layoutOut.legend.traceorder).toEqual('grouped+reversed');
     });
 
+    it('does not consider invisible traces for traceorder default', function() {
+        fullData = allShown([
+            {type: 'bar', visible: false},
+            {type: 'bar', visible: false},
+            {type: 'scatter'}
+        ]);
+
+        layoutOut.barmode = 'stack';
+
+        supplyLayoutDefaults(layoutIn, layoutOut, fullData);
+        expect(layoutOut.legend.traceorder).toEqual('normal');
+
+        fullData = allShown([
+            {type: 'scatter', legendgroup: 'group', visible: false},
+            {type: 'scatter'}
+        ]);
+
+        supplyLayoutDefaults(layoutIn, layoutOut, fullData);
+        expect(layoutOut.legend.traceorder).toEqual('normal');
+    });
+
     it('should default orientation to vertical', function() {
         supplyLayoutDefaults(layoutIn, layoutOut, []);
         expect(layoutOut.legend.orientation).toEqual('v');
@@ -382,24 +472,6 @@ describe('legend getLegendData', function() {
 describe('legend helpers:', function() {
     'use strict';
 
-    describe('legendGetsTraces', function() {
-        var legendGetsTrace = helpers.legendGetsTrace;
-
-        it('should return true when trace is visible and supports legend', function() {
-            expect(legendGetsTrace({ visible: true, showlegend: true })).toBe(true);
-            expect(legendGetsTrace({ visible: false, showlegend: true })).toBe(false);
-            expect(legendGetsTrace({ visible: 'legendonly', showlegend: true })).toBe(true);
-
-            expect(legendGetsTrace({ visible: true, showlegend: false })).toBe(true);
-            expect(legendGetsTrace({ visible: false, showlegend: false })).toBe(false);
-            expect(legendGetsTrace({ visible: 'legendonly', showlegend: false })).toBe(true);
-
-            expect(legendGetsTrace({ visible: true })).toBe(false);
-            expect(legendGetsTrace({ visible: false })).toBe(false);
-            expect(legendGetsTrace({ visible: 'legendonly' })).toBe(false);
-        });
-    });
-
     describe('isGrouped', function() {
         var isGrouped = helpers.isGrouped;