borderRadius gets ignored for the bottom corners of 0-value bars in bar chart when borderSkipped and minBarLength are set

[danielcolinjames]

Expected Behavior

When setting the following properties on a dataset:

minBarLength: 50,
borderRadius: 100,
borderSkipped: false,

bars with a value of 0 should show up as a circle with all corners respecting the borderRadius setting

Current Behavior

bars with a value of 0 are showing up instead with the bottom two corners ignoring the borderRadius setting and rendering as semi-circles with flat bottoms, like this:

Steps to Reproduce

Example: https://codepen.io/dcj/pen/VwMbGZM

Context

Trying to respect the designed version of the chart, where the value of some bars will be 0, and want to render those in a consistent way that respects the design

Environment

• Chart.js version: latest
• Browser name and version: happens the same in latest version of Safari, Chrome, Brave, Firefox
• Link to your project: [I can add a GitHub link to a PR where this will live, but it's not clean enough to push up yet]

[etimberg]

@kurkle @LeeLenaleee I traced this a bit. It's caused by https://github.com/chartjs/Chart.js/blob/master/src/controllers/controller.bar.js#L543-L545

When the chart is 150px high with the base at the bottom, the bar draws from 175px -> 125px. It looks like this code implicitly halves the size of the min bar length. This was added way back in #7642

Looking again at #7642, it might work to change the base to be actualBase when the base is hidden and minBarLength is applying since we want to apply the bar length from the scale base.

Thoughts?

[kurkle]

Maybe we need another option to configure that behavior. If the scale can go negative, then the current behavior could be desired.

[etimberg]

yaeh, it's been there for a long time and I don't recall a lot of issues with the min bar length feature so presumably it's working the way people expect

[etimberg]

I've got a super minimal working solution right now that keeps the base within the chart area but not sure if it has broken other tests

if (Math.abs(size) < minBarLength) {
  size = barSign(size, vScale, actualBase) * minBarLength;
  if (value === actualBase) {
    base -= size / 2;
  }
  const {right, left, top, bottom} = this.chart.chartArea;
  const max = vScale.axis === 'x' ? right : bottom;
  const min = vScale.axis === 'x' ? left : top;
  base = Math.max(Math.min(base, max), min);
  head = base + size;
}

[kurkle]

If that works, I think it still needs to use the scale boundaries, to make it work with stacked scales

[etimberg]

Good point. This is also working but I'd need to test if it works with stacked scales. I'm not familiar with how the scale boundaries work so not sure if this is the right way to do it.

if (Math.abs(size) < minBarLength) {
  size = barSign(size, vScale, actualBase) * minBarLength;
  if (value === actualBase) {
    base -= size / 2;
  }
  const start = vScale.getPixelForDecimal(0);
  const end = vScale.getPixelForDecimal(1);
  const min = Math.min(start, end);
  const max = Math.max(start, end);
  base = Math.max(Math.min(base, max), min);
  head = base + size;
}

[kurkle]

I think you can use top,left,bottom,right of the scale. Its a layout box afterall.
Edit: your solution is cleaner though :)

[etimberg]

my thinking on using the decimal 0/1 was that I can avoid checking if it's vertical or horizontal