From 7322d75177fc18f28121f8d21862853d9bf2f6bf Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Sat, 5 Jan 2019 14:52:11 +0100 Subject: [PATCH 1/7] Implement adapter to abstract date/time features --- src/adapters/adapter.moment.js | 84 ++++++++++++++++++ src/adapters/index.js | 10 +++ src/chart.js | 4 + src/core/core.adapters.js | 107 ++++++++++++++++++++++ src/scales/scale.time.js | 156 +++++++++++++++------------------ test/specs/scale.time.tests.js | 89 ++++++++++++++++--- 6 files changed, 354 insertions(+), 96 deletions(-) create mode 100644 src/adapters/adapter.moment.js create mode 100644 src/adapters/index.js create mode 100644 src/core/core.adapters.js diff --git a/src/adapters/adapter.moment.js b/src/adapters/adapter.moment.js new file mode 100644 index 00000000000..38f7e58d516 --- /dev/null +++ b/src/adapters/adapter.moment.js @@ -0,0 +1,84 @@ +// TODO v3 - make this adapter external (chartjs-adapter-moment) + +'use strict'; + +var moment = require('moment'); +var adapter = require('../core/core.adapters')._date; +var helpers = require('../helpers/helpers.core'); + +var SHORT_PRESETS = { + millisecond: 'h:mm:ss.SSS a', + second: 'h:mm:ss a', + minute: 'h:mm a', + hour: 'hA', + day: 'MMM D', + week: 'll', + month: 'MMM YYYY', + quarter: '[Q]Q - YYYY', + year: 'YYYY' +}; + +var LONG_PRESETS = { + millisecond: 'MMM D, YYYY h:mm:ss.SSS a', + second: 'MMM D, YYYY h:mm:ss a', + minute: 'MMM D, YYYY h:mm a', + hour: 'MMM D, YYYY hA', + day: 'MMM D, YYYY', + week: 'll', + month: 'MMM YYYY', + quarter: '[Q]Q - YYYY', + year: 'YYYY' +}; + +helpers.merge(adapter, moment ? { + _id: 'moment', // DEBUG ONLY + + presets: function(long) { + return long ? LONG_PRESETS : SHORT_PRESETS; + }, + + parse: function(value, format) { + if (typeof value === 'string' && typeof format === 'string') { + value = moment(value, format); + } else if (!(value instanceof moment)) { + value = moment(value); + } + return value.isValid() ? +value : null; + }, + + format: function(time, format) { + return moment(time).format(format); + }, + + add: function(time, amount, unit) { + return +moment(time).add(amount, unit); + }, + + diff: function(max, min, unit) { + return moment.duration(moment(max).diff(moment(min))).as(unit); + }, + + startOf: function(time, unit, opt) { + time = moment(time); + if (unit === 'isoWeek') { + return +time.isoWeekday(opt); + } + return +time.startOf(unit); + }, + + endOf: function(time, unit) { + return +moment(time).endOf(unit); + }, + + // DEPRECATIONS + + /** + * Provided for backward compatibility with scale.getValueForPixel(). + * @deprecated since version 2.8.0 + * @todo remove at version 3 + * @private + */ + _create: function(time) { + return moment(time); + }, +} : {}); diff --git a/src/adapters/index.js b/src/adapters/index.js new file mode 100644 index 00000000000..d072f85cdf6 --- /dev/null +++ b/src/adapters/index.js @@ -0,0 +1,10 @@ +'use strict'; + +// ----------------------------------------------------------------------------- +// IMPORTANT: do NOT submit new adapters to this repository, instead +// create an external library named `chartjs-adapter-{lib-name}` +// ----------------------------------------------------------------------------- + +// Built-in moment adapter that we need to keep for backward compatibility +// https://github.com/chartjs/Chart.js/issues/5542 +require('./adapter.moment'); diff --git a/src/chart.js b/src/chart.js index d74a6d82efb..9de4113ee40 100644 --- a/src/chart.js +++ b/src/chart.js @@ -8,6 +8,7 @@ Chart.helpers = require('./helpers/index'); // @todo dispatch these helpers into appropriated helpers/helpers.* file and write unit tests! require('./core/core.helpers')(Chart); +Chart._adapters = require('./core/core.adapters'); Chart.Animation = require('./core/core.animation'); Chart.animationService = require('./core/core.animations'); Chart.controllers = require('./controllers/index'); @@ -30,6 +31,9 @@ Chart.helpers.each(scales, function(scale, type) { Chart.scaleService.registerScaleType(type, scale, scale._defaults); }); +// Built-in adapters (loaded for side effects) +require('./adapters'); + // Loading built-in plugins var plugins = require('./plugins'); for (var k in plugins) { diff --git a/src/core/core.adapters.js b/src/core/core.adapters.js new file mode 100644 index 00000000000..afb016e6115 --- /dev/null +++ b/src/core/core.adapters.js @@ -0,0 +1,107 @@ +/** + * @namespace Chart._adapters + * @since 2.8.0 + * @private + */ + +'use strict'; + +function abstract() { + throw new Error( + 'This method is not implemented: either no adapter ' + + 'can be found or provide an incomplete integration.' + ); +} + +/** + * Date adapter (current used by the time scale) + * @namespace Chart._adapters._date + * @memberof Chart._adapters + * @private + */ + +/** + * Currently supported unit string values. + * @typedef {('millisecond'|'second'|'minute'|'hour'|'day'|'week'|'month'|'quarter'|'year')} + * @memberof Chart._adapters._date + * @name Unit + * */ + +/** @lends Chart._adapters._date */ +module.exports._date = { + /** + * Returns a map of time formats for the supported units. + * @param {boolean} [long] - query for long formats. + * @returns {{string: string}} + */ + presets: function() { + return {}; + }, + + /** + * Parses the given `value` and return the associated timestamp. + * @param {any} value - the value to parse (usually comes from the data) + * @param {string} [format] - the expected data format + * @returns {(number|null)} + * @function + */ + parse: abstract, + + /** + * Returns the formatted date in the specified `format` for a given `timestamp`. + * @param {number} timestamp - the timestamp to format + * @param {string} format - the date/time token + * @return {string} + * @function + */ + format: abstract, + + /** + * Adds the specified `amount` of `unit` to the given `timestamp`. + * @param {number} timestamp - the input timestamp + * @param {number} amount - the amount to add + * @param {Unit} unit - the unit as string + * @return {number} + * @function + */ + add: abstract, + + /** + * Returns the number of `unit` between the given timestamps. + * @param {number} max - the input timestamp (reference) + * @param {number} min - the timestamp to substract + * @param {Unit} unit - the unit as string + * @return {number} + * @function + */ + diff: abstract, + + /** + * Returns start of `unit` for the given `timestamp`. + * @param {number} timestamp - the input timestamp + * @param {Unit} unit - the unit as string + * @function + */ + startOf: abstract, + + /** + * Returns end of `unit` for the given `timestamp`. + * @param {number} timestamp - the input timestamp + * @param {Unit} unit - the unit as string + * @function + */ + endOf: abstract, + + // DEPRECATIONS + + /** + * Provided for backward compatibility for scale.getValueForPixel(), + * this method should be overridden only by the moment adapter. + * @deprecated since version 2.8.0 + * @todo remove at version 3 + * @private + */ + _create: function(value) { + return value; + } +}; diff --git a/src/scales/scale.time.js b/src/scales/scale.time.js index 356e8a1968d..08f54739fc3 100644 --- a/src/scales/scale.time.js +++ b/src/scales/scale.time.js @@ -1,7 +1,7 @@ /* global window: false */ 'use strict'; -var moment = require('moment'); +var adapter = require('../core/core.adapters')._date; var defaults = require('../core/core.defaults'); var helpers = require('../helpers/index'); var Scale = require('../core/core.scale'); @@ -178,34 +178,33 @@ function interpolate(table, skey, sval, tkey) { return prev[tkey] + offset; } -/** - * Convert the given value to a moment object using the given time options. - * @see https://momentjs.com/docs/#/parsing/ - */ -function momentify(value, options) { +function toTimestamp(input, options) { var parser = options.parser; - var format = options.parser || options.format; + var format = parser || options.format; + var value = input; if (typeof parser === 'function') { - return parser(value); + value = parser(value); } - if (typeof value === 'string' && typeof format === 'string') { - return moment(value, format); + // Only parse if its not a timestamp already + if (!helpers.isFinite(value)) { + value = adapter.parse(value, format); } - if (!(value instanceof moment)) { - value = moment(value); + if (value !== null) { + return +value; } - if (value.isValid()) { - return value; - } + // Labels are in an incompatible format and no `parser` has been provided. + // The user might still use the deprecated `format` option for parsing. + if (!parser && typeof format === 'function') { + value = format(input); - // Labels are in an incompatible moment format and no `parser` has been provided. - // The user might still use the deprecated `format` option to convert his inputs. - if (typeof format === 'function') { - return format(value); + // `format` could return something else than a timestamp, if so, parse it + if (!helpers.isFinite(value)) { + value = adapter.parse(value, format); + } } return value; @@ -217,16 +216,16 @@ function parse(input, scale) { } var options = scale.options.time; - var value = momentify(scale.getRightValue(input), options); - if (!value.isValid()) { - return null; + var value = toTimestamp(scale.getRightValue(input), options); + if (value === null) { + return value; } if (options.round) { - value.startOf(options.round); + value = +adapter.startOf(value, options.round); } - return value.valueOf(); + return value; } /** @@ -277,13 +276,12 @@ function determineUnitForAutoTicks(minUnit, min, max, capacity) { * Figures out what unit to format a set of ticks with */ function determineUnitForFormatting(ticks, minUnit, min, max) { - var duration = moment.duration(moment(max).diff(moment(min))); var ilen = UNITS.length; var i, unit; for (i = ilen - 1; i >= UNITS.indexOf(minUnit); i--) { unit = UNITS[i]; - if (INTERVALS[unit].common && duration.as(unit) >= ticks.length) { + if (INTERVALS[unit].common && adapter.diff(max, min, unit) >= ticks.length) { return unit; } } @@ -313,8 +311,8 @@ function generate(min, max, capacity, options) { var weekday = minor === 'week' ? timeOpts.isoWeekday : false; var majorTicksEnabled = options.ticks.major.enabled; var interval = INTERVALS[minor]; - var first = moment(min); - var last = moment(max); + var first = min; + var last = max; var ticks = []; var time; @@ -324,30 +322,30 @@ function generate(min, max, capacity, options) { // For 'week' unit, handle the first day of week option if (weekday) { - first = first.isoWeekday(weekday); - last = last.isoWeekday(weekday); + first = +adapter.startOf(first, 'isoWeek', weekday); + last = +adapter.startOf(last, 'isoWeek', weekday); } // Align first/last ticks on unit - first = first.startOf(weekday ? 'day' : minor); - last = last.startOf(weekday ? 'day' : minor); + first = +adapter.startOf(first, weekday ? 'day' : minor); + last = +adapter.startOf(last, weekday ? 'day' : minor); // Make sure that the last tick include max if (last < max) { - last.add(1, minor); + last = +adapter.add(last, 1, minor); } - time = moment(first); + time = first; if (majorTicksEnabled && major && !weekday && !timeOpts.round) { // Align the first tick on the previous `minor` unit aligned on the `major` unit: // we first aligned time on the previous `major` unit then add the number of full // stepSize there is between first and the previous major time. - time.startOf(major); - time.add(~~((first - time) / (interval.size * stepSize)) * stepSize, minor); + time = +adapter.startOf(time, major); + time = +adapter.add(time, ~~((first - time) / (interval.size * stepSize)) * stepSize, minor); } - for (; time < last; time.add(stepSize, minor)) { + for (; time < last; time = +adapter.add(time, stepSize, minor)) { ticks.push(+time); } @@ -395,7 +393,7 @@ function ticksFromTimestamps(values, majorUnit) { for (i = 0, ilen = values.length; i < ilen; ++i) { value = values[i]; - major = majorUnit ? value === +moment(value).startOf(majorUnit) : false; + major = majorUnit ? value === +adapter.startOf(value, majorUnit) : false; ticks.push({ value: value, @@ -406,25 +404,27 @@ function ticksFromTimestamps(values, majorUnit) { return ticks; } -function determineLabelFormat(data, timeOpts) { - var i, momentDate, hasTime; - var ilen = data.length; +/** + * Return the time format for the label with the most parts (milliseconds, second, etc.) + */ +function determineLabelFormat(timestamps) { + var presets = adapter.presets(true); + var ilen = timestamps.length; + var i, ts, hasTime; - // find the label with the most parts (milliseconds, minutes, etc.) - // format all labels with the same level of detail as the most specific label for (i = 0; i < ilen; i++) { - momentDate = momentify(data[i], timeOpts); - if (momentDate.millisecond() !== 0) { - return 'MMM D, YYYY h:mm:ss.SSS a'; + ts = timestamps[i]; + if (ts % INTERVALS.second.size !== 0) { + return presets.millisecond; } - if (momentDate.second() !== 0 || momentDate.minute() !== 0 || momentDate.hour() !== 0) { + if (ts % INTERVALS.day.size !== 0) { hasTime = true; } } if (hasTime) { - return 'MMM D, YYYY h:mm:ss a'; + return presets.second; } - return 'MMM D, YYYY'; + return presets.day; } var defaultConfig = { @@ -456,19 +456,7 @@ var defaultConfig = { displayFormat: false, // DEPRECATED isoWeekday: false, // override week start day - see https://momentjs.com/docs/#/get-set/iso-weekday/ minUnit: 'millisecond', - - // defaults to unit's corresponding unitFormat below or override using pattern string from https://momentjs.com/docs/#/displaying/format/ - displayFormats: { - millisecond: 'h:mm:ss.SSS a', // 11:20:01.123 AM, - second: 'h:mm:ss a', // 11:20:01 AM - minute: 'h:mm a', // 11:20 AM - hour: 'hA', // 5PM - day: 'MMM D', // Sep 4 - week: 'll', // Week 46, or maybe "[W]WW - YYYY" ? - month: 'MMM YYYY', // Sept 2015 - quarter: '[Q]Q - YYYY', // Q3 - year: 'YYYY' // 2015 - }, + displayFormats: {} }, ticks: { autoSkip: false, @@ -491,24 +479,26 @@ var defaultConfig = { module.exports = Scale.extend({ initialize: function() { - if (!moment) { - throw new Error('Chart.js - Moment.js could not be found! You must include it before Chart.js to use the time scale. Download at https://momentjs.com'); - } - this.mergeTicksOptions(); - Scale.prototype.initialize.call(this); }, update: function() { var me = this; var options = me.options; + var time = options.time || (options.time = {}); // DEPRECATIONS: output a message only one time per update - if (options.time && options.time.format) { + if (time.format) { console.warn('options.time.format is deprecated and replaced by options.time.parser.'); } + // Backward compatibility: before introducing adapter, `displayFormats` was + // supposed to contain *all* unit/string pairs but this can't be resolved + // when loading the scale (adapters are loaded afterward), so let's populate + // missing formats on update + helpers.mergeIf(time.displayFormats, adapter.presets()); + return Scale.prototype.update.apply(me, arguments); }, @@ -582,8 +572,8 @@ module.exports = Scale.extend({ max = parse(timeOpts.max, me) || max; // In case there is no valid min/max, set limits based on unit time option - min = min === MAX_INTEGER ? +moment().startOf(unit) : min; - max = max === MIN_INTEGER ? +moment().endOf(unit) + 1 : max; + min = min === MAX_INTEGER ? +adapter.startOf(+new Date(), unit) : min; + max = max === MIN_INTEGER ? +adapter.endOf(+new Date(), unit) + 1 : max; // Make sure that max is strictly higher than min (required by the lookup table) me.min = Math.min(min, max); @@ -646,7 +636,7 @@ module.exports = Scale.extend({ me._majorUnit = determineMajorUnit(me._unit); me._table = buildLookupTable(me._timestamps.data, min, max, options.distribution); me._offsets = computeOffsets(me._table, ticks, min, max, options); - me._labelFormat = determineLabelFormat(me._timestamps.data, timeOpts); + me._labelFormat = determineLabelFormat(me._timestamps.data); if (options.ticks.reverse) { ticks.reverse(); @@ -666,31 +656,30 @@ module.exports = Scale.extend({ label = me.getRightValue(value); } if (timeOpts.tooltipFormat) { - return momentify(label, timeOpts).format(timeOpts.tooltipFormat); + return adapter.format(toTimestamp(label, timeOpts), timeOpts.tooltipFormat); } if (typeof label === 'string') { return label; } - return momentify(label, timeOpts).format(me._labelFormat); + return adapter.format(toTimestamp(label, timeOpts), me._labelFormat); }, /** * Function to format an individual tick mark * @private */ - tickFormatFunction: function(tick, index, ticks, formatOverride) { + tickFormatFunction: function(time, index, ticks, format) { var me = this; var options = me.options; - var time = tick.valueOf(); var formats = options.time.displayFormats; var minorFormat = formats[me._unit]; var majorUnit = me._majorUnit; var majorFormat = formats[majorUnit]; - var majorTime = tick.clone().startOf(majorUnit).valueOf(); + var majorTime = +adapter.startOf(time, majorUnit); var majorTickOpts = options.ticks.major; var major = majorTickOpts.enabled && majorUnit && majorFormat && time === majorTime; - var label = tick.format(formatOverride ? formatOverride : major ? majorFormat : minorFormat); + var label = adapter.format(time, format ? format : major ? majorFormat : minorFormat); var tickOpts = major ? majorTickOpts : options.ticks.minor; var formatter = valueOrDefault(tickOpts.callback, tickOpts.userCallback); @@ -702,7 +691,7 @@ module.exports = Scale.extend({ var i, ilen; for (i = 0, ilen = ticks.length; i < ilen; ++i) { - labels.push(this.tickFormatFunction(moment(ticks[i].value), i, ticks)); + labels.push(this.tickFormatFunction(ticks[i].value, i, ticks)); } return labels; @@ -753,7 +742,8 @@ module.exports = Scale.extend({ var pos = (size ? (pixel - start) / size : 0) * (me._offsets.start + 1 + me._offsets.start) - me._offsets.end; var time = interpolate(me._table, 'pos', pos, 'time'); - return moment(time); + // DEPRECATION, we should return time directly + return adapter._create(time); }, /** @@ -778,13 +768,13 @@ module.exports = Scale.extend({ getLabelCapacity: function(exampleTime) { var me = this; - var formatOverride = me.options.time.displayFormats.millisecond; // Pick the longest format for guestimation - - var exampleLabel = me.tickFormatFunction(moment(exampleTime), 0, [], formatOverride); + // pick the longest format (milliseconds) for guestimation + var format = me.options.time.displayFormats.millisecond; + var exampleLabel = me.tickFormatFunction(exampleTime, 0, [], format); var tickLabelWidth = me.getLabelWidth(exampleLabel); var innerWidth = me.isHorizontal() ? me.width : me.height; - var capacity = Math.floor(innerWidth / tickLabelWidth); + return capacity > 0 ? capacity : 1; } }); diff --git a/test/specs/scale.time.tests.js b/test/specs/scale.time.tests.js index c0fd1bda93b..192ec8229c3 100755 --- a/test/specs/scale.time.tests.js +++ b/test/specs/scale.time.tests.js @@ -102,17 +102,7 @@ describe('Time scale tests', function() { isoWeekday: false, displayFormat: false, minUnit: 'millisecond', - displayFormats: { - millisecond: 'h:mm:ss.SSS a', // 11:20:01.123 AM - second: 'h:mm:ss a', // 11:20:01 AM - minute: 'h:mm a', // 11:20 AM - hour: 'hA', // 5PM - day: 'MMM D', // Sep 4 - week: 'll', // Week 46, or maybe "[W]WW - YYYY" ? - month: 'MMM YYYY', // Sept 2015 - quarter: '[Q]Q - YYYY', // Q3 - year: 'YYYY' // 2015 - }, + displayFormats: {} } }); @@ -672,8 +662,8 @@ describe('Time scale tests', function() { datasets: [{ xAxisID: 'xScale0', data: [ - {t: +new Date('2018-01-08 00:00:00'), y: 10}, - {t: +new Date('2018-01-09 00:00:00'), y: 3} + {t: +new Date('2018-01-08 00:00:00Z'), y: 10}, + {t: +new Date('2018-01-09 00:00:00Z'), y: 3} ] }], }, @@ -1523,4 +1513,77 @@ describe('Time scale tests', function() { }); }); }); + + describe('Deprecations', function() { + describe('options.time.displayFormats', function() { + it('should generate defaults from adapter presets', function() { + var chart = window.acquireChart({ + type: 'line', + data: {}, + options: { + scales: { + xAxes: [{ + id: 'x', + type: 'time' + }] + } + } + }); + + // NOTE: built-in adapter uses moment + var expected = { + millisecond: 'h:mm:ss.SSS a', + second: 'h:mm:ss a', + minute: 'h:mm a', + hour: 'hA', + day: 'MMM D', + week: 'll', + month: 'MMM YYYY', + quarter: '[Q]Q - YYYY', + year: 'YYYY' + }; + + expect(chart.scales.x.options.time.displayFormats).toEqual(expected); + expect(chart.options.scales.xAxes[0].time.displayFormats).toEqual(expected); + }); + + it('should merge user formats with adapter presets', function() { + var chart = window.acquireChart({ + type: 'line', + data: {}, + options: { + scales: { + xAxes: [{ + id: 'x', + type: 'time', + time: { + displayFormats: { + millisecond: 'foo', + hour: 'bar', + month: 'bla' + } + } + }] + } + } + }); + + // NOTE: built-in adapter uses moment + var expected = { + millisecond: 'foo', + second: 'h:mm:ss a', + minute: 'h:mm a', + hour: 'bar', + day: 'MMM D', + week: 'll', + month: 'bla', + quarter: '[Q]Q - YYYY', + year: 'YYYY' + }; + + expect(chart.scales.x.options.time.displayFormats).toEqual(expected); + expect(chart.options.scales.xAxes[0].time.displayFormats).toEqual(expected); + }); + }); + }); }); From f1610490837ab045cbb3c37411acf6876eb798a8 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Mon, 7 Jan 2019 12:03:54 +0100 Subject: [PATCH 2/7] Fix format usage in `toTimestamp` --- src/scales/scale.time.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/scales/scale.time.js b/src/scales/scale.time.js index 08f54739fc3..6ed217b5e18 100644 --- a/src/scales/scale.time.js +++ b/src/scales/scale.time.js @@ -189,7 +189,9 @@ function toTimestamp(input, options) { // Only parse if its not a timestamp already if (!helpers.isFinite(value)) { - value = adapter.parse(value, format); + value = typeof format === 'string' + ? adapter.parse(value, format) + : adapter.parse(value); } if (value !== null) { @@ -203,7 +205,7 @@ function toTimestamp(input, options) { // `format` could return something else than a timestamp, if so, parse it if (!helpers.isFinite(value)) { - value = adapter.parse(value, format); + value = adapter.parse(value); } } From 524f76547fe699138e7b1b4ec1bd876875f873b6 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Mon, 7 Jan 2019 12:08:06 +0100 Subject: [PATCH 3/7] Fix `abstract` exception message --- src/core/core.adapters.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/core.adapters.js b/src/core/core.adapters.js index afb016e6115..c1f7b50e0d7 100644 --- a/src/core/core.adapters.js +++ b/src/core/core.adapters.js @@ -8,8 +8,8 @@ function abstract() { throw new Error( - 'This method is not implemented: either no adapter ' + - 'can be found or provide an incomplete integration.' + 'This method is not implemented: either no adapter can ' + + 'be found or an incomplete integration was provided.' ); } From 81d71fb2222976eea9d8f07d65b2c51e97b87332 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Wed, 9 Jan 2019 20:28:12 +0100 Subject: [PATCH 4/7] Replace SHORT/LONG_PRESETS by formats() and presets() --- src/adapters/adapter.moment.js | 24 +++++++++++------------- src/core/core.adapters.js | 14 ++++++++++++-- src/scales/scale.time.js | 10 +++++----- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/adapters/adapter.moment.js b/src/adapters/adapter.moment.js index 38f7e58d516..045cfcd8c04 100644 --- a/src/adapters/adapter.moment.js +++ b/src/adapters/adapter.moment.js @@ -6,7 +6,7 @@ var moment = require('moment'); var adapter = require('../core/core.adapters')._date; var helpers = require('../helpers/helpers.core'); -var SHORT_PRESETS = { +var FORMATS = { millisecond: 'h:mm:ss.SSS a', second: 'h:mm:ss a', minute: 'h:mm a', @@ -18,23 +18,21 @@ var SHORT_PRESETS = { year: 'YYYY' }; -var LONG_PRESETS = { - millisecond: 'MMM D, YYYY h:mm:ss.SSS a', - second: 'MMM D, YYYY h:mm:ss a', - minute: 'MMM D, YYYY h:mm a', - hour: 'MMM D, YYYY hA', - day: 'MMM D, YYYY', - week: 'll', - month: 'MMM YYYY', - quarter: '[Q]Q - YYYY', - year: 'YYYY' +var PRESETS = { + full: 'MMM D, YYYY h:mm:ss.SSS a', + time: 'MMM D, YYYY h:mm:ss a', + date: 'MMM D, YYYY' }; helpers.merge(adapter, moment ? { _id: 'moment', // DEBUG ONLY - presets: function(long) { - return long ? LONG_PRESETS : SHORT_PRESETS; + formats: function() { + return FORMATS; + }, + + presets: function() { + return PRESETS; }, parse: function(value, format) { diff --git a/src/core/core.adapters.js b/src/core/core.adapters.js index c1f7b50e0d7..9798b7e0d62 100644 --- a/src/core/core.adapters.js +++ b/src/core/core.adapters.js @@ -25,13 +25,23 @@ function abstract() { * @typedef {('millisecond'|'second'|'minute'|'hour'|'day'|'week'|'month'|'quarter'|'year')} * @memberof Chart._adapters._date * @name Unit - * */ + */ /** @lends Chart._adapters._date */ module.exports._date = { /** * Returns a map of time formats for the supported units. - * @param {boolean} [long] - query for long formats. + * @returns {{string: string}} + */ + formats: function() { + return {}; + }, + + /** + * Returns a map of date/time formats for the following presets: + * 'full': date + time + millisecond + * 'time': date + time + * 'date': date * @returns {{string: string}} */ presets: function() { diff --git a/src/scales/scale.time.js b/src/scales/scale.time.js index 6ed217b5e18..96ecff48817 100644 --- a/src/scales/scale.time.js +++ b/src/scales/scale.time.js @@ -410,23 +410,23 @@ function ticksFromTimestamps(values, majorUnit) { * Return the time format for the label with the most parts (milliseconds, second, etc.) */ function determineLabelFormat(timestamps) { - var presets = adapter.presets(true); + var presets = adapter.presets(); var ilen = timestamps.length; var i, ts, hasTime; for (i = 0; i < ilen; i++) { ts = timestamps[i]; if (ts % INTERVALS.second.size !== 0) { - return presets.millisecond; + return presets.full; } if (ts % INTERVALS.day.size !== 0) { hasTime = true; } } if (hasTime) { - return presets.second; + return presets.time; } - return presets.day; + return presets.date; } var defaultConfig = { @@ -499,7 +499,7 @@ module.exports = Scale.extend({ // supposed to contain *all* unit/string pairs but this can't be resolved // when loading the scale (adapters are loaded afterward), so let's populate // missing formats on update - helpers.mergeIf(time.displayFormats, adapter.presets()); + helpers.mergeIf(time.displayFormats, adapter.formats()); return Scale.prototype.update.apply(me, arguments); }, From 52a77344d9ad5eb96508ffbad9b7213c94f5c820 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Wed, 9 Jan 2019 20:30:12 +0100 Subject: [PATCH 5/7] Fix detection of tooltip fallback format Use `adapter.startOf(ts, 'day') !== ts` instead of introducing a new adapter API to detect when the timestamp is midnight in the current timezone. --- src/scales/scale.time.js | 2 +- test/specs/scale.time.tests.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/scales/scale.time.js b/src/scales/scale.time.js index 96ecff48817..6cf481b4ce2 100644 --- a/src/scales/scale.time.js +++ b/src/scales/scale.time.js @@ -419,7 +419,7 @@ function determineLabelFormat(timestamps) { if (ts % INTERVALS.second.size !== 0) { return presets.full; } - if (ts % INTERVALS.day.size !== 0) { + if (!hasTime && adapter.startOf(ts, 'day') !== ts) { hasTime = true; } } diff --git a/test/specs/scale.time.tests.js b/test/specs/scale.time.tests.js index 192ec8229c3..c5be682d95d 100755 --- a/test/specs/scale.time.tests.js +++ b/test/specs/scale.time.tests.js @@ -662,8 +662,8 @@ describe('Time scale tests', function() { datasets: [{ xAxisID: 'xScale0', data: [ - {t: +new Date('2018-01-08 00:00:00Z'), y: 10}, - {t: +new Date('2018-01-09 00:00:00Z'), y: 3} + {t: +new Date('2018-01-08 00:00:00'), y: 10}, + {t: +new Date('2018-01-09 00:00:00'), y: 3} ] }], }, From 3183b551634a06f027f4a78ab0ff02026c3fc122 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Wed, 9 Jan 2019 21:18:58 +0100 Subject: [PATCH 6/7] Make `format()` and `presets()` abstract --- src/core/core.adapters.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/core/core.adapters.js b/src/core/core.adapters.js index 9798b7e0d62..ddb45562d21 100644 --- a/src/core/core.adapters.js +++ b/src/core/core.adapters.js @@ -33,9 +33,7 @@ module.exports._date = { * Returns a map of time formats for the supported units. * @returns {{string: string}} */ - formats: function() { - return {}; - }, + formats: abstract, /** * Returns a map of date/time formats for the following presets: @@ -44,9 +42,7 @@ module.exports._date = { * 'date': date * @returns {{string: string}} */ - presets: function() { - return {}; - }, + presets: abstract, /** * Parses the given `value` and return the associated timestamp. From b76d6b4c896b06367dbb60cc8a75605d6d8bbd56 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Thu, 10 Jan 2019 12:03:47 +0100 Subject: [PATCH 7/7] Better argument naming and comments --- src/adapters/adapter.moment.js | 4 ++-- src/chart.js | 2 +- src/core/core.adapters.js | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/adapters/adapter.moment.js b/src/adapters/adapter.moment.js index 045cfcd8c04..60124709986 100644 --- a/src/adapters/adapter.moment.js +++ b/src/adapters/adapter.moment.js @@ -56,10 +56,10 @@ helpers.merge(adapter, moment ? { return moment.duration(moment(max).diff(moment(min))).as(unit); }, - startOf: function(time, unit, opt) { + startOf: function(time, unit, weekday) { time = moment(time); if (unit === 'isoWeek') { - return +time.isoWeekday(opt); + return +time.isoWeekday(weekday); } return +time.startOf(unit); }, diff --git a/src/chart.js b/src/chart.js index 9de4113ee40..586dcc38295 100644 --- a/src/chart.js +++ b/src/chart.js @@ -31,7 +31,7 @@ Chart.helpers.each(scales, function(scale, type) { Chart.scaleService.registerScaleType(type, scale, scale._defaults); }); -// Built-in adapters (loaded for side effects) +// Load to register built-in adapters (as side effects) require('./adapters'); // Loading built-in plugins diff --git a/src/core/core.adapters.js b/src/core/core.adapters.js index ddb45562d21..5aa78e0765b 100644 --- a/src/core/core.adapters.js +++ b/src/core/core.adapters.js @@ -86,6 +86,8 @@ module.exports._date = { * Returns start of `unit` for the given `timestamp`. * @param {number} timestamp - the input timestamp * @param {Unit} unit - the unit as string + * @param {number} [weekday] - the ISO day of the week with 1 being Monday + * and 7 being Sunday (only needed if param *unit* is `isoWeek`). * @function */ startOf: abstract,