Skip to content

Commit

Permalink
Bar chores (#6889)
Browse files Browse the repository at this point in the history 
* Limit invisible bar section size

* Improve readability

* Fix for issue 6368

* Raview update

* Review update, add test

* Typos

* Try to make sense :)
  • Loading branch information
@kurkle @etimberg
kurkle authored and etimberg committed Jan 8, 2020
1 parent a0dd956 commit 16bb94e
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 15 deletions.
26 changes: 22 additions & 4 deletions src/controllers/controller.bar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@ function parseArrayOrPrimitive(meta, data, start, count) {
return parsed;
}

function isFloatBar(custom) {
return custom && custom.barStart !== undefined && custom.barEnd !== undefined;
}

module.exports = DatasetController.extend({

dataElementType: elements.Rectangle,
Expand Down Expand Up @@ -251,7 +255,7 @@ module.exports = DatasetController.extend({
const {iScale, vScale} = meta;
const parsed = me._getParsed(index);
const custom = parsed._custom;
const value = custom
const value = isFloatBar(custom)
? '[' + custom.start + ', ' + custom.end + ']'
: '' + vScale.getLabelForValue(parsed[vScale.axis]);

Expand Down Expand Up @@ -354,6 +358,13 @@ module.exports = DatasetController.extend({
}
}

// No stacks? that means there is no visible data. Let's still initialize an `undefined`
// stack where possible invisible bars will be located.
// https://github.com/chartjs/Chart.js/issues/6368
if (!stacks.length) {
stacks.push(undefined);
}

return stacks;
},

Expand Down Expand Up @@ -419,15 +430,15 @@ module.exports = DatasetController.extend({
const custom = parsed._custom;
let value = parsed[vScale.axis];
let start = 0;
let length = meta._stacked ? me._applyStack(vScale, parsed) : parsed[vScale.axis];
let length = meta._stacked ? me._applyStack(vScale, parsed) : value;
let base, head, size;

if (length !== value) {
start = length - value;
length = value;
}

if (custom && custom.barStart !== undefined && custom.barEnd !== undefined) {
if (isFloatBar(custom)) {
value = custom.barStart;
length = custom.barEnd - custom.barStart;
// bars crossing origin are not stacked
Expand All @@ -437,7 +448,14 @@ module.exports = DatasetController.extend({
start += value;
}

base = vScale.getPixelForValue(start);
// Limit the bar to only extend up to 10 pixels past scale bounds (chartArea)
// So we don't try to draw so huge rectangles.
// https://github.com/chartjs/Chart.js/issues/5247
// TODO: use borderWidth instead (need to move the parsing from rectangle)
base = helpers.math._limitValue(vScale.getPixelForValue(start),
vScale._startPixel - 10,
vScale._endPixel + 10);

head = vScale.getPixelForValue(start + length);
size = head - base;

Expand Down
12 changes: 8 additions & 4 deletions src/elements/element.rectangle.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ function parseBorderSkipped(bar) {
return res;
}

function skipOrLimit(skip, value, min, max) {
return skip ? 0 : Math.max(Math.min(value, max), min);
}

function parseBorderWidth(bar, maxW, maxH) {
var value = bar.options.borderWidth;
var skip = parseBorderSkipped(bar);
Expand All @@ -85,10 +89,10 @@ function parseBorderWidth(bar, maxW, maxH) {
}

return {
t: skip.top || (t < 0) ? 0 : t > maxH ? maxH : t,
r: skip.right || (r < 0) ? 0 : r > maxW ? maxW : r,
b: skip.bottom || (b < 0) ? 0 : b > maxH ? maxH : b,
l: skip.left || (l < 0) ? 0 : l > maxW ? maxW : l
t: skipOrLimit(skip.top, t, 0, maxH),
r: skipOrLimit(skip.right, r, 0, maxW),
b: skipOrLimit(skip.bottom, b, 0, maxH),
l: skipOrLimit(skip.left, l, 0, maxW)
};
}

Expand Down
11 changes: 11 additions & 0 deletions src/helpers/helpers.math.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,14 @@ export function _angleBetween(angle, start, end) {
const endToAngle = _normalizeAngle(a - e);
return a === s || a === e || (angleToStart > angleToEnd && startToAngle < endToAngle);
}

/**
* Limit `value` between `min` and `max`
* @param {number} value
* @param {number} min
* @param {number} max
* @private
*/
export function _limitValue(value, min, max) {
return Math.max(min, Math.min(max, value));
}
56 changes: 49 additions & 7 deletions test/specs/controller.bar.tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ describe('Chart.controllers.bar', function() {
].forEach(function(expected, i) {
expect(meta.data[i].x).toBeCloseToPixel(expected.x);
expect(meta.data[i].y).toBeCloseToPixel(expected.y);
expect(meta.data[i].base).toBeCloseToPixel(1024);
expect(meta.data[i].base).toBeCloseToPixel(522);
expect(meta.data[i].width).toBeCloseToPixel(46);
expect(meta.data[i].options).toEqual(jasmine.objectContaining({
backgroundColor: 'red',
Expand Down Expand Up @@ -793,6 +793,48 @@ describe('Chart.controllers.bar', function() {
expect(bar2.y).toBeCloseToPixel(0);
});

it('should get the bar points for hidden dataset', function() {
var chart = window.acquireChart({
type: 'bar',
data: {
datasets: [{
data: [1, 2],
label: 'dataset1',
hidden: true
}],
labels: ['label1', 'label2']
},
options: {
legend: false,
title: false,
scales: {
x: {
type: 'category',
display: false
},
y: {
type: 'linear',
min: 0,
max: 2,
display: false
}
}
}
});

var meta = chart.getDatasetMeta(0);
expect(meta.data.length).toBe(2);

var bar1 = meta.data[0];
var bar2 = meta.data[1];

expect(bar1.x).toBeCloseToPixel(128);
expect(bar1.y).toBeCloseToPixel(256);
expect(bar2.x).toBeCloseToPixel(384);
expect(bar2.y).toBeCloseToPixel(0);
});


it('should update elements when the scales are stacked', function() {
var chart = window.acquireChart({
type: 'bar',
Expand Down Expand Up @@ -887,10 +929,10 @@ describe('Chart.controllers.bar', function() {
var meta0 = chart.getDatasetMeta(0);

[
{b: 1024, w: 92 / 2, x: 38, y: 512},
{b: 1024, w: 92 / 2, x: 166, y: 819},
{b: 1024, w: 92 / 2, x: 294, y: 922},
{b: 1024, w: 92 / 2, x: 422.5, y: 0}
{b: 522, w: 92 / 2, x: 38, y: 512},
{b: 522, w: 92 / 2, x: 166, y: 819},
{b: 522, w: 92 / 2, x: 294, y: 922},
{b: 522, w: 92 / 2, x: 422.5, y: 0}
].forEach(function(values, i) {
expect(meta0.data[i].base).toBeCloseToPixel(values.b);
expect(meta0.data[i].width).toBeCloseToPixel(values.w);
Expand All @@ -902,8 +944,8 @@ describe('Chart.controllers.bar', function() {

[
{b: 512, w: 92 / 2, x: 89, y: 0},
{b: 819, w: 92 / 2, x: 217, y: 0},
{b: 922, w: 92 / 2, x: 345, y: 0},
{b: 522, w: 92 / 2, x: 217, y: 0},
{b: 522, w: 92 / 2, x: 345, y: 0},
{b: 0, w: 92 / 2, x: 473.5, y: 0}
].forEach(function(values, i) {
expect(meta1.data[i].base).toBeCloseToPixel(values.b);
Expand Down

0 comments on commit 16bb94e

Please sign in to comment.