From 85c59c5070e59d4bebcfa4d1a7add18413301808 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Fri, 28 Jul 2017 20:41:46 +0200 Subject: [PATCH] Change `ticks.mode` to `scale.distribution` Fix `ticks.mode` behavior when `ticks.source` is `auto`: the lookup table is now built from the data and not from the ticks, so data (and ticks) are correctly distributed along the scale. Rename the option to `distribution` (more explicit than `mode`) and since this option applies from now on the data, it seems better to have it under `scale` instead `scale.ticks`. --- src/scales/scale.time.js | 128 ++++++++++++++++++++------------- test/specs/scale.time.tests.js | 72 ++++++++----------- 2 files changed, 107 insertions(+), 93 deletions(-) diff --git a/src/scales/scale.time.js b/src/scales/scale.time.js index dbd389a3509..4a07df496b1 100644 --- a/src/scales/scale.time.js +++ b/src/scales/scale.time.js @@ -64,6 +64,22 @@ function sorter(a, b) { return a - b; } +function arrayUnique(items) { + var hash = {}; + var out = []; + var i, ilen, item; + + for (i = 0, ilen = items.length; i < ilen; ++i) { + item = items[i]; + if (!hash[item]) { + hash[item] = true; + out.push(item); + } + } + + return out; +} + /** * Returns an array of {time, pos} objects used to interpolate a specific `time` or position * (`pos`) on the scale, by searching entries before and after the requested value. `pos` is @@ -73,14 +89,14 @@ function sorter(a, b) { * to create the lookup table. The table ALWAYS contains at least two items: min and max. * * @param {Number[]} timestamps - timestamps sorted from lowest to highest. - * @param {Boolean} linear - If true, timestamps will be spread linearly along the min/max - * range, so basically, the table will contains only two items: {min, 0} and {max, 1}. If - * false, timestamps will be positioned at the same distance from each other. In this case, - * only timestamps that break the time linearity are registered, meaning that in the best - * case, all timestamps are linear, the table contains only min and max. + * @param {String} distribution - If 'linear', timestamps will be spread linearly along the min + * and max range, so basically, the table will contains only two items: {min, 0} and {max, 1}. + * If 'series', timestamps will be positioned at the same distance from each other. In this + * case, only timestamps that break the time linearity are registered, meaning that in the + * best case, all timestamps are linear, the table contains only min and max. */ -function buildLookupTable(timestamps, min, max, linear) { - if (linear || !timestamps.length) { +function buildLookupTable(timestamps, min, max, distribution) { + if (distribution === 'linear' || !timestamps.length) { return [ {time: min, pos: 0}, {time: max, pos: 1} @@ -88,16 +104,18 @@ function buildLookupTable(timestamps, min, max, linear) { } var table = []; - var items = timestamps.slice(0); + var items = [min]; var i, ilen, prev, curr, next; - if (min < timestamps[0]) { - items.unshift(min); - } - if (max > timestamps[timestamps.length - 1]) { - items.push(max); + for (i = 0, ilen = timestamps.length; i < ilen; ++i) { + curr = timestamps[i]; + if (curr > min && curr < max) { + items.push(curr); + } } + items.push(max); + for (i = 0, ilen = items.length; i < ilen; ++i) { next = items[i + 1]; prev = items[i - 1]; @@ -334,6 +352,15 @@ module.exports = function(Chart) { var defaultConfig = { position: 'bottom', + /** + * Data distribution along the scale: + * - 'linear': data are spread according to their time (distances can vary), + * - 'series': data are spread at the same distance from each other. + * @see https://github.com/chartjs/Chart.js/pull/4507 + * @since 2.7.0 + */ + distribution: 'linear', + time: { parser: false, // false == a pattern string from http://momentjs.com/docs/#/parsing/string-format/ or a custom callback that converts its argument to a moment format: false, // DEPRECATED false == date objects, moment object, callback or a pattern string from http://momentjs.com/docs/#/parsing/string-format/ @@ -359,15 +386,6 @@ module.exports = function(Chart) { ticks: { autoSkip: false, - /** - * Ticks distribution along the scale: - * - 'linear': ticks and data are spread according to their time (distances can vary), - * - 'series': ticks and data are spread at the same distance from each other. - * @see https://github.com/chartjs/Chart.js/pull/4507 - * @since 2.7.0 - */ - mode: 'linear', - /** * Ticks generation input values: * - 'auto': generates "optimal" ticks based on scale size and time options. @@ -430,44 +448,54 @@ module.exports = function(Chart) { var me = this; var chart = me.chart; var options = me.options; - var datasets = chart.data.datasets || []; var min = parse(options.time.min, me) || MAX_INTEGER; var max = parse(options.time.max, me) || MIN_INTEGER; var timestamps = []; + var datasets = []; var labels = []; var i, j, ilen, jlen, data, timestamp; // Convert labels to timestamps for (i = 0, ilen = chart.data.labels.length; i < ilen; ++i) { - timestamp = parse(chart.data.labels[i], me); - min = Math.min(min, timestamp); - max = Math.max(max, timestamp); - labels.push(timestamp); + labels.push(parse(chart.data.labels[i], me)); } // Convert data to timestamps - for (i = 0, ilen = datasets.length; i < ilen; ++i) { + for (i = 0, ilen = (chart.data.datasets || []).length; i < ilen; ++i) { if (chart.isDatasetVisible(i)) { - data = datasets[i].data; + data = chart.data.datasets[i].data; // Let's consider that all data have the same format. if (helpers.isObject(data[0])) { - timestamps[i] = []; + datasets[i] = []; for (j = 0, jlen = data.length; j < jlen; ++j) { timestamp = parse(data[j], me); - min = Math.min(min, timestamp); - max = Math.max(max, timestamp); - timestamps[i][j] = timestamp; + timestamps.push(timestamp); + datasets[i][j] = timestamp; } } else { - timestamps[i] = labels.slice(0); + timestamps.push.apply(timestamps, labels); + datasets[i] = labels.slice(0); } } else { - timestamps[i] = []; + datasets[i] = []; } } + if (labels.length) { + // Sort labels **after** data have been converted + labels = arrayUnique(labels).sort(sorter); + min = Math.min(min, labels[0]); + max = Math.max(max, labels[labels.length - 1]); + } + + if (timestamps.length) { + timestamps = arrayUnique(timestamps).sort(sorter); + min = Math.min(min, timestamps[0]); + max = Math.max(max, timestamps[timestamps.length - 1]); + } + // In case there is no valid min/max, let's use today limits min = min === MAX_INTEGER ? +moment().startOf('day') : min; max = max === MIN_INTEGER ? +moment().endOf('day') + 1 : max; @@ -477,36 +505,36 @@ module.exports = function(Chart) { me.max = Math.max(min + 1, max); // PRIVATE - me._datasets = timestamps; me._horizontal = me.isHorizontal(); - me._labels = labels.sort(sorter); // Sort labels **after** data have been converted me._table = []; + me._timestamps = { + data: timestamps, + datasets: datasets, + labels: labels + }; }, buildTicks: function() { var me = this; var min = me.min; var max = me.max; - var timeOpts = me.options.time; - var ticksOpts = me.options.ticks; + var options = me.options; + var timeOpts = options.time; + var ticksOpts = options.ticks; var formats = timeOpts.displayFormats; var capacity = me.getLabelCapacity(min); var unit = timeOpts.unit || determineUnit(timeOpts.minUnit, min, max, capacity); var majorUnit = determineMajorUnit(unit); var timestamps = []; var ticks = []; - var hash = {}; var i, ilen, timestamp; switch (ticksOpts.source) { case 'data': - for (i = 0, ilen = me._datasets.length; i < ilen; ++i) { - timestamps.push.apply(timestamps, me._datasets[i]); - } - timestamps.sort(sorter); + timestamps = me._timestamps.data; break; case 'labels': - timestamps = me._labels; + timestamps = me._timestamps.labels; break; case 'auto': default: @@ -522,12 +550,10 @@ module.exports = function(Chart) { min = parse(timeOpts.min, me) || min; max = parse(timeOpts.max, me) || max; - // Remove ticks outside the min/max range and duplicated entries + // Remove ticks outside the min/max range for (i = 0, ilen = timestamps.length; i < ilen; ++i) { timestamp = timestamps[i]; - if (timestamp >= min && timestamp <= max && !hash[timestamp]) { - // hash is used to efficiently detect timestamp duplicates - hash[timestamp] = true; + if (timestamp >= min && timestamp <= max) { ticks.push(timestamp); } } @@ -540,7 +566,7 @@ module.exports = function(Chart) { me._majorUnit = majorUnit; me._minorFormat = formats[unit]; me._majorFormat = formats[majorUnit]; - me._table = buildLookupTable(ticks, min, max, ticksOpts.mode === 'linear'); + me._table = buildLookupTable(me._timestamps.data, min, max, options.distribution); return ticksFromTimestamps(ticks, majorUnit); }, @@ -609,7 +635,7 @@ module.exports = function(Chart) { var time = null; if (index !== undefined && datasetIndex !== undefined) { - time = me._datasets[datasetIndex][index]; + time = me._timestamps.datasets[datasetIndex][index]; } if (time === null) { diff --git a/test/specs/scale.time.tests.js b/test/specs/scale.time.tests.js index 940d83513d4..91dc218008b 100755 --- a/test/specs/scale.time.tests.js +++ b/test/specs/scale.time.tests.js @@ -21,12 +21,6 @@ describe('Time scale tests', function() { return scale.ticks; } - function fetchTickPositions(scale) { - return scale.ticks.map(function(tick, index) { - return scale.getPixelForTick(index); - }); - } - beforeEach(function() { // Need a time matcher for getValueFromPixel jasmine.addMatchers({ @@ -83,12 +77,12 @@ describe('Time scale tests', function() { labelString: '', lineHeight: 1.2 }, + distribution: 'linear', ticks: { beginAtZero: false, minRotation: 0, maxRotation: 50, mirror: false, - mode: 'linear', source: 'auto', bounds: 'data', padding: 0, @@ -803,7 +797,7 @@ describe('Time scale tests', function() { }); }); - describe('when ticks.mode', function() { + describe('when distribution', function() { describe('is "series"', function() { beforeEach(function() { this.chart = window.acquireChart({ @@ -820,8 +814,8 @@ describe('Time scale tests', function() { time: { parser: 'YYYY' }, + distribution: 'series', ticks: { - mode: 'series', source: 'labels' } }], @@ -833,19 +827,18 @@ describe('Time scale tests', function() { }); }); - it ('should space ticks out with the same gap, whatever their time values', function() { + it ('should space data out with the same gap, whatever their time values', function() { var scale = this.chart.scales.x; var start = scale.left; var slice = scale.width / 4; - var pixels = fetchTickPositions(scale); - expect(pixels[0]).toBeCloseToPixel(start); - expect(pixels[1]).toBeCloseToPixel(start + slice); - expect(pixels[2]).toBeCloseToPixel(start + slice * 2); - expect(pixels[3]).toBeCloseToPixel(start + slice * 3); - expect(pixels[4]).toBeCloseToPixel(start + slice * 4); + expect(scale.getPixelForValue('2017')).toBeCloseToPixel(start); + expect(scale.getPixelForValue('2019')).toBeCloseToPixel(start + slice); + expect(scale.getPixelForValue('2020')).toBeCloseToPixel(start + slice * 2); + expect(scale.getPixelForValue('2025')).toBeCloseToPixel(start + slice * 3); + expect(scale.getPixelForValue('2042')).toBeCloseToPixel(start + slice * 4); }); - it ('should add a step before if scale.min is before the first tick', function() { + it ('should add a step before if scale.min is before the first data', function() { var chart = this.chart; var scale = chart.scales.x; var options = chart.options.scales.xAxes[0]; @@ -855,12 +848,11 @@ describe('Time scale tests', function() { var start = scale.left; var slice = scale.width / 5; - var pixels = fetchTickPositions(scale); - expect(pixels[0]).toBeCloseToPixel(start + slice); - expect(pixels[4]).toBeCloseToPixel(start + slice * 5); + expect(scale.getPixelForValue('2017')).toBeCloseToPixel(start + slice); + expect(scale.getPixelForValue('2042')).toBeCloseToPixel(start + slice * 5); }); - it ('should add a step after if scale.max is after the last tick', function() { + it ('should add a step after if scale.max is after the last data', function() { var chart = this.chart; var scale = chart.scales.x; var options = chart.options.scales.xAxes[0]; @@ -870,12 +862,11 @@ describe('Time scale tests', function() { var start = scale.left; var slice = scale.width / 5; - var pixels = fetchTickPositions(scale); - expect(pixels[0]).toBeCloseToPixel(start); - expect(pixels[4]).toBeCloseToPixel(start + slice * 4); + expect(scale.getPixelForValue('2017')).toBeCloseToPixel(start); + expect(scale.getPixelForValue('2042')).toBeCloseToPixel(start + slice * 4); }); - it ('should add steps before and after if scale.min/max are outside the labels range', function() { + it ('should add steps before and after if scale.min/max are outside the data range', function() { var chart = this.chart; var scale = chart.scales.x; var options = chart.options.scales.xAxes[0]; @@ -886,10 +877,9 @@ describe('Time scale tests', function() { var start = scale.left; var slice = scale.width / 6; - var pixels = fetchTickPositions(scale); - expect(pixels[0]).toBeCloseToPixel(start + slice); - expect(pixels[4]).toBeCloseToPixel(start + slice * 5); + expect(scale.getPixelForValue('2017')).toBeCloseToPixel(start + slice); + expect(scale.getPixelForValue('2042')).toBeCloseToPixel(start + slice * 5); }); }); describe('is "linear"', function() { @@ -908,8 +898,8 @@ describe('Time scale tests', function() { time: { parser: 'YYYY' }, + distribution: 'linear', ticks: { - mode: 'linear', source: 'labels' } }], @@ -921,17 +911,16 @@ describe('Time scale tests', function() { }); }); - it ('should space ticks out with a gap relative to their time values', function() { + it ('should space data out with a gap relative to their time values', function() { var scale = this.chart.scales.x; var start = scale.left; var slice = scale.width / (2042 - 2017); - var pixels = fetchTickPositions(scale); - expect(pixels[0]).toBeCloseToPixel(start); - expect(pixels[1]).toBeCloseToPixel(start + slice * (2019 - 2017)); - expect(pixels[2]).toBeCloseToPixel(start + slice * (2020 - 2017)); - expect(pixels[3]).toBeCloseToPixel(start + slice * (2025 - 2017)); - expect(pixels[4]).toBeCloseToPixel(start + slice * (2042 - 2017)); + expect(scale.getPixelForValue('2017')).toBeCloseToPixel(start); + expect(scale.getPixelForValue('2019')).toBeCloseToPixel(start + slice * (2019 - 2017)); + expect(scale.getPixelForValue('2020')).toBeCloseToPixel(start + slice * (2020 - 2017)); + expect(scale.getPixelForValue('2025')).toBeCloseToPixel(start + slice * (2025 - 2017)); + expect(scale.getPixelForValue('2042')).toBeCloseToPixel(start + slice * (2042 - 2017)); }); it ('should take in account scale min and max if outside the ticks range', function() { var chart = this.chart; @@ -944,13 +933,12 @@ describe('Time scale tests', function() { var start = scale.left; var slice = scale.width / (2050 - 2012); - var pixels = fetchTickPositions(scale); - expect(pixels[0]).toBeCloseToPixel(start + slice * (2017 - 2012)); - expect(pixels[1]).toBeCloseToPixel(start + slice * (2019 - 2012)); - expect(pixels[2]).toBeCloseToPixel(start + slice * (2020 - 2012)); - expect(pixels[3]).toBeCloseToPixel(start + slice * (2025 - 2012)); - expect(pixels[4]).toBeCloseToPixel(start + slice * (2042 - 2012)); + expect(scale.getPixelForValue('2017')).toBeCloseToPixel(start + slice * (2017 - 2012)); + expect(scale.getPixelForValue('2019')).toBeCloseToPixel(start + slice * (2019 - 2012)); + expect(scale.getPixelForValue('2020')).toBeCloseToPixel(start + slice * (2020 - 2012)); + expect(scale.getPixelForValue('2025')).toBeCloseToPixel(start + slice * (2025 - 2012)); + expect(scale.getPixelForValue('2042')).toBeCloseToPixel(start + slice * (2042 - 2012)); }); }); });