Skip to content

Commit

Permalink
fix(shape): Fix incorrect area/bar rendering
Browse files Browse the repository at this point in the history
Fix side-effect caused by #1304, making to return y Axis base value based on the current y Axis scale value.
Make sure this to used only non-grouped data and for y Axis min is greater than 0.

Fix #1316
  • Loading branch information
netil authored Apr 10, 2020
1 parent 267eea6 commit 9f81fb2
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 4 deletions.
89 changes: 89 additions & 0 deletions spec/shape/shape-spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/**
* Copyright (c) 2017 ~ present NAVER Corp.
* billboard.js project is licensed under the MIT license
*/
/* eslint-disable */
/* global describe, beforeEach, it, expect */
import util from "../assets/util";

describe("SHAPE", () => {
let chart;
let args;

beforeEach(() => {
chart = util.generate(args);
});

describe("shapes rendering", () => {
before(() => {
args = {
data: {
columns: [
["data1", 100, 200, 300, 400, 500],
["data2", -100, -200, -300, -400, -500]
],
types: {
data1: "area-step",
data2: "bar"
},
axes: {
data1: "y",
data2: "y2"
},
labels: {
show: true
}
},
area: {
linearGradient: true
},
axis: {
y: {
show: true,
min: -1000,
max: 1000
},
y2: {
show: true,
min: -1000,
max: 1000
}
},
legend: {
show: false
}
};
});

it("area/bars combination with positive and negative values", () => {
const expectedY = 228;

// check area
let rect = chart.$.line.areas.node().getBoundingClientRect();

expect(rect.y + rect.height).to.be.equal(expectedY);

// check area data label text position
chart.$.text.texts.filter(d => d.id === "data1").each(function() {
expect(this.getBoundingClientRect().y).to.be.below(expectedY);
});

// check bars
let height = 0;

chart.$.bar.bars.each(function() {
rect = this.getBoundingClientRect();

expect(rect.y).to.be.equal(expectedY);
expect(rect.height).to.be.above(height);

height = rect.height;
});

// check bars data label text position
chart.$.text.texts.filter(d => d.id === "data2").each(function() {
expect(this.getBoundingClientRect().y).to.be.above(expectedY);
});
});
});
});
6 changes: 3 additions & 3 deletions src/shape/line.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ extend(ChartInternal.prototype, {
const yScale = isSub ? $$.getSubYScale : $$.getYScale;

return (d, i) => {
const y0 = yScale.call($$, d.id)(0);
const y0 = yScale.call($$, d.id)($$.getShapeYMin(d.id));
const offset = lineOffset(d, i) || y0; // offset is for stacked area chart
const posX = x(d);
let posY = y(d);
Expand Down Expand Up @@ -449,7 +449,7 @@ extend(ChartInternal.prototype, {
getPoints(d, i)[0][1] :
yScaleGetter.call($$, d.id)(
$$.isAreaRangeType(d) ?
$$.getAreaRangeData(d, "high") : ($$.getShapeYMin(d.id))
$$.getAreaRangeData(d, "high") : $$.getShapeYMin(d.id)
));
const value1 = (d, i) => ($$.isGrouped(d.id) ?
getPoints(d, i)[1][1] :
Expand Down Expand Up @@ -507,7 +507,7 @@ extend(ChartInternal.prototype, {
const yScale = isSub ? $$.getSubYScale : $$.getYScale;

return function(d, i) {
const y0 = yScale.call($$, d.id)(0);
const y0 = yScale.call($$, d.id)($$.getShapeYMin(d.id));
const offset = areaOffset(d, i) || y0; // offset is for stacked area chart
const posX = x(d);
let posY = y(d);
Expand Down
3 changes: 2 additions & 1 deletion src/shape/shape.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,9 @@ extend(ChartInternal.prototype, {
*/
getShapeYMin(id) {
const $$ = this;
const [yMin] = $$[$$.axis.getId(id)].domain();

return (!$$.isGrouped(id) && $$.config[`axis_${$$.axis.getId(id)}_min`]) || 0;
return !$$.isGrouped(id) && yMin > 0 ? yMin : 0;
},

/**
Expand Down

0 comments on commit 9f81fb2

Please sign in to comment.