From f0781c7614e646f24d752c73c0d0ac9c03e6fd0f Mon Sep 17 00:00:00 2001 From: Akihiko Kusanagi Date: Tue, 2 Apr 2019 15:45:34 +0800 Subject: [PATCH] Fix ticks.minor and ticks.major configuration issues (#6102) --- docs/axes/labelling.md | 2 + src/core/core.scale.js | 79 +++++++-------- src/scales/scale.time.js | 12 ++- test/specs/global.deprecations.tests.js | 8 ++ test/specs/scale.time.tests.js | 125 ++++++++++++++++++++++++ 5 files changed, 184 insertions(+), 42 deletions(-) diff --git a/docs/axes/labelling.md b/docs/axes/labelling.md index 8f39235640f..96c5eae3784 100644 --- a/docs/axes/labelling.md +++ b/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. diff --git a/src/core/core.scale.js b/src/core/core.scale.js index 1f65bc564b6..196ca4a6843 100644 --- a/src/core/core.scale.js +++ b/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); diff --git a/src/scales/scale.time.js b/src/scales/scale.time.js index 69b9bb3d7b8..eebacacbff6 100644 --- a/src/scales/scale.time.js +++ b/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; }, diff --git a/test/specs/global.deprecations.tests.js b/test/specs/global.deprecations.tests.js index 437a316fe0b..b5509a3e221 100644 --- a/test/specs/global.deprecations.tests.js +++ b/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'], diff --git a/test/specs/scale.time.tests.js b/test/specs/scale.time.tests.js index 3312c68ca39..d67b60cbb12 100755 --- a/test/specs/scale.time.tests.js +++ b/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',