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

Author: nagix

docs/axes/labelling.md

@@ -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.

src/core/core.scale.js

@@ -99,6 +99,24 @@ function computeTextSize(context, tick, font) {
 		context.measureText(tick).width;
 }
 
+function parseFontOptions(options, nestedOpts) {
+	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
@@ -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]);
 	},
@@ -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;
@@ -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);
@@ -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.
@@ -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;
@@ -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);
 
@@ -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
@@ -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;
 
@@ -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);

src/scales/scale.time.js

@@ -678,11 +678,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([
+			nestedTickOpts.callback,
+			nestedTickOpts.userCallback,
+			tickOpts.callback,
+			tickOpts.userCallback
+		]);
 
 		return formatter ? formatter(label, index, ticks) : label;
 	},

test/specs/global.deprecations.tests.js

@@ -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'],

test/specs/scale.time.tests.js

@@ -616,6 +616,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',