Skip to content

Commit

Permalink
Improve offsetGridLine behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
nagix committed Jul 27, 2017
1 parent 56cad49 commit 524629d
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 155 deletions.
10 changes: 1 addition & 9 deletions docs/charts/bar.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,9 @@ The bar chart defines the following configuration options. These options are mer
| `barThickness` | `Number` | | Manually set width of each bar in pixels. If not set, the bars are sized automatically using `barPercentage` and `categoryPercentage`;
| `maxBarThickness` | `Number` | | Set this to ensure that the automatically sized bars are not sized thicker than this. Only works if `barThickness` is not set (automatic sizing is enabled).
| `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 sample width. If false, the grid line will go right down the middle of the bars. [more...](#offsetgridlines)
| `sampleSize` | `Number/Object/String` | | Manually set width for each sample. The unit and the data type vary between scales. If not set, the sample size will be the available width (the space between the gridlines for small datasets). Only works if `barThickness` is not set (automatic sizing is enabled). [more...](#samplesize)

### 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 sample width, which is the space between the gridlines by default but can be customized using `sampleSize`. If false, the grid line will go right down the middle of the bars. This is set to true for a bar chart while false for other charts by default.
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 sample width, which is the space between the bars. If false, the grid line will go right down the middle of the bars. This is set to true for a bar chart while false for other charts by default.

This setting applies to the axis configuration. If axes are added to the chart, this setting will need to be set for each new axis.

Expand All @@ -117,13 +116,6 @@ options = {
}
```

### sampleSize
If specified, the width for each sample will be calculated based on this value. If not specified, it will be the available width (the space between the gridlines). This only works if `barThickness` is not set.

The unit and data type vary between scales.
* category scale: This has no effect
* time scale: `Number/Object/String` that [`moment.duration()`](https://momentjs.com/docs/#/durations/) accepts

## Default Options

It is common to want to apply a configuration setting to all created bar charts. The global bar chart settings are stored in `Chart.defaults.bar`. Changing the global options only affects charts created after the change. Existing charts are not changed.
Expand Down
15 changes: 8 additions & 7 deletions src/controllers/controller.bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,16 +275,17 @@ module.exports = function(Chart) {
var me = this;
var options = scale.options;
var stackIndex = me.getStackIndex(datasetIndex);
var barPixels = scale.getBarPixelsForIndex(index, datasetIndex);
var base = barPixels.min;
var sampleSize = barPixels.max - barPixels.min;
var pos = scale.getPixelForValue(null, index, datasetIndex);
var offset = scale.getPixelForValue(null, index, datasetIndex, true) - pos;
var base = options.gridLines.offsetGridLines ? pos : pos - offset;
var sampleSize = offset * 2;
var categorySize = sampleSize * options.categoryPercentage;
var categorySpacing = sampleSize - categorySize;
var fullBarSize = categorySize / me.getStackCount();
var barSize = fullBarSize * options.barPercentage;
var barSpacing = fullBarSize - barSize;

barSize = Math.min(
var size = Math.min(
helpers.valueOrDefault(options.barThickness, barSize),
helpers.valueOrDefault(options.maxBarThickness, Infinity));

Expand All @@ -293,10 +294,10 @@ module.exports = function(Chart) {
base += barSpacing / 2;

return {
size: barSize,
size: size,
base: base,
head: base + barSize,
center: base + barSize / 2
head: base + size,
center: base + size / 2
};
},

Expand Down
3 changes: 1 addition & 2 deletions src/controllers/controller.line.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,7 @@ module.exports = function(Chart) {
var xScale = me.getScaleForId(meta.xAxisID);
var pointOptions = me.chart.options.elements.point;
var x, y;
var labels = me.chart.data.labels || [];
var includeOffset = (labels.length === 1 || dataset.data.length === 1) || xScale.options.gridLines.offsetGridLines;
var includeOffset = xScale.options.gridLines.offsetGridLines;

// Compatibility: If the properties are defined with only the old name, use those values
if ((dataset.radius !== undefined) && (dataset.pointRadius === undefined)) {
Expand Down
13 changes: 2 additions & 11 deletions src/core/core.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ module.exports = function(Chart) {
var me = this;
var types = [];
var newControllers = [];
var i, ilen;

helpers.each(me.data.datasets, function(dataset, datasetIndex) {
var meta = me.getDatasetMeta(datasetIndex);
Expand All @@ -308,17 +309,7 @@ module.exports = function(Chart) {
}
}, me);

// isCombo is no longer used, but left for backwards compatibility
if (types.length > 1) {
for (var i = 1; i < types.length; i++) {
if (types[i] !== types[i - 1]) {
me.isCombo = true;
break;
}
}
}

for (i = 0; i < types.length; i++) {
for (i = 0, ilen = types.length; i < ilen; i++) {
if (types[i] === 'bar' || types[i] === 'horizontalBar') {
me.hasBar = true;
break;
Expand Down
13 changes: 0 additions & 13 deletions src/core/core.scale.js
Original file line number Diff line number Diff line change
Expand Up @@ -515,19 +515,6 @@ module.exports = function(Chart) {
0;
},

// Get the bar pixels for the data at the given index
getBarPixelsForIndex: function(index, datasetIndex) {
var me = this;
var base = me.getPixelForValue(null, index, datasetIndex, me.options.gridLines.offsetGridLines);
var fullSize = me.isHorizontal() ? me.width : me.height;
var size = fullSize / Math.max(me.ticks.length, 1);

return {
min: base - size / 2,
max: base + size / 2
};
},

// Actually draw the scale on the canvas
// @param {rectangle} chartArea : the area of the chart to draw full grid lines on
draw: function(chartArea) {
Expand Down
41 changes: 27 additions & 14 deletions src/scales/scale.category.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,13 @@ module.exports = function(Chart) {
// If we are viewing some subset of labels, slice the original array
me.ticks = (me.minIndex === 0 && me.maxIndex === labels.length - 1) ? labels : labels.slice(me.minIndex, me.maxIndex + 1);

// Disable zero line if a chart has bars and offsetGridLines is false
if (me.chart.hasBar && !me.options.gridLines.offsetGridLines) {
// Offset ticks if a chart has bars and offsetGridLines is false
if ((labels.length === 1 || me.chart.hasBar) && !me.options.gridLines.offsetGridLines) {
me.tickOffset = true;
me.zeroLineIndex = -1;
} else {
me.tickOffset = false;
me.zeroLineIndex = undefined;
}
},

Expand All @@ -67,10 +71,8 @@ module.exports = function(Chart) {
// Used to get data value locations. Value can either be an index or a numerical value
getPixelForValue: function(value, index, datasetIndex, includeOffset) {
var me = this;
var offsetGridLines = me.options.gridLines.offsetGridLines;
var hasBar = me.chart.hasBar;
// 1 is added because we need the length but we have the indexes
var offsetAmt = Math.max((me.maxIndex + 1 - me.minIndex - ((offsetGridLines || hasBar) ? 0 : 1)), 1);
var offsetAmt = Math.max((me.maxIndex + 1 - me.minIndex - ((me.options.gridLines.offsetGridLines || me.chart.hasBar) ? 0 : 1)), 1);

// If value is a data object, then index is the index in the data array,
// not the index of the scale. We need to change that.
Expand All @@ -89,8 +91,13 @@ module.exports = function(Chart) {
var valueWidth = me.width / offsetAmt;
var widthOffset = (valueWidth * (index - me.minIndex));

// Add extra offset regardless of includeOffset when offsetGridLines is false and the chart has bars
if (includeOffset || (!offsetGridLines && hasBar)) {
// Add extra offset
if (includeOffset) {
widthOffset += (valueWidth / 2);
}

// Add extra offset when the chart has bars and offsetGridLines is false
if (me.tickOffset) {
widthOffset += (valueWidth / 2);
}

Expand All @@ -99,8 +106,13 @@ module.exports = function(Chart) {
var valueHeight = me.height / offsetAmt;
var heightOffset = (valueHeight * (index - me.minIndex));

// Add extra offset regardless of includeOffset when offsetGridLines is false and the chart has bars
if (includeOffset || (!offsetGridLines && hasBar)) {
// Add extra offset
if (includeOffset) {
heightOffset += (valueHeight / 2);
}

// Add extra offset when the chart has bars and offsetGridLines is false
if (me.tickOffset) {
heightOffset += (valueHeight / 2);
}

Expand All @@ -111,23 +123,24 @@ module.exports = function(Chart) {
},
getValueForPixel: function(pixel) {
var me = this;
var offsetGridLines = me.options.gridLines.offsetGridLines;
var hasBar = me.chart.hasBar;
var value;
var offsetAmt = Math.max((me.ticks.length - ((offsetGridLines || hasBar) ? 0 : 1)), 1);
var offsetAmt = Math.max((me.ticks.length - ((me.options.gridLines.offsetGridLines || me.chart.hasBar) ? 0 : 1)), 1);
var horz = me.isHorizontal();
var valueDimension = (horz ? me.width : me.height) / offsetAmt;

pixel -= horz ? me.left : me.top;

if (offsetGridLines || hasBar) {
if (me.tickOffset) {
pixel -= (valueDimension / 2);
}
if (me.options.gridLines.offsetGridLines) {
pixel -= (valueDimension / 2);
}

if (pixel <= 0) {
value = 0;
} else {
value = Math.round(pixel / valueDimension);
value = Math.round((pixel + 0.5) / valueDimension);
}

return value + me.minIndex;
Expand Down
Loading

0 comments on commit 524629d

Please sign in to comment.