Fix style of bar 'outside' labels being pushed 'inside' [2951]

- Bar text labels specified as `textposition: outside` being pushed `inside` (to avoid collisions with bars stacked on top) were not styled as inside labels.
plotly · rmoestl · Oct 25, 2018 · Sep 27, 2018 · Sep 27, 2018 · Oct 11, 2018
commit df0d6c396abf7caaaf775f0d1a7b85d7037d22e3
diff --git a/src/traces/bar/defaults.js b/src/traces/bar/defaults.js
@@ -46,16 +46,21 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
 
     if(hasInside || hasOutside) {
         var textFont = coerceFont(coerce, 'textfont', layout.font);
-        if(hasInside) {
-            var insideTextFontDefault = Lib.extendFlat({}, textFont);
-            var isTraceTextfontColorSet = traceIn.textfont && traceIn.textfont.color;
-            var isColorInheritedFromLayoutFont = !isTraceTextfontColorSet;
-            if(isColorInheritedFromLayoutFont) {
-                delete insideTextFontDefault.color;
-            }
-            coerceFont(coerce, 'insidetextfont', insideTextFontDefault);
+
+        // Note that coercing `insidetextfont` is always needed –
+        // even if `textposition` is `outside` for each trace – since
+        // an outside label can become an inside one, e.g. because
+        // bar is stacked on top of it.
+        var insideTextFontDefault = Lib.extendFlat({}, textFont);
+        var isTraceTextfontColorSet = traceIn.textfont && traceIn.textfont.color;
+        var isColorInheritedFromLayoutFont = !isTraceTextfontColorSet;
+        if(isColorInheritedFromLayoutFont) {
+            delete insideTextFontDefault.color;
         }
+        coerceFont(coerce, 'insidetextfont', insideTextFontDefault);
+
         if(hasOutside) coerceFont(coerce, 'outsidetextfont', textFont);
+
         coerce('constraintext');
         coerce('selected.textfont.color');
         coerce('unselected.textfont.color');

diff --git a/src/traces/bar/style.js b/src/traces/bar/style.js
@@ -44,28 +44,7 @@ function style(gd, cd) {
 
 function stylePoints(sel, trace, gd) {
     var pts = sel.selectAll('path');
-    var txs = sel.selectAll('text');
-
     Drawing.pointStyle(pts, trace, gd);
-
-    txs.each(function(d) {
-        var tx = d3.select(this);
-        var textFont;
-
-        if(tx.classed('bartext-inside')) {
-            textFont = trace.insidetextfont;
-        } else if(tx.classed('bartext-outside')) {
-            textFont = trace.outsidetextfont;
-        }
-        if(!textFont) textFont = trace.textfont;
-
-        function cast(k) {
-            var cont = textFont[k];
-            return Array.isArray(cont) ? cont[d.i] : cont;
-        }
-
-        Drawing.font(tx, cast('family'), cast('size'), cast('color'));
-    });
 }
 
 function styleOnSelect(gd, cd) {

diff --git a/src/traces/pie/plot.js b/src/traces/pie/plot.js
@@ -410,7 +410,6 @@ module.exports = function plot(gd, cdpie) {
     }, 0);
 };
 
-// TODO DRY?
 function determineOutsideTextFont(trace, pt, layoutFont) {
     var customColor = helpers.castOption(trace.outsidetextfont.color, pt.pts) ||
       helpers.castOption(trace.textfont.color, pt.pts) ||

diff --git a/test/jasmine/tests/bar_test.js b/test/jasmine/tests/bar_test.js
@@ -1114,6 +1114,44 @@ describe('A bar plot', function() {
           .then(done);
     });
 
+    it('should use a contrasting text color by default for outside labels being pushed inside ' +
+      'because of another bar stacked above', function(done) {
+        var trace1 = {
+            x: [5],
+            y: [5],
+            text: ['Giraffes']
8EB1
,
+            type: 'bar',
+            textposition: 'outside'
+        };
+        var trace2 = Lib.extendFlat({}, trace1);
+        var layout = {barmode: 'stack'};
+
+        Plotly.plot(gd, [trace1, trace2], layout)
+          .then(assertTextFontColors([LIGHT, DARK]))
+          .catch(failTest)
+          .then(done);
+    });
+
+    it('should style outside labels pushed inside by bars stacked above as inside labels', function(done) {
+        var trace1 = {
+            x: [5],
+            y: [5],
+            text: ['Giraffes'],
+            type: 'bar',
+            textposition: 'outside',
+            insidetextfont: {color: 'blue', family: 'serif', size: 24}
+        };
+        var trace2 = Lib.extendFlat({}, trace1);
+        var layout = {barmode: 'stack', font: {family: 'Arial'}};
+
+        Plotly.plot(gd, [trace1, trace2], layout)
+          .then(assertTextFontColors([rgb('blue'), DARK]))
+          .then(assertTextFontFamilies(['serif', 'Arial']))
+          .then(assertTextFontSizes([24, 12]))
+          .catch(failTest)
+          .then(done);
+    });
+
     it('should fall back to textfont array values if insidetextfont array values don\'t ' +
       'cover all bars', function(done) {
         var trace = Lib.extendFlat({}, insideTextTestsTraceDef, {