Skip to content

Bar fixes #1142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 14, 2016
Merged

Bar fixes #1142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6aa7adc
bar: fix normalised group bar plots
n-riesco Nov 14, 2016
864dd9e
bar: fix hover label
n-riesco Nov 14, 2016
f5198b9
test: update hover tests in bar_test
n-riesco Nov 14, 2016
74f3fa0
test: update mock bar_attrs_group_norm
n-riesco Nov 14, 2016
f4ac3a4
test: make linter happy
n-riesco Nov 14, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/traces/bar/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,15 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {

if(trace.orientation === 'h') {
pointData.x0 = pointData.x1 = xa.c2p(di.x, true);
pointData.xLabelVal = di.s;
pointData.xLabelVal = di.b + di.s;

pointData.y0 = ya.c2p(barPos(di) - barDelta, true);
pointData.y1 = ya.c2p(barPos(di) + barDelta, true);
pointData.yLabelVal = di.p;
}
else {
pointData.y0 = pointData.y1 = ya.c2p(di.y, true);
pointData.yLabelVal = di.s;
pointData.yLabelVal = di.b + di.s;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@n-riesco this line here is breaking hover for barmode: 'stack' bar charts.

Can share some insights on why you added this?

Copy link
Contributor Author

@n-riesco n-riesco Nov 17, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have a look and see what's broken. Is di.b defined? If I remember correctly it isn't for size-0 bars.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. Thanks!

Here's the simplest case http://codepen.io/etpinard/pen/Woopae

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the problem is that now the bar size in non-stacked bars is di.b + di.s, whereas in stacked bars is just di.s. We could distinguish both cases just by checking whether the trace defines base. I'll submit a PR.


pointData.x0 = xa.c2p(barPos(di) - barDelta, true);
pointData.x1 = xa.c2p(barPos(di) + barDelta, true);
Expand Down
6 changes: 3 additions & 3 deletions src/traces/bar/set_positions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,8 +485,8 @@ function stackBars(gd, sa, sieve) {
if(!isNumeric(bar.s)) continue;

// stack current bar and get previous sum
var barBase = sieve.put(bar.p, bar.s),
barTop = barBase + bar.s;
var barBase = sieve.put(bar.p, bar.b + bar.s),
barTop = barBase + bar.b + bar.s;

// store the bar base and top in each calcdata item
bar.b = barBase;
Expand Down Expand Up @@ -524,7 +524,7 @@ function sieveBars(gd, sa, sieve) {
for(var j = 0; j < trace.length; j++) {
var bar = trace[j];

if(isNumeric(bar.s)) sieve.put(bar.p, bar.s);
if(isNumeric(bar.s)) sieve.put(bar.p, bar.b + bar.s);
}
}
}
Expand Down
Binary file modified test/image/baselines/bar_attrs_group_norm.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 5 additions & 5 deletions test/image/mocks/bar_attrs_group_norm.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
{
"data":[
{
"base":4,
"x":[3,2,1,0],
"base":[100,40,25,10],
"x":[-50,10,25,40],
"type":"bar"
}, {
"base":[7,6,5,4],
"x":[1,2,3,4],
"base":[0,60,75,90],
"x":[50,-10,-25,-40],
"type":"bar"
}
],
"layout":{
"height":400,
"width":400,
"barmode":"group",
"barnorm":"percent"
"barnorm":"fraction"
}
}
52 changes: 45 additions & 7 deletions test/jasmine/tests/bar_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,27 @@ describe('Bar.setPositions', function() {
assertPointField(cd, 'y', [[0.75, 0.50, 0.25], [0.25, 0.50, 0.75]]);
});

it('should honor barnorm (group+base case)', function() {
var gd = mockBarPlot([{
base: [3, 2, 1],
y: [0, 0, 0]
}, {
y: [1, 2, 3]
}], {
bargap: 0,
barmode: 'group',
barnorm: 'fraction'
});

expect(gd._fullLayout.barnorm).toBe('fraction');

var cd = gd.calcdata;
assertPointField(cd, 'b', [[0.75, 0.50, 0.25], [0, 0, 0]]);
assertPointField(cd, 's', [[0, 0, 0], [0.25, 0.50, 0.75]]);
assertPointField(cd, 'x', [[-0.25, 0.75, 1.75], [0.25, 1.25, 2.25]]);
assertPointField(cd, 'y', [[0.75, 0.50, 0.25], [0.25, 0.50, 0.75]]);
});

it('should honor barnorm (stack case)', function() {
var gd = mockBarPlot([{
y: [3, 2, 1]
Expand Down Expand Up @@ -753,17 +774,34 @@ describe('bar hover', function() {
});

it('should return the correct hover point data (case y)', function() {
var out = _hover(gd, 185.645, 0.15, 'y');
var out = _hover(gd, 0.75, 0.15, 'y'),
subplot = gd._fullLayout._plots.xy,
xa = subplot.xaxis,
ya = subplot.yaxis,
barDelta = 1 * 0.8 / 2,
x0 = xa.c2p(0.5, true),
x1 = x0,
y0 = ya.c2p(0 - barDelta, true),
y1 = ya.c2p(0 + barDelta, true);

expect(out.style).toEqual([0, '#1f77b4', 75, 0]);
assertPos(out.pos, [182.88, 182.88, 214.5, 170.5]);
expect(out.style).toEqual([0, '#1f77b4', 0.5, 0]);
assertPos(out.pos, [x0, x1, y0, y1]);
});

it('should return the correct hover point data (case closest)', function() {
var out = _hover(gd, 135.88, -0.15, 'closest');

expect(out.style).toEqual([0, '#1f77b4', 75, 0]);
assertPos(out.pos, [182.88, 182.88, 214.5, 192.5]);
var out = _hover(gd, 0.75, -0.15, 'closest'),
subplot = gd._fullLayout._plots.xy,
xa = subplot.xaxis,
ya = subplot.yaxis,
barDelta = 1 * 0.8 / 2 / 2,
barPos = 0 - 1 * 0.8 / 2 + barDelta,
x0 = xa.c2p(0.5, true),
x1 = x0,
y0 = ya.c2p(barPos - barDelta, true),
y1 = ya.c2p(barPos + barDelta, true);

expect(out.style).toEqual([0, '#1f77b4', 0.5, 0]);
assertPos(out.pos, [x0, x1, y0, y1]);
});
});

Expand Down