Use contrasting inside text color for pie by default [2951]

Author: rmoestl

src/traces/pie/defaults.js

@@ -60,7 +60,15 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
 
         if(hasInside || hasOutside) {
             var dfltFont = coerceFont(coerce, 'textfont', layout.font);
-            if(hasInside) coerceFont(coerce, 'insidetextfont', dfltFont);
+            if(hasInside) {
+                var insideTextFontDefault = Lib.extendFlat({}, dfltFont);
+                var isTraceTextfontColorSet = traceIn.textfont && traceIn.textfont.color;
+                var isColorInheritedFromLayoutFont = !isTraceTextfontColorSet;
+                if(isColorInheritedFromLayoutFont) {
+                    delete insideTextFontDefault.color;
+                }
+                coerceFont(coerce, 'insidetextfont', insideTextFontDefault);
+            }
             if(hasOutside) coerceFont(coerce, 'outsidetextfont', dfltFont);
         }
     }

src/traces/pie/plot.js

@@ -264,7 +264,7 @@ module.exports = function plot(gd, cdpie) {
                             'text-anchor': 'middle'
                         })
                         .call(Drawing.font, textPosition === 'outside' ?
-                            trace.outsidetextfont : trace.insidetextfont)
+                            trace.outsidetextfont : determineInsideTextFont(trace, pt))
                         .call(svgTextUtils.convertToTspans, gd);
 
                     // position the text relative to the slice
@@ -418,10 +418,10 @@ function prerenderTitles(cdpie, gd) {
 
         if(trace.title) {
             var dummyTitle = Drawing.tester.append('text')
-                .attr('data-notex', 1)
-                .text(trace.title)
-                .call(Drawing.font, trace.titlefont)
-                .call(svgTextUtils.convertToTspans, gd);
+              .attr('data-notex', 1)
+              .text(trace.title)
+              .call(Drawing.font, trace.titlefont)
+              .call(svgTextUtils.convertToTspans, gd);
             var bBox = Drawing.bBox(dummyTitle.node(), true);
             cd0.titleBox = {
                 width: bBox.width,
@@ -432,6 +432,11 @@ function prerenderTitles(cdpie, gd) {
     }
 }
 
+function determineInsideTextFont(trace, pt) {
+    var contrast = Color.contrast(pt.color);
+    return Lib.extendFlat({}, { color: contrast }, trace.insidetextfont);
+}
+
 function transformInsideText(textBB, pt, cd0) {
     var textDiameter = Math.sqrt(textBB.width * textBB.width + textBB.height * textBB.height);
     var textAspect = textBB.width / textBB.height;

test/jasmine/tests/pie_test.js

@@ -9,19 +9,22 @@ var click = require('../assets/click');
 var getClientPosition = require('../assets/get_client_position');
 var mouseEvent = require('../assets/mouse_event');
 var supplyAllDefaults = require('../assets/supply_defaults');
+var color = require('../../../src/components/color');
+var rgb = color.rgb;
 
 var customAssertions = require('../assets/custom_assertions');
 var assertHoverLabelStyle = customAssertions.assertHoverLabelStyle;
 var assertHoverLabelContent = customAssertions.assertHoverLabelContent;
 
 var SLICES_SELECTOR = '.slice path';
+var SLICES_TEXT_SELECTOR = '.pielayer text.slicetext';
 var LEGEND_ENTRIES_SELECTOR = '.legendpoints path';
 
 describe('Pie defaults', function() {
-    function _supply(trace) {
+    function _supply(trace, layout) {
         var gd = {
             data: [trace],
-            layout: {}
+            layout: layout || {}
         };
 
         supplyAllDefaults(gd);
@@ -59,11 +62,24 @@ describe('Pie defaults', function() {
         out = _supply({type: 'pie', labels: ['A', 'B'], values: []});
         expect(out.visible).toBe(false);
     });
+
+    it('does not apply layout.font.color to insidetextfont.color (it\'ll be contrasting instead)', function() {
+        var out = _supply({type: 'pie', values: [1, 2]}, {font: {color: 'blue'}});
+        expect(out.insidetextfont.color).toBe(undefined);
+    });
+
+    it('does apply textfont.color to insidetextfont.color', function() {
+        var out = _supply({type: 'pie', values: [1, 2], textfont: {color: 'blue'}});
+        expect(out.insidetextfont.color).toBe('blue');
+    });
 });
 
 describe('Pie traces', function() {
     'use strict';
 
+    var DARK = color.rgb('#444');
+    var LIGHT = color.rgb('#fff');
+
     var gd;
 
     beforeEach(function() { gd = createGraphDiv(); });
@@ -149,6 +165,14 @@ describe('Pie traces', function() {
         };
     }
 
+    function _checkFontColors(expFontColors) {
+        return function() {
+            d3.selectAll(SLICES_TEXT_SELECTOR).each(function(d, i) {
+                expect(this.style.fill).toBe(expFontColors[i], i);
+            });
+        };
+    }
+
     it('propagate 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]}},
@@ -465,6 +489,36 @@ describe('Pie traces', function() {
           .catch(failTest)
           .then(done);
     });
+
+    var insideTextTestsTraceDef = {
+        values: [6, 5, 4, 3, 2, 1],
+        type: 'pie',
+        marker: {
+            colors: ['#ee1', '#eee', '#333', '#9467bd', '#dda', '#922'],
+        }
+    };
+
+    it('should use inside text colors contrasting to slice colors by default', function(done) {
+        Plotly.plot(gd, [insideTextTestsTraceDef])
+          .then(_checkFontColors([DARK, DARK, LIGHT, LIGHT, DARK, LIGHT]))
+          .catch(failTest)
+          .then(done);
+    });
+
+    it('should use textfont.color for inside text instead of the contrasting default', function(done) {
+        var data = Lib.extendFlat({}, insideTextTestsTraceDef, { textfont: { color: 'red' } });
+        Plotly.plot(gd, [data])
+          .then(_checkFontColors(Lib.repeat(rgb('red'), 6)))
+          .catch(failTest)
+          .then(done);
+    });
+
+    it('should not use layout.font.color for inside text, but a contrasting color instead', function(done) {
+        Plotly.plot(gd, [insideTextTestsTraceDef], {font: {color: 'green'}})
+          .then(_checkFontColors([DARK, DARK, LIGHT, LIGHT, DARK, LIGHT]))
+          .catch(failTest)
+          .then(done);
+    });
 });
 
 describe('pie hovering', function() {