Added a maxBarThickness setting for bar charts xAxis

[seven7seven]

Fixes #3036

[etimberg]

I think this could be slightly simpler with:

if (maxBarThickness) {
  barWidth = Math.min(barWidth, maxBarThickness);
}

[seven7seven]

Cool. What about:

barWidth = xScale.options.stacked ? ruler.categoryWidth * xScale.options.barPercentage : ruler.barWidth;
return Math.min(barWidth, maxBarThickness || Infinity);

[etimberg]

I think that would also work 😄

[etimberg]

Just my one comment. Would love to get @potatopeelings to look at this too since he's done a lot of bar stuff

[seven7seven]

Weird, gulp coverage runs fine locally w/ the same node version. Also weird how it didn't fail on the test earlier, nothing related seems to have changed w/ the last commit.

[potatopeelings]

@seven7seven - you'll also need to change the horizontalBar equivalent (https://github.com/seven7seven/Chart.js/blob/9db4950b30a00d694c99fbd5e2bcb6b9e8842cc1/src/controllers/controller.bar.js#L469-L477)

Also, mostly nitpicking, I'd move the || Infinity to where maxBarThickness its initialised and the barWidth declaration to where its initialised.

Cheers!

[simonbrunel]

I would also cache var options = xScale.options.

[seven7seven]

@potatopeelings Agree with the || Infinity, but I reckon moving barWidth to where it's declared will be considerably slower in the case that xScale.options.barThickness is set, i.e. barWidth would be computed then discarded.

[potatopeelings]

@seven7seven - sounds good. Cheers!

[seven7seven]

@potatopeelings Any tips on how to refactor the duplicated code? How important do you feel that is?

[potatopeelings]

@seven7seven - if you mean the duplication between bar and horizontalBar, that would be best done in a separate issue (perhaps #2466 or possibly a new one).

Most of it can be refactored by making adding 2 more variables (the base axis and the measurement axis) that would be x and y for bar and y and x for horizontalBar and changing the method names to be orientation agnostic (eg. calculateBarThickness, instead of calculateBarWidth and calculateBarHeight). There are however a couple of places (eg. the rectangle properties) where this won't work.

[seven7seven]

What's the release strategy with this one for example? Minor 2.5 release, or wait for 2.6?