From 16bb94ebc1109500ce890175bc80415eb36c22d4 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Thu, 9 Jan 2020 00:27:17 +0200 Subject: [PATCH] Bar chores (#6889) * Limit invisible bar section size * Improve readability * Fix for issue 6368 * Raview update * Review update, add test * Typos * Try to make sense :) --- src/controllers/controller.bar.js | 26 +++++++++++--- src/elements/element.rectangle.js | 12 ++++--- src/helpers/helpers.math.js | 11 ++++++ test/specs/controller.bar.tests.js | 56 ++++++++++++++++++++++++++---- 4 files changed, 90 insertions(+), 15 deletions(-) diff --git a/src/controllers/controller.bar.js b/src/controllers/controller.bar.js index 7a72ce29e..956c2a680 100644 --- a/src/controllers/controller.bar.js +++ b/src/controllers/controller.bar.js @@ -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, @@ -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]); @@ -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; }, @@ -419,7 +430,7 @@ 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) { @@ -427,7 +438,7 @@ module.exports = DatasetController.extend({ 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 @@ -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; diff --git a/src/elements/element.rectangle.js b/src/elements/element.rectangle.js index 8ebfc3380..c4ad49a9e 100644 --- a/src/elements/element.rectangle.js +++ b/src/elements/element.rectangle.js @@ -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); @@ -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) }; } diff --git a/src/helpers/helpers.math.js b/src/helpers/helpers.math.js index d5a5680b8..215b44db7 100644 --- a/src/helpers/helpers.math.js +++ b/src/helpers/helpers.math.js @@ -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)); +} diff --git a/test/specs/controller.bar.tests.js b/test/specs/controller.bar.tests.js index e7d36c3fc..240d2e554 100644 --- a/test/specs/controller.bar.tests.js +++ b/test/specs/controller.bar.tests.js @@ -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', @@ -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', @@ -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); @@ -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); -- 2.47.2