Bar options should not be defined on scale

[benmccann]

Moves the bar options barPercentage, barThickness, categoryPercentage, minBarLength, and maxBarThickness from scale defaults to bar dataset defaults.

Closes #5134 #4587 #4096

[kurkle]

I'd first merge #5621 and rebase / refactor this to utilize changes in that.

[benmccann]

I've rebased on to #5621 but I'm not sure it's really possible to use those changes to help much since this has to support reading the values from the scale options for backwards compatibility

[nagix]

My concern is that the "category width" is calculated per scale, and categoryPercentage and barThickness (when 'flex' or default) should be common between datasets on the same scale. So, it seems to be natural that those two options belong to a scale, and other options including barThickness with a number value belong to a dataset.

By the way, these options might be able to be indexable and scriptable. If they are dataset options, they can be resolved in _resolveDataElementOptions and used in calculateBarIndexPixels/calculateBarValuePixels . In order to support scale options for backward compatibility, we can override _resolveDataElementOptions to add extra logic like the bubble controller.

[benmccann]

and categoryPercentage and barThickness (when 'flex' or default) should be common between datasets on the same scale

I actually like that this is more configurable and I'm not sure why they would have to be common. If you want one thick bar and one thin bar I don't see an issue with that. However, if you'd like to have them be common, you can still do that by setting options.datasets.bar.barPercentage

we can override _resolveDataElementOptions

Yeah, good idea. Done!

[nagix]

I actually like that this is more configurable and I'm not sure why they would have to be common. If you want one thick bar and one thin bar I don't see an issue with that.

It is completely fine to set barPercentage per dataset. But, if you have different categoryPercentage values in the datasets in the same group, the result will be corrupted (See barPercentage vs categoryPercentage). Similarly, if one dataset has barThickness: 'flex' and the other doesn't define it, the result will also be inconsistent.

I realized that both categoryPercentage and barThickness should be unique to neither a dataset nor a scale, but to a stack group.

Edit: The comment above was not correct. A stack group is used to group multiple bars into a stack but the group is nothing to do with categoryPercentage/barThickness.

However, if you'd like to have them be common, you can still do that by setting options.datasets.bar.barPercentage.

This may be acceptable, and it makes more sense than having these options in a scale.

Edit: So, these options are unique to a scale. I think they are still better to be dataset's options, though.

[benmccann]

@kurkle thanks for approving this PR earlier. I just rebased it to fix merge conflicts with recent commits. Mind giving it another look?

[kurkle]

Looks good, one minimization thing can be fixed and I'd add some fixture tests for mixed configuration between datasets (if the results are acceptable).
If results are bad , just add note / issue / todo / comment

[kurkle]

So after this barThickness can be changed between datasets?
I think a fixture test for this would be good addition.

thickness instead of options.barThickness

Also I'm curious what happens if datasets with mixed barThickness are stacked?

[benmccann]

I've fixed the minimization and added a test. Here's the image I added for the test:

[kurkle]

Thanks, but I don't see that test committed? I'd same without stacking too (if it works alright)

[benmccann]

Oops. Looks like I forgot to push the commit earlier. Done now!

[kurkle]

I'm not sure if that other test provides anything new; I was thinking multiple unstacked datasets with mixed barThickness. (So same config as in stacked one, but just with stacked: false)

[benmccann]

Ok. I've made it so that there are multiple datasets now