Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ticks.minor and ticks.major configuration issues #6102

Merged
merged 5 commits into from
Apr 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/axes/labelling.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,5 @@ var chart = new Chart(ctx, {
}
});
```

The third parameter passed to the callback function is an array of labels, but in the time scale, it is an array of `{label: string, major: boolean}` objects.
79 changes: 40 additions & 39 deletions src/core/core.scale.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,24 @@ function computeTextSize(context, tick, font) {
context.measureText(tick).width;
}

function parseFontOptions(options, nestedOpts) {
nagix marked this conversation as resolved.
Show resolved Hide resolved
return helpers.extend(helpers.options._parseFont({
fontFamily: valueOrDefault(nestedOpts.fontFamily, options.fontFamily),
fontSize: valueOrDefault(nestedOpts.fontSize, options.fontSize),
fontStyle: valueOrDefault(nestedOpts.fontStyle, options.fontStyle),
lineHeight: valueOrDefault(nestedOpts.lineHeight, options.lineHeight)
}), {
color: helpers.options.resolve([nestedOpts.fontColor, options.fontColor, defaults.global.defaultFontColor])
});
}

function parseTickFontOptions(options) {
var minor = parseFontOptions(options, options.minor);
var major = options.major.enabled ? parseFontOptions(options, options.major) : minor;

return {minor: minor, major: major};
}

module.exports = Element.extend({
/**
* Get the padding needed for the scale
Expand Down Expand Up @@ -128,29 +146,16 @@ module.exports = Element.extend({
// Any function defined here is inherited by all scale types.
// Any function can be extended by the scale type

/**
* Provided for backward compatibility, not available anymore
* @function Chart.Scale.mergeTicksOptions
* @deprecated since version 2.8.0
* @todo remove at version 3
*/
mergeTicksOptions: function() {
var ticks = this.options.ticks;
if (ticks.minor === false) {
ticks.minor = {
display: false
};
}
if (ticks.major === false) {
ticks.major = {
display: false
};
}
for (var key in ticks) {
if (key !== 'major' && key !== 'minor') {
if (typeof ticks.minor[key] === 'undefined') {
ticks.minor[key] = ticks[key];
}
if (typeof ticks.major[key] === 'undefined') {
ticks.major[key] = ticks[key];
}
}
}
// noop
},

beforeUpdate: function() {
helpers.callback(this.options.beforeUpdate, [this]);
},
Expand Down Expand Up @@ -628,7 +633,7 @@ module.exports = Element.extend({
_autoSkip: function(ticks) {
var me = this;
var isHorizontal = me.isHorizontal();
var optionTicks = me.options.ticks.minor;
var optionTicks = me.options.ticks;
var tickCount = ticks.length;
var skipRatio = false;
var maxTicks = optionTicks.maxTicksLimit;
Expand Down Expand Up @@ -673,7 +678,7 @@ module.exports = Element.extend({
_tickSize: function() {
var me = this;
var isHorizontal = me.isHorizontal();
var optionTicks = me.options.ticks.minor;
var optionTicks = me.options.ticks;

// Calculate space needed by label in axis direction.
var rot = helpers.toRadians(me.labelRotation);
Expand All @@ -683,7 +688,7 @@ module.exports = Element.extend({
var padding = optionTicks.autoSkipPadding || 0;
var w = (me.longestLabelWidth + padding) || 0;

var tickFont = helpers.options._parseFont(optionTicks);
var tickFont = parseTickFontOptions(optionTicks).minor;
var h = (me._maxLabelLines * tickFont.lineHeight + padding) || 0;

// Calculate space needed for 1 tick in axis direction.
Expand Down Expand Up @@ -732,10 +737,7 @@ module.exports = Element.extend({

var chart = me.chart;
var context = me.ctx;
var globalDefaults = defaults.global;
var defaultFontColor = globalDefaults.defaultFontColor;
var optionTicks = options.ticks.minor;
var optionMajorTicks = options.ticks.major || optionTicks;
var optionTicks = options.ticks;
var gridLines = options.gridLines;
var scaleLabel = options.scaleLabel;
var position = options.position;
Expand All @@ -744,20 +746,15 @@ module.exports = Element.extend({
var isMirrored = optionTicks.mirror;
var isHorizontal = me.isHorizontal();

var parseFont = helpers.options._parseFont;
var ticks = optionTicks.display && optionTicks.autoSkip ? me._autoSkip(me.getTicks()) : me.getTicks();
var tickFontColor = valueOrDefault(optionTicks.fontColor, defaultFontColor);
var tickFont = parseFont(optionTicks);
var lineHeight = tickFont.lineHeight;
var majorTickFontColor = valueOrDefault(optionMajorTicks.fontColor, defaultFontColor);
var majorTickFont = parseFont(optionMajorTicks);
var tickFonts = parseTickFontOptions(optionTicks);
var tickPadding = optionTicks.padding;
var labelOffset = optionTicks.labelOffset;

var tl = gridLines.drawTicks ? gridLines.tickMarkLength : 0;

var scaleLabelFontColor = valueOrDefault(scaleLabel.fontColor, defaultFontColor);
var scaleLabelFont = parseFont(scaleLabel);
var scaleLabelFontColor = valueOrDefault(scaleLabel.fontColor, defaults.global.defaultFontColor);
var scaleLabelFont = helpers.options._parseFont(scaleLabel);
var scaleLabelPadding = helpers.options.toPadding(scaleLabel.padding);
var labelRotationRadians = helpers.toRadians(me.labelRotation);

Expand Down Expand Up @@ -794,6 +791,8 @@ module.exports = Element.extend({
}

var label = tick.label;
var tickFont = tick.major ? tickFonts.major : tickFonts.minor;
var lineHeight = tickFont.lineHeight;
var lineWidth, lineColor, borderDash, borderDashOffset;
if (index === me.zeroLineIndex && options.offset === gridLines.offsetGridLines) {
// Draw the first index specially
Expand Down Expand Up @@ -918,12 +917,14 @@ module.exports = Element.extend({
}

if (optionTicks.display) {
var tickFont = itemToDraw.major ? tickFonts.major : tickFonts.minor;

// Make sure we draw text in the correct color and font
context.save();
context.translate(itemToDraw.labelX, itemToDraw.labelY);
context.rotate(itemToDraw.rotation);
context.font = itemToDraw.major ? majorTickFont.string : tickFont.string;
context.fillStyle = itemToDraw.major ? majorTickFontColor : tickFontColor;
context.font = tickFont.string;
context.fillStyle = tickFont.color;
context.textBaseline = 'middle';
context.textAlign = itemToDraw.textAlign;

Expand All @@ -933,7 +934,7 @@ module.exports = Element.extend({
for (var i = 0; i < label.length; ++i) {
// We just make sure the multiline element is a string here..
context.fillText('' + label[i], 0, y);
y += lineHeight;
y += tickFont.lineHeight;
}
} else {
context.fillText(label, 0, y);
Expand Down
12 changes: 9 additions & 3 deletions src/scales/scale.time.js
Original file line number Diff line number Diff line change
Expand Up @@ -662,11 +662,17 @@ module.exports = Scale.extend({
var majorUnit = me._majorUnit;
var majorFormat = formats[majorUnit];
var majorTime = +adapter.startOf(time, majorUnit);
var majorTickOpts = options.ticks.major;
var tickOpts = options.ticks;
var majorTickOpts = tickOpts.major;
var major = majorTickOpts.enabled && majorUnit && majorFormat && time === majorTime;
var label = adapter.format(time, format ? format : major ? majorFormat : minorFormat);
var tickOpts = major ? majorTickOpts : options.ticks.minor;
var formatter = valueOrDefault(tickOpts.callback, tickOpts.userCallback);
var nestedTickOpts = major ? majorTickOpts : tickOpts.minor;
var formatter = helpers.options.resolve([
simonbrunel marked this conversation as resolved.
Show resolved Hide resolved
nestedTickOpts.callback,
nestedTickOpts.userCallback,
tickOpts.callback,
tickOpts.userCallback
]);

return formatter ? formatter(label, index, ticks) : label;
},
Expand Down
8 changes: 8 additions & 0 deletions test/specs/global.deprecations.tests.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
describe('Deprecations', function() {
describe('Version 2.9.0', function() {
describe('Chart.Scale.mergeTicksOptions', function() {
it('should be defined as a function', function() {
expect(typeof Chart.Scale.prototype.mergeTicksOptions).toBe('function');
});
});
});

describe('Version 2.8.0', function() {
[
['Bar', 'bar'],
Expand Down
125 changes: 125 additions & 0 deletions test/specs/scale.time.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,131 @@ describe('Time scale tests', function() {
expect(xScale.getLabelForIndex(6, 0)).toBe('2015-01-10T12:00');
});

describe('when ticks.callback is specified', function() {
beforeEach(function() {
this.chart = window.acquireChart({
type: 'line',
data: {
datasets: [{
xAxisID: 'xScale0',
data: [0, 0]
}],
labels: ['2015-01-01T20:00:00', '2015-01-01T20:01:00']
},
options: {
scales: {
xAxes: [{
id: 'xScale0',
type: 'time',
ticks: {
callback: function(value) {
return '<' + value + '>';
}
}
}]
}
}
});
this.scale = this.chart.scales.xScale0;
});

it('should get the correct labels for ticks', function() {
var scale = this.scale;

expect(scale._ticks.map(function(tick) {
return tick.major;
})).toEqual([true, false, false, false, false, false, true]);
expect(scale.ticks).toEqual(['<8:00:00 pm>', '<8:00:10 pm>', '<8:00:20 pm>', '<8:00:30 pm>', '<8:00:40 pm>', '<8:00:50 pm>', '<8:01:00 pm>']);
});

it('should update ticks.callback correctly', function() {
var chart = this.chart;
var scale = this.scale;

chart.options.scales.xAxes[0].ticks.callback = function(value) {
return '{' + value + '}';
};
chart.update();
expect(scale.ticks).toEqual(['{8:00:00 pm}', '{8:00:10 pm}', '{8:00:20 pm}', '{8:00:30 pm}', '{8:00:40 pm}', '{8:00:50 pm}', '{8:01:00 pm}']);
});
});

describe('when ticks.major.callback and ticks.minor.callback are specified', function() {
beforeEach(function() {
this.chart = window.acquireChart({
type: 'line',
data: {
datasets: [{
xAxisID: 'xScale0',
data: [0, 0]
}],
labels: ['2015-01-01T20:00:00', '2015-01-01T20:01:00']
},
options: {
scales: {
xAxes: [{
id: 'xScale0',
type: 'time',
ticks: {
callback: function(value) {
return '<' + value + '>';
},
major: {
enabled: true,
callback: function(value) {
return '[' + value + ']';
}
},
minor: {
callback: function(value) {
return '(' + value + ')';
}
}
}
}]
}
}
});
this.scale = this.chart.scales.xScale0;
});

it('should get the correct labels for major and minor ticks', function() {
var scale = this.scale;

expect(scale._ticks.map(function(tick) {
return tick.major;
})).toEqual([true, false, false, false, false, false, true]);
expect(scale.ticks).toEqual(['[8:00 pm]', '(8:00:10 pm)', '(8:00:20 pm)', '(8:00:30 pm)', '(8:00:40 pm)', '(8:00:50 pm)', '[8:01 pm]']);
});

it('should only use ticks.minor callback if ticks.major.enabled is false', function() {
var chart = this.chart;
var scale = this.scale;

chart.options.scales.xAxes[0].ticks.major.enabled = false;
chart.update();
expect(scale.ticks).toEqual(['(8:00:00 pm)', '(8:00:10 pm)', '(8:00:20 pm)', '(8:00:30 pm)', '(8:00:40 pm)', '(8:00:50 pm)', '(8:01:00 pm)']);
});

it('should use ticks.callback if ticks.major.callback is omitted', function() {
var chart = this.chart;
var scale = this.scale;

chart.options.scales.xAxes[0].ticks.major.callback = undefined;
chart.update();
expect(scale.ticks).toEqual(['<8:00 pm>', '(8:00:10 pm)', '(8:00:20 pm)', '(8:00:30 pm)', '(8:00:40 pm)', '(8:00:50 pm)', '<8:01 pm>']);
});

it('should use ticks.callback if ticks.minor.callback is omitted', function() {
var chart = this.chart;
var scale = this.scale;

chart.options.scales.xAxes[0].ticks.minor.callback = undefined;
chart.update();
expect(scale.ticks).toEqual(['[8:00 pm]', '<8:00:10 pm>', '<8:00:20 pm>', '<8:00:30 pm>', '<8:00:40 pm>', '<8:00:50 pm>', '[8:01 pm]']);
});
});

it('should get the correct label when time is specified as a string', function() {
var chart = window.acquireChart({
type: 'line',
Expand Down