Skip to content

Commit

Permalink
Merge pull request #4056 from plotly/fix4047-bar-size-and-width-zero
Browse files Browse the repository at this point in the history
Bug fix - do not display empty bars when line.width is zero
  • Loading branch information
archmoj authored Jul 18, 2019
2 parents 20e033f + 579a1b5 commit 9927079
Show file tree
Hide file tree
Showing 9 changed files with 724 additions and 18 deletions.
16 changes: 14 additions & 2 deletions src/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -459,13 +459,25 @@ lib.noneOrAll = function(containerIn, containerOut, attrList) {
* @param {object} cd : calcdata trace
* @param {string} cdAttr : calcdata key
*/
lib.mergeArray = function(traceAttr, cd, cdAttr) {
lib.mergeArray = function(traceAttr, cd, cdAttr, fn) {
var hasFn = typeof fn === 'function';
if(lib.isArrayOrTypedArray(traceAttr)) {
var imax = Math.min(traceAttr.length, cd.length);
for(var i = 0; i < imax; i++) cd[i][cdAttr] = traceAttr[i];
for(var i = 0; i < imax; i++) {
var v = traceAttr[i];
cd[i][cdAttr] = hasFn ? fn(v) : v;
}
}
};

// cast numbers to positive numbers, returns 0 if not greater than 0
lib.mergeArrayCastPositive = function(traceAttr, cd, cdAttr) {
return lib.mergeArray(traceAttr, cd, cdAttr, function(v) {
var w = +v;
return w > 0 ? w : 0;
});
};

/** fills calcdata field (given by cdAttr) with traceAttr values
* or function of traceAttr values (e.g. some fallback)
*
Expand Down
14 changes: 7 additions & 7 deletions src/traces/bar/arrays_to_calcdata.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,24 @@

'use strict';

var mergeArray = require('../../lib').mergeArray;
var Lib = require('../../lib');

// arrayOk attributes, merge them into calcdata array
module.exports = function arraysToCalcdata(cd, trace) {
for(var i = 0; i < cd.length; i++) cd[i].i = i;

mergeArray(trace.text, cd, 'tx');
mergeArray(trace.hovertext, cd, 'htx');
Lib.mergeArray(trace.text, cd, 'tx');
Lib.mergeArray(trace.hovertext, cd, 'htx');

var marker = trace.marker;
if(marker) {
mergeArray(marker.opacity, cd, 'mo');
mergeArray(marker.color, cd, 'mc');
Lib.mergeArray(marker.opacity, cd, 'mo', true);
Lib.mergeArray(marker.color, cd, 'mc');

var markerLine = marker.line;
if(markerLine) {
mergeArray(markerLine.color, cd, 'mlc');
mergeArray(markerLine.width, cd, 'mlw');
Lib.mergeArray(markerLine.color, cd, 'mlc');
Lib.mergeArrayCastPositive(markerLine.width, cd, 'mlw');
}
}
};
10 changes: 10 additions & 0 deletions src/traces/bar/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

var isNumeric = require('fast-isnumeric');
var tinycolor = require('tinycolor2');
var isArrayOrTypedArray = require('../../lib').isArrayOrTypedArray;

exports.coerceString = function(attributeDefinition, value, defaultValue) {
if(typeof value === 'string') {
Expand Down Expand Up @@ -64,3 +65,12 @@ exports.getValue = function(arrayOrScalar, index) {
else if(index < arrayOrScalar.length) value = arrayOrScalar[index];
return value;
};

exports.getLineWidth = function(trace, di) {
var w =
(0 < di.mlw) ? di.mlw :
!isArrayOrTypedArray(trace.marker.line.width) ? trace.marker.line.width :
0;

return w;
};
3 changes: 2 additions & 1 deletion src/traces/bar/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var Registry = require('../../registry');
var Color = require('../../components/color');

var fillText = require('../../lib').fillText;
var getLineWidth = require('./helpers').getLineWidth;

function hoverPoints(pointData, xval, yval, hovermode) {
var barPointData = hoverOnBars(pointData, xval, yval, hovermode);
Expand Down Expand Up @@ -165,7 +166,7 @@ function hoverOnBars(pointData, xval, yval, hovermode) {
function getTraceColor(trace, di) {
var mc = di.mcc || trace.marker.color;
var mlc = di.mlcc || trace.marker.line.color;
var mlw = di.mlw || trace.marker.line.width;
var mlw = getLineWidth(trace, di);

if(Color.opacity(mc)) return mc;
else if(Color.opacity(mlc) && mlw) return mlc;
Expand Down
22 changes: 14 additions & 8 deletions src/traces/bar/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function plot(gd, plotinfo, cdModule, traceLayer, opts) {
var isWaterfall = (trace.type === 'waterfall');
var isFunnel = (trace.type === 'funnel');
var isBar = (trace.type === 'bar');
var shouldDisplayZeros = isBar || isFunnel;
var shouldDisplayZeros = (isBar || isFunnel);

var adjustPixel = 0;
if(isWaterfall && trace.connector.visible && trace.connector.mode === 'between') {
Expand Down Expand Up @@ -102,12 +102,19 @@ function plot(gd, plotinfo, cdModule, traceLayer, opts) {
var y0 = xy[1][0];
var y1 = xy[1][1];

var isBlank = di.isBlank = !(
isNumeric(x0) && isNumeric(x1) &&
isNumeric(y0) && isNumeric(y1) &&
(x0 !== x1 || (shouldDisplayZeros && isHorizontal)) &&
(y0 !== y1 || (shouldDisplayZeros && !isHorizontal))
var isBlank = (
x0 === x1 ||
y0 === y1 ||
!isNumeric(x0) ||
!isNumeric(x1) ||
!isNumeric(y0) ||
!isNumeric(y1)
);
// display zeros if line.width > 0
if(isBlank && shouldDisplayZeros && helpers.getLineWidth(trace, di) && (isHorizontal ? x1 - x0 === 0 : y1 - y0 === 0)) {
isBlank = false;
}
di.isBlank = isBlank;

// in waterfall mode `between` we need to adjust bar end points to match the connector width
if(adjustPixel) {
Expand All @@ -130,8 +137,7 @@ function plot(gd, plotinfo, cdModule, traceLayer, opts) {
mc = cont.color;
}
} else {
lw = (di.mlw + 1 || trace.marker.line.width + 1 ||
(di.trace ? di.trace.marker.line.width : 0) + 1) - 1;
lw = helpers.getLineWidth(trace, di);
mc = di.mc || trace.marker.color;
}

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading

0 comments on commit 9927079

Please sign in to comment.