Merge pull request #2870 from plotly/pie-colors

Pie colors fix & enhancements

Author: alexcjohnson

Diff for: src/plot_api/plot_api.js

@@ -292,11 +292,6 @@ exports.plot = function(gd, data, layout, config) {
             return;
         }
 
-        Plots.doCrossTraceCalc(gd);
-
-        // calc and autorange for errorbars
-        Registry.getComponentMethod('errorbars', 'calc')(gd);
-
         // TODO: autosize extra for text markers and images
         // see https://github.com/plotly/plotly.js/issues/1111
         return Lib.syncOrAsync([
@@ -331,7 +326,6 @@ exports.plot = function(gd, data, layout, config) {
     ];
 
     if(hasCartesian) seq.push(positionAndAutorange);
-    else seq.push(Plots.doCrossTraceCalc);
 
     seq.push(subroutines.layoutStyles);
     if(hasCartesian) seq.push(drawAxes);

Diff for: src/plots/plots.js

@@ -2236,8 +2236,6 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts)
 
         plots.supplyDefaults(gd);
         plots.doCalcdata(gd);
-        plots.doCrossTraceCalc(gd);
-        Registry.getComponentMethod('errorbars', 'calc')(gd);
 
         return Promise.resolve();
     }
@@ -2435,8 +2433,6 @@ plots.doCalcdata = function(gd, traces) {
 
     // for sharing colors across pies (and for legend)
     fullLayout._piecolormap = {};
-    fullLayout._piecolorway = null;
-    fullLayout._piedefaultcolorcount = 0;
 
     // If traces were specified and this trace was not included,
     // then transfer it over from the old calcdata:
@@ -2555,7 +2551,10 @@ plots.doCalcdata = function(gd, traces) {
     for(i = 0; i < fullData.length; i++) calci(i, true);
     for(i = 0; i < fullData.length; i++) calci(i, false);
 
+    doCrossTraceCalc(gd);
+
     Registry.getComponentMethod('fx', 'calc')(gd);
+    Registry.getComponentMethod('errorbars', 'calc')(gd);
 };
 
 function clearAxesCalc(axList) {
@@ -2564,7 +2563,7 @@ function clearAxesCalc(axList) {
     }
 }
 
-plots.doCrossTraceCalc = function(gd) {
+function doCrossTraceCalc(gd) {
     var fullLayout = gd._fullLayout;
     var modules = fullLayout._visibleModules;
     var hash = {};
@@ -2591,18 +2590,25 @@ plots.doCrossTraceCalc = function(gd) {
         var methods = hash[k];
         var subplots = fullLayout._subplots[k];
 
-        for(i = 0; i < subplots.length; i++) {
-            var sp = subplots[i];
-            var spInfo = k === 'cartesian' ?
-                fullLayout._plots[sp] :
-                fullLayout[sp];
+        if(Array.isArray(subplots)) {
+            for(i = 0; i < subplots.length; i++) {
+                var sp = subplots[i];
+                var spInfo = k === 'cartesian' ?
+                    fullLayout._plots[sp] :
+                    fullLayout[sp];
 
+                for(j = 0; j < methods.length; j++) {
+                    methods[j](gd, spInfo);
+                }
+            }
+        }
+        else {
             for(j = 0; j < methods.length; j++) {
-                methods[j](gd, spInfo);
+                methods[j](gd);
             }
         }
     }
-};
+}
 
 plots.rehover = function(gd) {
     if(gd._fullLayout._rehover) {

Diff for: src/traces/pie/calc.js

@@ -15,25 +15,20 @@ var tinycolor = require('tinycolor2');
 var Color = require('../../components/color');
 var helpers = require('./helpers');
 
-module.exports = function calc(gd, trace) {
+exports.calc = function calc(gd, trace) {
     var vals = trace.values;
     var hasVals = isArrayOrTypedArray(vals) && vals.length;
     var labels = trace.labels;
     var colors = trace.marker.colors || [];
     var cd = [];
     var fullLayout = gd._fullLayout;
-    var colorWay = fullLayout.colorway;
     var colorMap = fullLayout._piecolormap;
     var allThisTraceLabels = {};
     var vTotal = 0;
     var hiddenLabels = fullLayout.hiddenlabels || [];
 
     var i, v, label, hidden, pt;
 
-    if(!fullLayout._piecolorway && colorWay !== Color.defaults) {
-        fullLayout._piecolorway = generateDefaultColors(colorWay);
-    }
-
     if(trace.dlabel) {
         labels = new Array(vals.length);
         for(i = 0; i < vals.length; i++) {
@@ -79,7 +74,7 @@ module.exports = function calc(gd, trace) {
             cd.push({
                 v: v,
                 label: label,
-                color: pullColor(colors[i]),
+                color: pullColor(colors[i], label),
                 i: i,
                 pts: [i],
                 hidden: hidden
@@ -99,29 +94,6 @@ module.exports = function calc(gd, trace) {
 
     if(trace.sort) cd.sort(function(a, b) { return b.v - a.v; });
 
-    /**
-     * now go back and fill in colors we're still missing
-     * this is done after sorting, so we pick defaults
-     * in the order slices will be displayed
-     */
-
-    for(i = 0; i < cd.length; i++) {
-        pt = cd[i];
-        if(pt.color === false) {
-            // have we seen this label and assigned a color to it in a previous trace?
-            if(colorMap[pt.label]) {
-                pt.color = colorMap[pt.label];
-            }
-            else {
-                colorMap[pt.label] = pt.color = nextDefaultColor(
-                    fullLayout._piedefaultcolorcount,
-                    fullLayout._piecolorway
-                );
-                fullLayout._piedefaultcolorcount++;
-            }
-        }
-    }
-
     // include the sum of all values in the first point
     if(cd[0]) cd[0].vTotal = vTotal;
 
@@ -151,34 +123,66 @@ module.exports = function calc(gd, trace) {
     return cd;
 };
 
-/**
- * pick a default color from the main default set, augmented by
- * itself lighter then darker before repeating
+/*
+ * `calc` filled in (and collated) explicit colors.
+ * Now we need to propagate these explicit colors to other traces,
+ * and fill in default colors.
+ * This is done after sorting, so we pick defaults
+ * in the order slices will be displayed
  */
-var pieDefaultColors;
+exports.crossTraceCalc = function(gd) {
+    var fullLayout = gd._fullLayout;
+    var calcdata = gd.calcdata;
+    var pieColorWay = fullLayout.piecolorway;
+    var colorMap = fullLayout._piecolormap;
 
-function nextDefaultColor(index, pieColorWay) {
-    if(!pieDefaultColors) {
-        // generate this default set on demand (but then it gets saved in the module)
-        var mainDefaults = Color.defaults;
-        pieDefaultColors = generateDefaultColors(mainDefaults);
+    if(fullLayout.extendpiecolors) {
+        pieColorWay = generateExtendedColors(pieColorWay);
     }
+    var dfltColorCount = 0;
+
+    var i, j, cd, pt;
+    for(i = 0; i < calcdata.length; i++) {
+        cd = calcdata[i];
+        if(cd[0].trace.type !== 'pie') continue;
+
+        for(j = 0; j < cd.length; j++) {
+            pt = cd[j];
+            if(pt.color === false) {
+                // have we seen this label and assigned a color to it in a previous trace?
+                if(colorMap[pt.label]) {
+                    pt.color = colorMap[pt.label];
+                }
+                else {
+                    colorMap[pt.label] = pt.color = pieColorWay[dfltColorCount % pieColorWay.length];
+                    dfltColorCount++;
+                }
+            }
+        }
+    }
+};
 
-    var pieColors = pieColorWay || pieDefaultColors;
-    return pieColors[index % pieColors.length];
-}
+/**
+ * pick a default color from the main default set, augmented by
+ * itself lighter then darker before repeating
+ */
+var extendedColorWays = {};
 
-function generateDefaultColors(colorList) {
+function generateExtendedColors(colorList) {
     var i;
+    var colorString = JSON.stringify(colorList);
+    var pieColors = extendedColorWays[colorString];
+    if(!pieColors) {
+        pieColors = colorList.slice();
 
-    var pieColors = colorList.slice();
-
-    for(i = 0; i < colorList.length; i++) {
-        pieColors.push(tinycolor(colorList[i]).lighten(20).toHexString());
-    }
+        for(i = 0; i < colorList.length; i++) {
+            pieColors.push(tinycolor(colorList[i]).lighten(20).toHexString());
+        }
 
-    for(i = 0; i < colorList.length; i++) {
-        pieColors.push(tinycolor(colorList[i]).darken(20).toHexString());
+        for(i = 0; i < colorList.length; i++) {
+            pieColors.push(tinycolor(colorList[i]).darken(20).toHexString());
+        }
+        extendedColorWays[colorString] = pieColors;
     }
 
     return pieColors;

Diff for: src/traces/pie/index.js

@@ -14,7 +14,11 @@ Pie.attributes = require('./attributes');
 Pie.supplyDefaults = require('./defaults');
 Pie.supplyLayoutDefaults = require('./layout_defaults');
 Pie.layoutAttributes = require('./layout_attributes');
-Pie.calc = require('./calc');
+
+var calcModule = require('./calc');
+Pie.calc = calcModule.calc;
+Pie.crossTraceCalc = calcModule.crossTraceCalc;
+
 Pie.plot = require('./plot');
 Pie.style = require('./style');
 Pie.styleOne = require('./style_one');

Diff for: src/traces/pie/layout_attributes.js

@@ -17,5 +17,32 @@ module.exports = {
     hiddenlabels: {
         valType: 'data_array',
         editType: 'calc'
+    },
+    piecolorway: {
+        valType: 'colorlist',
+        role: 'style',
+        editType: 'calc',
+        description: [
+            'Sets the default pie slice colors. Defaults to the main',
+            '`colorway` used for trace colors. If you specify a new',
+            'list here it can still be extended with lighter and darker',
+            'colors, see `extendpiecolors`.'
+        ].join(' ')
+    },
+    extendpiecolors: {
+        valType: 'boolean',
+        dflt: true,
+        role: 'style',
+        editType: 'calc',
+        description: [
+            'If `true`, the pie slice colors (whether given by `piecolorway` or',
+            'inherited from `colorway`) will be extended to three times its',
+            'original length by first repeating every color 20% lighter then',
+            'each color 20% darker. This is intended to reduce the likelihood',
+            'of reusing the same color when you have many slices, but you can',
+            'set `false` to disable.',
+            'Colors provided in the trace, using `marker.colors`, are never',
+            'extended.'
+        ].join(' ')
     }
 };

Diff for: src/traces/pie/layout_defaults.js

@@ -17,4 +17,6 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut) {
         return Lib.coerce(layoutIn, layoutOut, layoutAttributes, attr, dflt);
     }
     coerce('hiddenlabels');
+    coerce('piecolorway', layoutOut.colorway);
+    coerce('extendpiecolors');
 };

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

@@ -775,7 +775,7 @@ describe('annotations autorange', function() {
             });
         })
         .then(function() {
-            _assert('auto rng / big tx', [-0.22, 3.57], [0.84, 3.365]);
+            _assert('auto rng / big tx', [-0.22, 3.59], [0.84, 3.365]);
             return Plotly.relayout(gd, 'annotations[0].text', 'a');
         })
         .then(function() {

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

@@ -1801,14 +1801,6 @@ function mockBarPlot(dataWithoutTraceType, layout) {
     supplyAllDefaults(gd);
     Plots.doCalcdata(gd);
 
-    var plotinfo = {
-        xaxis: gd._fullLayout.xaxis,
-        yaxis: gd._fullLayout.yaxis
-    };
-
-    // call Bar.crossTraceCalc
-    Bar.crossTraceCalc(gd, plotinfo);
-
     return gd;
 }
 

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

@@ -138,6 +138,50 @@ describe('Pie traces:', function() {
         .catch(failTest)
         .then(done);
     });
+
+    function _checkSliceColors(colors) {
+        return function() {
+            d3.select(gd).selectAll('.slice path').each(function(d, i) {
+                expect(this.style.fill.replace(/(\s|rgb\(|\))/g, '')).toBe(colors[i], i);
+            });
+        };
+    }
+
+    it('propagates explicit colors to the same labels in earlier OR later traces', function(done) {
+        var data1 = [
+            {type: 'pie', values: [3, 2], marker: {colors: ['red', 'black']}, domain: {x: [0.5, 1]}},
+            {type: 'pie', values: [2, 5], domain: {x: [0, 0.5]}}
+        ];
+        var data2 = Lib.extendDeep([], [data1[1], data1[0]]);
+
+        Plotly.newPlot(gd, data1)
+        .then(_checkSliceColors(['255,0,0', '0,0,0', '0,0,0', '255,0,0']))
+        .then(function() {
+            return Plotly.newPlot(gd, data2);
+        })
+        .then(_checkSliceColors(['0,0,0', '255,0,0', '255,0,0', '0,0,0']))
+        .catch(failTest)
+        .then(done);
+    });
+
+    it('can use a separate pie colorway and disable extended colors', function(done) {
+        Plotly.newPlot(gd, [{type: 'pie', values: [7, 6, 5, 4, 3, 2, 1]}], {colorway: ['#777', '#F00']})
+        .then(_checkSliceColors(['119,119,119', '255,0,0', '170,170,170', '255,102,102', '68,68,68', '153,0,0', '119,119,119']))
+        .then(function() {
+            return Plotly.relayout(gd, {extendpiecolors: false});
+        })
+        .then(_checkSliceColors(['119,119,119', '255,0,0', '119,119,119', '255,0,0', '119,119,119', '255,0,0', '119,119,119']))
+        .then(function() {
+            return Plotly.relayout(gd, {piecolorway: ['#FF0', '#0F0', '#00F']});
+        })
+        .then(_checkSliceColors(['255,255,0', '0,255,0', '0,0,255', '255,255,0', '0,255,0', '0,0,255', '255,255,0']))
+        .then(function() {
+            return Plotly.relayout(gd, {extendpiecolors: null});
+        })
+        .then(_checkSliceColors(['255,255,0', '0,255,0', '0,0,255', '255,255,102', '102,255,102', '102,102,255', '153,153,0']))
+        .catch(failTest)
+        .then(done);
+    });
 });
 
 describe('pie hovering', function() {