Skip to content

Commit df0d6c3

Browse files
committed
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.
1 parent 66fc665 commit df0d6c3

File tree

4 files changed

+51
-30
lines changed

4 files changed

+51
-30
lines changed

src/traces/bar/defaults.js

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,21 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
4646

4747
if(hasInside || hasOutside) {
4848
var textFont = coerceFont(coerce, 'textfont', layout.font);
49-
if(hasInside) {
50-
var insideTextFontDefault = Lib.extendFlat({}, textFont);
51-
var isTraceTextfontColorSet = traceIn.textfont && traceIn.textfont.color;
52-
var isColorInheritedFromLayoutFont = !isTraceTextfontColorSet;
53-
if(isColorInheritedFromLayoutFont) {
54-
delete insideTextFontDefault.color;
55-
}
56-
coerceFont(coerce, 'insidetextfont', insideTextFontDefault);
49+
50+
// Note that coercing `insidetextfont` is always needed –
51+
// even if `textposition` is `outside` for each trace – since
52+
// an outside label can become an inside one, e.g. because
53+
// bar is stacked on top of it.
54+
var insideTextFontDefault = Lib.extendFlat({}, textFont);
55+
var isTraceTextfontColorSet = traceIn.textfont && traceIn.textfont.color;
56+
var isColorInheritedFromLayoutFont = !isTraceTextfontColorSet;
57+
if(isColorInheritedFromLayoutFont) {
58+
delete insideTextFontDefault.color;
5759
}
60+
coerceFont(coerce, 'insidetextfont', insideTextFontDefault);
61+
5862
if(hasOutside) coerceFont(coerce, 'outsidetextfont', textFont);
63+
5964
coerce('constraintext');
6065
coerce('selected.textfont.color');
6166
coerce('unselected.textfont.color');

src/traces/bar/style.js

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -44,28 +44,7 @@ function style(gd, cd) {
4444

4545
function stylePoints(sel, trace, gd) {
4646
var pts = sel.selectAll('path');
47-
var txs = sel.selectAll('text');
48-
4947
Drawing.pointStyle(pts, trace, gd);
50-
51-
txs.each(function(d) {
52-
var tx = d3.select(this);
53-
var textFont;
54-
55-
if(tx.classed('bartext-inside')) {
56-
textFont = trace.insidetextfont;
57-
} else if(tx.classed('bartext-outside')) {
58-
textFont = trace.outsidetextfont;
59-
}
60-
if(!textFont) textFont = trace.textfont;
61-
62-
function cast(k) {
63-
var cont = textFont[k];
64-
return Array.isArray(cont) ? cont[d.i] : cont;
65-
}
66-
67-
Drawing.font(tx, cast('family'), cast('size'), cast('color'));
68-
});
6948
}
7049

7150
function styleOnSelect(gd, cd) {

src/traces/pie/plot.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,6 @@ module.exports = function plot(gd, cdpie) {
410410
}, 0);
411411
};
412412

413-
// TODO DRY?
414413
function determineOutsideTextFont(trace, pt, layoutFont) {
415414
var customColor = helpers.castOption(trace.outsidetextfont.color, pt.pts) ||
416415
helpers.castOption(trace.textfont.color, pt.pts) ||

test/jasmine/tests/bar_test.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,6 +1114,44 @@ describe('A bar plot', function() {
11141114
.then(done);
11151115
});
11161116

1117+
it('should use a contrasting text color by default for outside labels being pushed inside ' +
1118+
'because of another bar stacked above', function(done) {
1119+
var trace1 = {
1120+
x: [5],
1121+
y: [5],
1122+
text: ['Giraffes'],
1123+
type: 'bar',
1124+
textposition: 'outside'
1125+
};
1126+
var trace2 = Lib.extendFlat({}, trace1);
1127+
var layout = {barmode: 'stack'};
1128+
1129+
Plotly.plot(gd, [trace1, trace2], layout)
1130+
.then(assertTextFontColors([LIGHT, DARK]))
1131+
.catch(failTest)
1132+
.then(done);
1133+
});
1134+
1135+
it('should style outside labels pushed inside by bars stacked above as inside labels', function(done) {
1136+
var trace1 = {
1137+
x: [5],
1138+
y: [5],
1139+
text: ['Giraffes'],
1140+
type: 'bar',
1141+
textposition: 'outside',
1142+
insidetextfont: {color: 'blue', family: 'serif', size: 24}
1143+
};
1144+
var trace2 = Lib.extendFlat({}, trace1);
1145+
var layout = {barmode: 'stack', font: {family: 'Arial'}};
1146+
1147+
Plotly.plot(gd, [trace1, trace2], layout)
1148+
.then(assertTextFontColors([rgb('blue'), DARK]))
1149+
.then(assertTextFontFamilies(['serif', 'Arial']))
1150+
.then(assertTextFontSizes([24, 12]))
1151+
.catch(failTest)
1152+
.then(done);
1153+
});
1154+
11171155
it('should fall back to textfont array values if insidetextfont array values don\'t ' +
11181156
'cover all bars', function(done) {
11191157
var trace = Lib.extendFlat({}, insideTextTestsTraceDef, {

0 commit comments

Comments
 (0)