Skip to content

Commit

Permalink
Merge pull request #3701 from plotly/outsidetext-on-blank-bars
Browse files Browse the repository at this point in the history
Draw blank bars as empty paths + allow blank bars to draw outsidetext
  • Loading branch information
etpinard authored Apr 1, 2019
2 parents e674979 + f05a29a commit 189ad5d
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 26 deletions.
37 changes: 19 additions & 18 deletions src/traces/bar/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,11 @@ module.exports = function plot(gd, plotinfo, cdModule, traceLayer) {
di.ct = [(x0 + x1) / 2, y1];
}

if(!isNumeric(x0) || !isNumeric(x1) ||
!isNumeric(y0) || !isNumeric(y1) ||
x0 === x1 || y0 === y1) {
bar.remove();
return;
}
var isBlank = di.isBlank = (
!isNumeric(x0) || !isNumeric(x1) ||
!isNumeric(y0) || !isNumeric(y1) ||
x0 === x1 || y0 === y1
);

// in waterfall mode `between` we need to adjust bar end points to match the connector width
if(adjustPixel) {
Expand Down Expand Up @@ -158,7 +157,7 @@ module.exports = function plot(gd, plotinfo, cdModule, traceLayer) {

Lib.ensureSingle(bar, 'path')
.style('vector-effect', 'non-scaling-stroke')
.attr('d', 'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z')
.attr('d', isBlank ? 'M0,0Z' : 'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z')
.call(Drawing.setClipUrl, plotinfo.layerClipId, gd);

appendBarText(gd, bar, cd, i, x0, x1, y0, y1);
Expand Down Expand Up @@ -206,11 +205,6 @@ function appendBarText(gd, bar, calcTrace, i, x0, x1, y0, y1) {
var text = getText(trace, i);
textPosition = getTextPosition(trace, i);

if(!text || textPosition === 'none') {
bar.select('text').remove();
return;
}

var layoutFont = fullLayout.font;
var barColor = style.getBarColor(calcTrace[i], trace);
var insideTextFont = style.getInsideTextFont(trace, i, layoutFont, barColor);
Expand All @@ -219,15 +213,20 @@ function appendBarText(gd, bar, calcTrace, i, x0, x1, y0, y1) {
// compute text position
var prefix = trace.type === 'waterfall' ? 'waterfall' : 'bar';
var barmode = fullLayout[prefix + 'mode'];
var inStackMode = (barmode === 'stack');
var inRelativeMode = (barmode === 'relative');
var inStackOrRelativeMode = inStackMode || inRelativeMode;
var inStackOrRelativeMode = barmode === 'stack' || barmode === 'relative';

var calcBar = calcTrace[i];
var isOutmostBar = !inStackOrRelativeMode || calcBar._outmost;

var barWidth = Math.abs(x1 - x0) - 2 * TEXTPAD; // padding excluded
var barHeight = Math.abs(y1 - y0) - 2 * TEXTPAD; // padding excluded
// padding excluded
var barWidth = Math.abs(x1 - x0) - 2 * TEXTPAD;
var barHeight = Math.abs(y1 - y0) - 2 * TEXTPAD;

if(!text || textPosition === 'none' ||
(calcBar.isBlank && (textPosition === 'auto' || textPosition === 'inside'))) {
bar.select('text').remove();
return;
}

var textSelection;
var textBB;
Expand Down Expand Up @@ -263,7 +262,9 @@ function appendBarText(gd, bar, calcTrace, i, x0, x1, y0, y1) {
textSelection.remove();
textSelection = null;
}
} else textPosition = 'inside';
} else {
textPosition = 'inside';
}
}

if(!textSelection) {
Expand Down
Binary file added test/image/baselines/blank-bar-outsidetext.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
29 changes: 29 additions & 0 deletions test/image/mocks/blank-bar-outsidetext.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"data": [
{
"type": "bar",
"x": [ "textposition:outside" ],
"y": [ 0 ],
"text": [ "Text" ],
"textposition": [ "outside" ]
},
{
"type": "bar",
"x": [ "textpostion:auto" ],
"y": [ 0 ],
"text": [ "should not see" ],
"textposition": [ "auto" ]
},
{
"type": "bar",
"x": [ "textpostion:inside" ],
"y": [ 0 ],
"text": [ "should not see" ],
"textposition": [ "inside" ]
}
],
"layout": {
"showlegend": false,
"width": 600
}
}
10 changes: 6 additions & 4 deletions test/jasmine/tests/bar_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1547,7 +1547,8 @@ describe('A bar plot', function() {
assertTraceField(cd, 't.bargroupwidth', [0.8, 0.8, 0.8, 0.8]);

return Plotly.restyle(gd, 'offset', 0);
}).then(function() {
})
.then(function() {
var cd = gd.calcdata;
assertPointField(cd, 'x', [
[1.5, 2.4, 3.3, 4.2], [1.2, 2.3, 3.4, 4.5],
Expand Down Expand Up @@ -1584,7 +1585,7 @@ describe('A bar plot', function() {
var trace2Bar0 = getAllBarNodes(traceNodes[2])[0];
var path20 = trace2Bar0.querySelector('path');
var text20 = trace2Bar0.querySelector('text');
var trace3Bar0 = getAllBarNodes(traceNodes[3])[0];
var trace3Bar0 = getAllBarNodes(traceNodes[3])[1];
var path30 = trace3Bar0.querySelector('path');
var text30 = trace3Bar0.querySelector('text');

Expand All @@ -1610,7 +1611,8 @@ describe('A bar plot', function() {
Drawing.savedBBoxes = {};

return Plotly.restyle(gd, 'textposition', 'inside');
}).then(function() {
})
.then(function() {
var cd = gd.calcdata;
assertPointField(cd, 'x', [
[1.5, 2.4, 3.3, 4.2], [1.2, 2.3, 3.4, 4.5],
Expand Down Expand Up @@ -1647,7 +1649,7 @@ describe('A bar plot', function() {
var trace2Bar0 = getAllBarNodes(traceNodes[2])[0];
var path20 = trace2Bar0.querySelector('path');
var text20 = trace2Bar0.querySelector('text');
var trace3Bar0 = getAllBarNodes(traceNodes[3])[0];
var trace3Bar0 = getAllBarNodes(traceNodes[3])[1];
var path30 = trace3Bar0.querySelector('path');
var text30 = trace3Bar0.querySelector('text');

Expand Down
10 changes: 6 additions & 4 deletions test/jasmine/tests/waterfall_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,8 @@ describe('A waterfall plot', function() {
assertTraceField(cd, 't.bargroupwidth', [0.8, 0.8, 0.8, 0.8]);

return Plotly.restyle(gd, 'offset', 0);
}).then(function() {
})
.then(function() {
var cd = gd.calcdata;
assertPointField(cd, 'x', [
[1.5, 2.4, 3.3, 4.2], [1.2, 2.3, 3.4, 4.5],
Expand Down Expand Up @@ -836,7 +837,7 @@ describe('A waterfall plot', function() {
var trace2Waterfall0 = getAllWaterfallNodes(traceNodes[2])[0];
var path20 = trace2Waterfall0.querySelector('path');
var text20 = trace2Waterfall0.querySelector('text');
var trace3Waterfall0 = getAllWaterfallNodes(traceNodes[3])[0];
var trace3Waterfall0 = getAllWaterfallNodes(traceNodes[3])[1];
var path30 = trace3Waterfall0.querySelector('path');
var text30 = trace3Waterfall0.querySelector('text');

Expand All @@ -862,7 +863,8 @@ describe('A waterfall plot', function() {
Drawing.savedBBoxes = {};

return Plotly.restyle(gd, 'textposition', 'inside');
}).then(function() {
})
.then(function() {
var cd = gd.calcdata;
assertPointField(cd, 'x', [
[1.5, 2.4, 3.3, 4.2], [1.2, 2.3, 3.4, 4.5],
Expand Down Expand Up @@ -896,7 +898,7 @@ describe('A waterfall plot', function() {
var trace2Waterfall0 = getAllWaterfallNodes(traceNodes[2])[0];
var path20 = trace2Waterfall0.querySelector('path');
var text20 = trace2Waterfall0.querySelector('text');
var trace3Waterfall0 = getAllWaterfallNodes(traceNodes[3])[0];
var trace3Waterfall0 = getAllWaterfallNodes(traceNodes[3])[1];
var path30 = trace3Waterfall0.querySelector('path');
var text30 = trace3Waterfall0.querySelector('text');

Expand Down

0 comments on commit 189ad5d

Please sign in to comment.