Skip to content

Commit 9ff1c84

Browse files
benmccannetimberg
authored andcommitted
Bar options should not be defined on scale (#6249)
* Bar options should not be defined on scale * Improve minimization * Add tests * Multiple datasets in test
1 parent 0b62f28 commit 9ff1c84

File tree

13 files changed

+311
-97
lines changed

13 files changed

+311
-97
lines changed

docs/axes/cartesian/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ All of the included cartesian axes support a number of common options.
1515
| ---- | ---- | ------- | -----------
1616
| `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.
1717
| `position` | `string` | | Position of the axis in the chart. Possible values are: `'top'`, `'left'`, `'bottom'`, `'right'`
18-
| `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.
18+
| `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.
1919
| `id` | `string` | | The ID is used to link datasets and scale axes together. [more...](#axis-id)
2020
| `gridLines` | `object` | | Grid line configuration. [more...](../styling.md#grid-line-configuration)
2121
| `scaleLabel` | `object` | | Scale title configuration. [more...](../labelling.md#scale-title-configuration)

docs/axes/styling.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ The grid line configuration is nested under the scale configuration in the `grid
2222
| `zeroLineColor` | `Color` | `'rgba(0, 0, 0, 0.25)'` | Stroke color of the grid line for the first index (index 0).
2323
| `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).
2424
| `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).
25-
| `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.
25+
| `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.
2626
| `z` | `number` | `0` | z-index of gridline layer. Values <= 0 are drawn under datasets, > 0 on top.
2727

2828
## Tick Configuration

docs/charts/bar.md

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ The interaction with each bar can be controlled with the following properties:
134134

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

137-
## Scale Configuration
138-
The bar chart accepts the following configuration from the associated `scale` options:
137+
## Dataset Configuration
138+
The bar chart accepts the following configuration from the associated dataset options:
139139

140140
| Name | Type | Default | Description
141141
| ---- | ---- | ------- | -----------
@@ -144,6 +144,33 @@ The bar chart accepts the following configuration from the associated `scale` op
144144
| `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)
145145
| `maxBarThickness` | `number` | | Set this to ensure that bars are not sized thicker than this.
146146
| `minBarLength` | `number` | | Set this to ensure that bars have a minimum length in pixels.
147+
148+
### Example Usage
149+
150+
```javascript
151+
data: {
152+
datasets: [{
153+
barPercentage: 0.5,
154+
barThickness: 6,
155+
maxBarThickness: 8,
156+
minBarLength: 2,
157+
data: [10, 20, 30, 40, 50, 60, 70]
158+
}]
159+
};
160+
```
161+
### barThickness
162+
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.
163+
164+
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.
165+
166+
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.
167+
168+
## Scale Configuration
169+
The bar chart sets unique default values for the following configuration from the associated `scale` options:
170+
171+
| Name | Type | Default | Description
172+
| ---- | ---- | ------- | -----------
173+
| `offset` | `boolean` | `true` | If true, extra space is added to the both edges and the axis is scaled to fit into the chart area.
147174
| `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)
148175

149176
### Example Usage
@@ -152,23 +179,13 @@ The bar chart accepts the following configuration from the associated `scale` op
152179
options = {
153180
scales: {
154181
xAxes: [{
155-
barPercentage: 0.5,
156-
barThickness: 6,
157-
maxBarThickness: 8,
158-
minBarLength: 2,
159182
gridLines: {
160183
offsetGridLines: true
161184
}
162185
}]
163186
}
164187
};
165188
```
166-
### barThickness
167-
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.
168-
169-
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.
170-
171-
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.
172189

173190
### offsetGridLines
174191
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.

samples/scales/time/financial.html

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@
9797

9898
var color = Chart.helpers.color;
9999
var cfg = {
100-
type: 'bar',
101100
data: {
102101
datasets: [{
103102
label: 'CHRT - Chart.js Corporation',
@@ -119,6 +118,7 @@
119118
xAxes: [{
120119
type: 'time',
121120
distribution: 'series',
121+
offset: true,
122122
ticks: {
123123
major: {
124124
enabled: true,
@@ -158,6 +158,9 @@
158158
}
159159
}],
160160
yAxes: [{
161+
gridLines: {
162+
drawBorder: false
163+
},
161164
scaleLabel: {
162165
display: true,
163166
labelString: 'Closing price ($)'

src/controllers/controller.bar.js

Lines changed: 58 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ var defaults = require('../core/core.defaults');
55
var elements = require('../elements/index');
66
var helpers = require('../helpers/index');
77

8+
var deprecated = helpers._deprecated;
9+
var valueOrDefault = helpers.valueOrDefault;
10+
811
defaults._set('bar', {
912
hover: {
1013
mode: 'label'
@@ -13,8 +16,6 @@ defaults._set('bar', {
1316
scales: {
1417
xAxes: [{
1518
type: 'category',
16-
categoryPercentage: 0.8,
17-
barPercentage: 0.9,
1819
offset: true,
1920
gridLines: {
2021
offsetGridLines: true
@@ -27,6 +28,15 @@ defaults._set('bar', {
2728
}
2829
});
2930

31+
defaults._set('global', {
32+
datasets: {
33+
bar: {
34+
categoryPercentage: 0.8,
35+
barPercentage: 0.9
36+
}
37+
}
38+
});
39+
3040
/**
3141
* Computes the "optimal" sample size to maintain bars equally sized while preventing overlap.
3242
* @private
@@ -58,10 +68,13 @@ function computeFitCategoryTraits(index, ruler, options) {
5868
var thickness = options.barThickness;
5969
var count = ruler.stackCount;
6070
var curr = ruler.pixels[index];
71+
var min = helpers.isNullOrUndef(thickness)
72+
? computeMinSampleSize(ruler.scale, ruler.pixels)
73+
: -1;
6174
var size, ratio;
6275

6376
if (helpers.isNullOrUndef(thickness)) {
64-
size = ruler.min * options.categoryPercentage;
77+
size = min * options.categoryPercentage;
6578
ratio = options.barPercentage;
6679
} else {
6780
// When bar thickness is enforced, category and bar percentages are ignored.
@@ -124,18 +137,30 @@ module.exports = DatasetController.extend({
124137
'backgroundColor',
125138
'borderColor',
126139
'borderSkipped',
127-
'borderWidth'
140+
'borderWidth',
141+
'barPercentage',
142+
'barThickness',
143+
'categoryPercentage',
144+
'maxBarThickness',
145+
'minBarLength'
128146
],
129147

130148
initialize: function() {
131149
var me = this;
132-
var meta;
150+
var meta, scaleOpts;
133151

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

136154
meta = me.getMeta();
137155
meta.stack = me.getDataset().stack;
138156
meta.bar = true;
157+
158+
scaleOpts = me._getIndexScale().options;
159+
deprecated('bar chart', scaleOpts.barPercentage, 'scales.[x/y]Axes.barPercentage', 'dataset.barPercentage');
160+
deprecated('bar chart', scaleOpts.barThickness, 'scales.[x/y]Axes.barThickness', 'dataset.barThickness');
161+
deprecated('bar chart', scaleOpts.categoryPercentage, 'scales.[x/y]Axes.categoryPercentage', 'dataset.categoryPercentage');
162+
deprecated('bar chart', me._getValueScale().options.minBarLength, 'scales.[x/y]Axes.minBarLength', 'dataset.minBarLength');
163+
deprecated('bar chart', scaleOpts.maxBarThickness, 'scales.[x/y]Axes.maxBarThickness', 'dataset.maxBarThickness');
139164
},
140165

141166
update: function(reset) {
@@ -173,23 +198,23 @@ module.exports = DatasetController.extend({
173198
rectangle._model.borderSkipped = null;
174199
}
175200

176-
me._updateElementGeometry(rectangle, index, reset);
201+
me._updateElementGeometry(rectangle, index, reset, options);
177202

178203
rectangle.pivot();
179204
},
180205

181206
/**
182207
* @private
183208
*/
184-
_updateElementGeometry: function(rectangle, index, reset) {
209+
_updateElementGeometry: function(rectangle, index, reset, options) {
185210
var me = this;
186211
var model = rectangle._model;
187212
var vscale = me._getValueScale();
188213
var base = vscale.getBasePixel();
189214
var horizontal = vscale.isHorizontal();
190215
var ruler = me._ruler || me.getRuler();
191-
var vpixels = me.calculateBarValuePixels(me.index, index);
192-
var ipixels = me.calculateBarIndexPixels(me.index, index, ruler);
216+
var vpixels = me.calculateBarValuePixels(me.index, index, options);
217+
var ipixels = me.calculateBarIndexPixels(me.index, index, ruler, options);
193218

194219
model.horizontal = horizontal;
195220
model.base = reset ? base : vpixels.base;
@@ -266,18 +291,13 @@ module.exports = DatasetController.extend({
266291
var me = this;
267292
var scale = me._getIndexScale();
268293
var pixels = [];
269-
var i, ilen, min;
294+
var i, ilen;
270295

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

275-
min = helpers.isNullOrUndef(scale.options.barThickness)
276-
? computeMinSampleSize(scale, pixels)
277-
: -1;
278-
279300
return {
280-
min: min,
281301
pixels: pixels,
282302
start: scale._startPixel,
283303
end: scale._endPixel,
@@ -290,15 +310,15 @@ module.exports = DatasetController.extend({
290310
* Note: pixel values are not clamped to the scale area.
291311
* @private
292312
*/
293-
calculateBarValuePixels: function(datasetIndex, index) {
313+
calculateBarValuePixels: function(datasetIndex, index, options) {
294314
var me = this;
295315
var chart = me.chart;
296316
var scale = me._getValueScale();
297317
var isHorizontal = scale.isHorizontal();
298318
var datasets = chart.data.datasets;
299319
var metasets = scale._getMatchingVisibleMetas(me._type);
300320
var value = scale._parseValue(datasets[datasetIndex].data[index]);
301-
var minBarLength = scale.options.minBarLength;
321+
var minBarLength = options.minBarLength;
302322
var stacked = scale.options.stacked;
303323
var stack = me.getMeta().stack;
304324
var start = value.start === undefined ? 0 : value.max >= 0 && value.min >= 0 ? value.min : value.max;
@@ -349,17 +369,16 @@ module.exports = DatasetController.extend({
349369
/**
350370
* @private
351371
*/
352-
calculateBarIndexPixels: function(datasetIndex, index, ruler) {
372+
calculateBarIndexPixels: function(datasetIndex, index, ruler, options) {
353373
var me = this;
354-
var options = ruler.scale.options;
355374
var range = options.barThickness === 'flex'
356375
? computeFlexCategoryTraits(index, ruler, options)
357376
: computeFitCategoryTraits(index, ruler, options);
358377

359378
var stackIndex = me.getStackIndex(datasetIndex, me.getMeta().stack);
360379
var center = range.start + (range.chunk * stackIndex) + (range.chunk / 2);
361380
var size = Math.min(
362-
helpers.valueOrDefault(options.maxBarThickness, Infinity),
381+
valueOrDefault(options.maxBarThickness, Infinity),
363382
range.chunk * range.ratio);
364383

365384
return {
@@ -389,5 +408,24 @@ module.exports = DatasetController.extend({
389408
}
390409

391410
helpers.canvas.unclipArea(chart.ctx);
411+
},
412+
413+
/**
414+
* @private
415+
*/
416+
_resolveDataElementOptions: function() {
417+
var me = this;
418+
var values = helpers.extend({}, DatasetController.prototype._resolveDataElementOptions.apply(me, arguments));
419+
var indexOpts = me._getIndexScale().options;
420+
var valueOpts = me._getValueScale().options;
421+
422+
values.barPercentage = valueOrDefault(indexOpts.barPercentage, values.barPercentage);
423+
values.barThickness = valueOrDefault(indexOpts.barThickness, values.barThickness);
424+
values.categoryPercentage = valueOrDefault(indexOpts.categoryPercentage, values.categoryPercentage);
425+
values.maxBarThickness = valueOrDefault(indexOpts.maxBarThickness, values.maxBarThickness);
426+
values.minBarLength = valueOrDefault(valueOpts.minBarLength, values.minBarLength);
427+
428+
return values;
392429
}
430+
393431
});

src/helpers/helpers.core.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,13 @@ var helpers = {
306306

307307
ChartElement.__super__ = me.prototype;
308308
return ChartElement;
309+
},
310+
311+
_deprecated: function(scope, value, previous, current) {
312+
if (value !== undefined) {
313+
console.warn(scope + ': "' + previous +
314+
'" is deprecated. Please use "' + current + '" instead');
315+
}
309316
}
310317
};
311318

src/scales/scale.time.js

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ var defaults = require('../core/core.defaults');
55
var helpers = require('../helpers/index');
66
var Scale = require('../core/core.scale');
77

8+
var deprecated = helpers._deprecated;
89
var resolve = helpers.options.resolve;
910
var valueOrDefault = helpers.valueOrDefault;
1011

@@ -61,14 +62,6 @@ var INTERVALS = {
6162

6263
var UNITS = Object.keys(INTERVALS);
6364

64-
function deprecated(value, previous, current) {
65-
if (value !== undefined) {
66-
console.warn(
67-
'time scale: "' + previous + '" is deprecated. ' +
68-
'Please use "' + current + '" instead');
69-
}
70-
}
71-
7265
function sorter(a, b) {
7366
return a - b;
7467
}
@@ -460,9 +453,9 @@ module.exports = Scale.extend({
460453
var adapter = me._adapter = new adapters._date(options.adapters.date);
461454

462455
// DEPRECATIONS: output a message only one time per update
463-
deprecated(time.format, 'time.format', 'time.parser');
464-
deprecated(time.min, 'time.min', 'ticks.min');
465-
deprecated(time.max, 'time.max', 'ticks.max');
456+
deprecated('time scale', time.format, 'time.format', 'time.parser');
457+
deprecated('time scale', time.min, 'time.min', 'ticks.min');
458+
deprecated('time scale', time.max, 'time.max', 'ticks.max');
466459

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

0 commit comments

Comments
 (0)