Skip to content

Commit

Permalink
Bar options should not be defined on scale (#6249)
Browse files Browse the repository at this point in the history
* Bar options should not be defined on scale

* Improve minimization

* Add tests

* Multiple datasets in test
  • Loading branch information
benmccann authored and etimberg committed Oct 25, 2019
1 parent 0b62f28 commit 9ff1c84
Show file tree
Hide file tree
Showing 13 changed files with 311 additions and 97 deletions.
2 changes: 1 addition & 1 deletion docs/axes/cartesian/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ All of the included cartesian axes support a number of common options.
| ---- | ---- | ------- | -----------
| `type` | `string` | | Type of scale being employed. Custom scales can be created and registered with a string key. This allows changing the type of an axis for a chart.
| `position` | `string` | | Position of the axis in the chart. Possible values are: `'top'`, `'left'`, `'bottom'`, `'right'`
| `offset` | `boolean` | `false` | If true, extra space is added to the both edges and the axis is scaled to fit into the chart area. This is set to `true` for a category scale in a bar chart by default.
| `offset` | `boolean` | `false` | If true, extra space is added to the both edges and the axis is scaled to fit into the chart area. This is set to `true` for a bar chart by default.
| `id` | `string` | | The ID is used to link datasets and scale axes together. [more...](#axis-id)
| `gridLines` | `object` | | Grid line configuration. [more...](../styling.md#grid-line-configuration)
| `scaleLabel` | `object` | | Scale title configuration. [more...](../labelling.md#scale-title-configuration)
Expand Down
2 changes: 1 addition & 1 deletion docs/axes/styling.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ The grid line configuration is nested under the scale configuration in the `grid
| `zeroLineColor` | `Color` | `'rgba(0, 0, 0, 0.25)'` | Stroke color of the grid line for the first index (index 0).
| `zeroLineBorderDash` | `number[]` | `[]` | Length and spacing of dashes of the grid line for the first index (index 0). See [MDN](https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/setLineDash).
| `zeroLineBorderDashOffset` | `number` | `0.0` | Offset for line dashes of the grid line for the first index (index 0). See [MDN](https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/lineDashOffset).
| `offsetGridLines` | `boolean` | `false` | If true, grid lines will be shifted to be between labels. This is set to `true` for a category scale in a bar chart by default.
| `offsetGridLines` | `boolean` | `false` | If true, grid lines will be shifted to be between labels. This is set to `true` for a bar chart by default.
| `z` | `number` | `0` | z-index of gridline layer. Values <= 0 are drawn under datasets, > 0 on top.

## Tick Configuration
Expand Down
41 changes: 29 additions & 12 deletions docs/charts/bar.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ The interaction with each bar can be controlled with the following properties:

All these values, if `undefined`, fallback to the associated [`elements.rectangle.*`](../configuration/elements.md#rectangle-configuration) options.

## Scale Configuration
The bar chart accepts the following configuration from the associated `scale` options:
## Dataset Configuration
The bar chart accepts the following configuration from the associated dataset options:

| Name | Type | Default | Description
| ---- | ---- | ------- | -----------
Expand All @@ -144,6 +144,33 @@ The bar chart accepts the following configuration from the associated `scale` op
| `barThickness` | <code>number&#124;string</code> | | Manually set width of each bar in pixels. If set to `'flex'`, it computes "optimal" sample widths that globally arrange bars side by side. If not set (default), bars are equally sized based on the smallest interval. [more...](#barthickness)
| `maxBarThickness` | `number` | | Set this to ensure that bars are not sized thicker than this.
| `minBarLength` | `number` | | Set this to ensure that bars have a minimum length in pixels.

### Example Usage

```javascript
data: {
datasets: [{
barPercentage: 0.5,
barThickness: 6,
maxBarThickness: 8,
minBarLength: 2,
data: [10, 20, 30, 40, 50, 60, 70]
}]
};
```
### barThickness
If this value is a number, it is applied to the width of each bar, in pixels. When this is enforced, `barPercentage` and `categoryPercentage` are ignored.

If set to `'flex'`, the base sample widths are calculated automatically based on the previous and following samples so that they take the full available widths without overlap. Then, bars are sized using `barPercentage` and `categoryPercentage`. There is no gap when the percentage options are 1. This mode generates bars with different widths when data are not evenly spaced.

If not set (default), the base sample widths are calculated using the smallest interval that prevents bar overlapping, and bars are sized using `barPercentage` and `categoryPercentage`. This mode always generates bars equally sized.

## Scale Configuration
The bar chart sets unique default values for the following configuration from the associated `scale` options:

| Name | Type | Default | Description
| ---- | ---- | ------- | -----------
| `offset` | `boolean` | `true` | If true, extra space is added to the both edges and the axis is scaled to fit into the chart area.
| `gridLines.offsetGridLines` | `boolean` | `true` | If true, the bars for a particular data point fall between the grid lines. The grid line will move to the left by one half of the tick interval. If false, the grid line will go right down the middle of the bars. [more...](#offsetgridlines)

### Example Usage
Expand All @@ -152,23 +179,13 @@ The bar chart accepts the following configuration from the associated `scale` op
options = {
scales: {
xAxes: [{
barPercentage: 0.5,
barThickness: 6,
maxBarThickness: 8,
minBarLength: 2,
gridLines: {
offsetGridLines: true
}
}]
}
};
```
### barThickness
If this value is a number, it is applied to the width of each bar, in pixels. When this is enforced, `barPercentage` and `categoryPercentage` are ignored.

If set to `'flex'`, the base sample widths are calculated automatically based on the previous and following samples so that they take the full available widths without overlap. Then, bars are sized using `barPercentage` and `categoryPercentage`. There is no gap when the percentage options are 1. This mode generates bars with different widths when data are not evenly spaced.

If not set (default), the base sample widths are calculated using the smallest interval that prevents bar overlapping, and bars are sized using `barPercentage` and `categoryPercentage`. This mode always generates bars equally sized.

### offsetGridLines
If true, the bars for a particular data point fall between the grid lines. The grid line will move to the left by one half of the tick interval, which is the space between the grid lines. If false, the grid line will go right down the middle of the bars. This is set to true for a category scale in a bar chart while false for other scales or chart types by default.
Expand Down
5 changes: 4 additions & 1 deletion samples/scales/time/financial.html
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@

var color = Chart.helpers.color;
var cfg = {
type: 'bar',
data: {
datasets: [{
label: 'CHRT - Chart.js Corporation',
Expand All @@ -119,6 +118,7 @@
xAxes: [{
type: 'time',
distribution: 'series',
offset: true,
ticks: {
major: {
enabled: true,
Expand Down Expand Up @@ -158,6 +158,9 @@
}
}],
yAxes: [{
gridLines: {
drawBorder: false
},
scaleLabel: {
display: true,
labelString: 'Closing price ($)'
Expand Down
78 changes: 58 additions & 20 deletions src/controllers/controller.bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ var defaults = require('../core/core.defaults');
var elements = require('../elements/index');
var helpers = require('../helpers/index');

var deprecated = helpers._deprecated;
var valueOrDefault = helpers.valueOrDefault;

defaults._set('bar', {
hover: {
mode: 'label'
Expand All @@ -13,8 +16,6 @@ defaults._set('bar', {
scales: {
xAxes: [{
type: 'category',
categoryPercentage: 0.8,
barPercentage: 0.9,
offset: true,
gridLines: {
offsetGridLines: true
Expand All @@ -27,6 +28,15 @@ defaults._set('bar', {
}
});

defaults._set('global', {
datasets: {
bar: {
categoryPercentage: 0.8,
barPercentage: 0.9
}
}
});

/**
* Computes the "optimal" sample size to maintain bars equally sized while preventing overlap.
* @private
Expand Down Expand Up @@ -58,10 +68,13 @@ function computeFitCategoryTraits(index, ruler, options) {
var thickness = options.barThickness;
var count = ruler.stackCount;
var curr = ruler.pixels[index];
var min = helpers.isNullOrUndef(thickness)
? computeMinSampleSize(ruler.scale, ruler.pixels)
: -1;
var size, ratio;

if (helpers.isNullOrUndef(thickness)) {
size = ruler.min * options.categoryPercentage;
size = min * options.categoryPercentage;
ratio = options.barPercentage;
} else {
// When bar thickness is enforced, category and bar percentages are ignored.
Expand Down Expand Up @@ -124,18 +137,30 @@ module.exports = DatasetController.extend({
'backgroundColor',
'borderColor',
'borderSkipped',
'borderWidth'
'borderWidth',
'barPercentage',
'barThickness',
'categoryPercentage',
'maxBarThickness',
'minBarLength'
],

initialize: function() {
var me = this;
var meta;
var meta, scaleOpts;

DatasetController.prototype.initialize.apply(me, arguments);

meta = me.getMeta();
meta.stack = me.getDataset().stack;
meta.bar = true;

scaleOpts = me._getIndexScale().options;
deprecated('bar chart', scaleOpts.barPercentage, 'scales.[x/y]Axes.barPercentage', 'dataset.barPercentage');
deprecated('bar chart', scaleOpts.barThickness, 'scales.[x/y]Axes.barThickness', 'dataset.barThickness');
deprecated('bar chart', scaleOpts.categoryPercentage, 'scales.[x/y]Axes.categoryPercentage', 'dataset.categoryPercentage');
deprecated('bar chart', me._getValueScale().options.minBarLength, 'scales.[x/y]Axes.minBarLength', 'dataset.minBarLength');
deprecated('bar chart', scaleOpts.maxBarThickness, 'scales.[x/y]Axes.maxBarThickness', 'dataset.maxBarThickness');
},

update: function(reset) {
Expand Down Expand Up @@ -173,23 +198,23 @@ module.exports = DatasetController.extend({
rectangle._model.borderSkipped = null;
}

me._updateElementGeometry(rectangle, index, reset);
me._updateElementGeometry(rectangle, index, reset, options);

rectangle.pivot();
},

/**
* @private
*/
_updateElementGeometry: function(rectangle, index, reset) {
_updateElementGeometry: function(rectangle, index, reset, options) {
var me = this;
var model = rectangle._model;
var vscale = me._getValueScale();
var base = vscale.getBasePixel();
var horizontal = vscale.isHorizontal();
var ruler = me._ruler || me.getRuler();
var vpixels = me.calculateBarValuePixels(me.index, index);
var ipixels = me.calculateBarIndexPixels(me.index, index, ruler);
var vpixels = me.calculateBarValuePixels(me.index, index, options);
var ipixels = me.calculateBarIndexPixels(me.index, index, ruler, options);

model.horizontal = horizontal;
model.base = reset ? base : vpixels.base;
Expand Down Expand Up @@ -266,18 +291,13 @@ module.exports = DatasetController.extend({
var me = this;
var scale = me._getIndexScale();
var pixels = [];
var i, ilen, min;
var i, ilen;

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

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

return {
min: min,
pixels: pixels,
start: scale._startPixel,
end: scale._endPixel,
Expand All @@ -290,15 +310,15 @@ module.exports = DatasetController.extend({
* Note: pixel values are not clamped to the scale area.
* @private
*/
calculateBarValuePixels: function(datasetIndex, index) {
calculateBarValuePixels: function(datasetIndex, index, options) {
var me = this;
var chart = me.chart;
var scale = me._getValueScale();
var isHorizontal = scale.isHorizontal();
var datasets = chart.data.datasets;
var metasets = scale._getMatchingVisibleMetas(me._type);
var value = scale._parseValue(datasets[datasetIndex].data[index]);
var minBarLength = scale.options.minBarLength;
var minBarLength = options.minBarLength;
var stacked = scale.options.stacked;
var stack = me.getMeta().stack;
var start = value.start === undefined ? 0 : value.max >= 0 && value.min >= 0 ? value.min : value.max;
Expand Down Expand Up @@ -349,17 +369,16 @@ module.exports = DatasetController.extend({
/**
* @private
*/
calculateBarIndexPixels: function(datasetIndex, index, ruler) {
calculateBarIndexPixels: function(datasetIndex, index, ruler, options) {
var me = this;
var options = ruler.scale.options;
var range = options.barThickness === 'flex'
? computeFlexCategoryTraits(index, ruler, options)
: computeFitCategoryTraits(index, ruler, options);

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),
valueOrDefault(options.maxBarThickness, Infinity),
range.chunk * range.ratio);

return {
Expand Down Expand Up @@ -389,5 +408,24 @@ module.exports = DatasetController.extend({
}

helpers.canvas.unclipArea(chart.ctx);
},

/**
* @private
*/
_resolveDataElementOptions: function() {
var me = this;
var values = helpers.extend({}, DatasetController.prototype._resolveDataElementOptions.apply(me, arguments));
var indexOpts = me._getIndexScale().options;
var valueOpts = me._getValueScale().options;

values.barPercentage = valueOrDefault(indexOpts.barPercentage, values.barPercentage);
values.barThickness = valueOrDefault(indexOpts.barThickness, values.barThickness);
values.categoryPercentage = valueOrDefault(indexOpts.categoryPercentage, values.categoryPercentage);
values.maxBarThickness = valueOrDefault(indexOpts.maxBarThickness, values.maxBarThickness);
values.minBarLength = valueOrDefault(valueOpts.minBarLength, values.minBarLength);

return values;
}

});
7 changes: 7 additions & 0 deletions src/helpers/helpers.core.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,13 @@ var helpers = {

ChartElement.__super__ = me.prototype;
return ChartElement;
},

_deprecated: function(scope, value, previous, current) {
if (value !== undefined) {
console.warn(scope + ': "' + previous +
'" is deprecated. Please use "' + current + '" instead');
}
}
};

Expand Down
15 changes: 4 additions & 11 deletions src/scales/scale.time.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ var defaults = require('../core/core.defaults');
var helpers = require('../helpers/index');
var Scale = require('../core/core.scale');

var deprecated = helpers._deprecated;
var resolve = helpers.options.resolve;
var valueOrDefault = helpers.valueOrDefault;

Expand Down Expand Up @@ -61,14 +62,6 @@ var INTERVALS = {

var UNITS = Object.keys(INTERVALS);

function deprecated(value, previous, current) {
if (value !== undefined) {
console.warn(
'time scale: "' + previous + '" is deprecated. ' +
'Please use "' + current + '" instead');
}
}

function sorter(a, b) {
return a - b;
}
Expand Down Expand Up @@ -460,9 +453,9 @@ module.exports = Scale.extend({
var adapter = me._adapter = new adapters._date(options.adapters.date);

// DEPRECATIONS: output a message only one time per update
deprecated(time.format, 'time.format', 'time.parser');
deprecated(time.min, 'time.min', 'ticks.min');
deprecated(time.max, 'time.max', 'ticks.max');
deprecated('time scale', time.format, 'time.format', 'time.parser');
deprecated('time scale', time.min, 'time.min', 'ticks.min');
deprecated('time scale', time.max, 'time.max', 'ticks.max');

// Backward compatibility: before introducing adapter, `displayFormats` was
// supposed to contain *all* unit/string pairs but this can't be resolved
Expand Down
Loading

0 comments on commit 9ff1c84

Please sign in to comment.