Skip to content
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.

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

Implement equally sized bars #4994

Merged
merged 3 commits into from
Dec 2, 2017
Merged
Changes from 1 commit
Commits
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
149 changes: 105 additions & 44 deletions src/controllers/controller.bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,92 @@ defaults._set('horizontalBar', {
}
});

/**
* Computes the "optimal" sample size to maintain bars equally sized while preventing overlap.
* @private
*/
function computeMinSampleSize(scale, pixels) {
var min = scale.isHorizontal() ? scale.width : scale.height;
var ticks = scale.getTicks();
var prev, curr, i, ilen;

for (i = 0, ilen = pixels.length; i < ilen; ++i) {
min = i > 0 ? Math.min(min, pixels[i] - pixels[i - 1]) : min;
Copy link
Member

Choose a reason for hiding this comment

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

I think we can start this loop at 1 and then the ternary condition goes away

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, that was originally part of this loop (that's why i > 0), but forgot after moving it into a separate method.

}

for (i = 0, ilen = ticks.length; i < ilen; ++i) {
curr = scale.getPixelForTick(i);
min = i > 0 ? Math.min(min, curr - prev) : min;
prev = curr;
}

return min;
}

/**
* Computes the "ideal" sample range based on the absolute bar thickness or, if undefined or
Copy link
Contributor

Choose a reason for hiding this comment

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

It would help to explain what a sample range is and potentially why there are different fit and flex sample ranges

Copy link
Member Author

Choose a reason for hiding this comment

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

Relates to the description from the docs, but that's actually not "sample" but "category". It was initially a range, but after some refactor, it's not anymore one: I will rename these 2 methods compute*CategoryTraits (though, I don't like the "category" term here).

* null, uses the smallest interval (see computeMinSampleSize) that prevents bar overlapping.
* @private
*/
function computeFitSampleRange(index, ruler, options) {
var thickness = options.barThickness;
var count = ruler.stackCount;
var pixels = ruler.pixels;
Copy link
Contributor

Choose a reason for hiding this comment

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

You could probably inline pixels and curr since they are only used once. I think that should actually result in smaller minified code too

var curr = pixels[index];
var size, ratio;

if (helpers.isNullOrUndef(thickness)) {
size = ruler.min * options.categoryPercentage;
ratio = options.barPercentage;
} else {
// When bar thickness is enforced, category and bar percentages are ignored.
// Note(SB): we could add support for relative bar thickness (e.g. barThickness: '50%')
// and deprecate barPercentage since this value is ignored when thickness is absolute.
size = thickness * count;
ratio = 1;
}

return {
chunk: size / count,
ratio: ratio,
start: curr - (size / 2)
};
}

/**
* Computes a "dynamic" sample range that globally arranges bars side by side (no
* gap when percentage options are 1), based on the previous and following range.
* @private
*/
function computeFlexSampleRange(index, ruler, options) {
var pixels = ruler.pixels;
var curr = pixels[index];
var prev = index > 0 ? pixels[index - 1] : null;
var next = index < pixels.length - 1 ? pixels[index + 1] : null;
var percent = options.categoryPercentage;
var start, size;

if (prev === null) {
// first data: its size is double based on the next point or,
// if it's also the last data, we use the scale end extremity.
prev = curr - (next === null ? ruler.end - curr : next - curr);
}

if (next === null) {
// last data: its size is also double based on the previous point.
next = curr + curr - prev;
}

start = curr - ((curr - prev) / 2) * percent;
size = ((next - prev) / 2) * percent;

return {
chunk: size / ruler.stackCount,
ratio: options.barPercentage,
start: start
};
}

module.exports = function(Chart) {

Chart.controllers.bar = Chart.DatasetController.extend({
Expand Down Expand Up @@ -262,17 +348,22 @@ module.exports = function(Chart) {
var scale = me.getIndexScale();
var stackCount = me.getStackCount();
var datasetIndex = me.index;
var pixels = [];
var isHorizontal = scale.isHorizontal();
var start = isHorizontal ? scale.left : scale.top;
var end = start + (isHorizontal ? scale.width : scale.height);
var i, ilen;
var pixels = [];
var i, ilen, min;

for (i = 0, ilen = me.getMeta().data.length; i < ilen; ++i) {
pixels.push(scale.getPixelForValue(null, i, datasetIndex));
}

min = helpers.isNullOrUndef(scale.options.barThickness)
? computeMinSampleSize(scale, pixels)
: -1;

return {
min: min,
pixels: pixels,
start: start,
end: end,
Expand Down Expand Up @@ -332,51 +423,21 @@ module.exports = function(Chart) {
calculateBarIndexPixels: function(datasetIndex, index, ruler) {
var me = this;
var options = ruler.scale.options;
var meta = me.getMeta();
var stackIndex = me.getStackIndex(datasetIndex, meta.stack);
var pixels = ruler.pixels;
var base = pixels[index];
var length = pixels.length;
var start = ruler.start;
var end = ruler.end;
var leftSampleSize, rightSampleSize, leftCategorySize, rightCategorySize, fullBarSize, size;

if (length === 1) {
leftSampleSize = base > start ? base - start : end - base;
rightSampleSize = base < end ? end - base : base - start;
} else {
if (index > 0) {
leftSampleSize = (base - pixels[index - 1]) / 2;
if (index === length - 1) {
rightSampleSize = leftSampleSize;
}
}
if (index < length - 1) {
rightSampleSize = (pixels[index + 1] - base) / 2;
if (index === 0) {
leftSampleSize = rightSampleSize;
}
}
}

leftCategorySize = leftSampleSize * options.categoryPercentage;
rightCategorySize = rightSampleSize * options.categoryPercentage;
fullBarSize = (leftCategorySize + rightCategorySize) / ruler.stackCount;
size = fullBarSize * options.barPercentage;
var range = options.barThickness === 'flex'
? computeFlexSampleRange(index, ruler, options)
: computeFitSampleRange(index, ruler, options);

size = Math.min(
helpers.valueOrDefault(options.barThickness, size),
helpers.valueOrDefault(options.maxBarThickness, Infinity));

base -= leftCategorySize;
base += fullBarSize * stackIndex;
base += (fullBarSize - size) / 2;
var stackIndex = me.getStackIndex(datasetIndex, me.getMeta().stack);
var center = range.start + (range.chunk * stackIndex) + (range.chunk / 2);
var size = Math.min(
helpers.valueOrDefault(options.maxBarThickness, Infinity),
range.chunk * range.ratio);

return {
size: size,
base: base,
head: base + size,
center: base + size / 2
base: center - size / 2,
head: center + size / 2,
center: center,
size: size
};
},

Expand Down