Skip to content

Commit

Permalink
Use datetime as default time scale tooltip format (#6019)
Browse files Browse the repository at this point in the history
Remove the logic that computed an "optimal" tooltip format. Instead, always fallback to the `datetime` adapter format which is more efficient and stable. Additionally, remove the adapter `presets` API, which is not needed anymore.
  • Loading branch information
benmccann authored and simonbrunel committed Feb 18, 2019
1 parent 32aeeac commit 3e18708
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 104 deletions.
11 changes: 1 addition & 10 deletions src/adapters/adapter.moment.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ var adapter = require('../core/core.adapters')._date;
var helpers = require('../helpers/helpers.core');

var FORMATS = {
datetime: 'MMM D, YYYY, h:mm:ss a',
millisecond: 'h:mm:ss.SSS a',
second: 'h:mm:ss a',
minute: 'h:mm a',
Expand All @@ -18,23 +19,13 @@ var FORMATS = {
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

formats: function() {
return FORMATS;
},

presets: function() {
return PRESETS;
},

parse: function(value, format) {
if (typeof value === 'string' && typeof format === 'string') {
value = moment(value, format);
Expand Down
12 changes: 2 additions & 10 deletions src/core/core.adapters.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,12 @@ function abstract() {
/** @lends Chart._adapters._date */
module.exports._date = {
/**
* Returns a map of time formats for the supported units.
* Returns a map of time formats for the supported formatting units defined
* in Unit as well as 'datetime' representing a detailed date/time string.
* @returns {{string: string}}
*/
formats: abstract,

/**
* Returns a map of date/time formats for the following presets:
* 'full': date + time + millisecond
* 'time': date + time
* 'date': date
* @returns {{string: string}}
*/
presets: abstract,

/**
* Parses the given `value` and return the associated timestamp.
* @param {any} value - the value to parse (usually comes from the data)
Expand Down
27 changes: 1 addition & 26 deletions src/scales/scale.time.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,29 +405,6 @@ function ticksFromTimestamps(values, majorUnit) {
return ticks;
}

/**
* Return the time format for the label with the most parts (milliseconds, second, etc.)
*/
function determineLabelFormat(timestamps) {
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.full;
}
if (!hasTime && adapter.startOf(ts, 'day') !== ts) {
hasTime = true;
}
}
if (hasTime) {
return presets.time;
}
return presets.date;
}

var defaultConfig = {
position: 'bottom',

Expand Down Expand Up @@ -637,7 +614,6 @@ 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);

if (options.ticks.reverse) {
ticks.reverse();
Expand All @@ -662,8 +638,7 @@ module.exports = Scale.extend({
if (typeof label === 'string') {
return label;
}

return adapter.format(toTimestamp(label, timeOpts), me._labelFormat);
return adapter.format(toTimestamp(label, timeOpts), timeOpts.displayFormats.datetime);
},

/**
Expand Down
62 changes: 4 additions & 58 deletions test/specs/scale.time.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ describe('Time scale tests', function() {
expect(xScale.getLabelForIndex(0, 0)).toBe('2015-01-01T20:00:00');
});

it('should get the correct label for a timestamp with milliseconds', function() {
it('should get the correct label for a timestamp', function() {
var chart = window.acquireChart({
type: 'line',
data: {
Expand All @@ -624,63 +624,7 @@ describe('Time scale tests', function() {

var xScale = chart.scales.xScale0;
var label = xScale.getLabelForIndex(0, 0);
expect(label).toEqual('Jan 8, 2018 5:14:23.234 am');
});

it('should get the correct label for a timestamp with time', function() {
var chart = window.acquireChart({
type: 'line',
data: {
datasets: [{
xAxisID: 'xScale0',
data: [
{t: +new Date('2018-01-08 05:14:23'), y: 10},
{t: +new Date('2018-01-09 06:17:43'), y: 3}
]
}],
},
options: {
scales: {
xAxes: [{
id: 'xScale0',
type: 'time',
position: 'bottom'
}],
}
}
});

var xScale = chart.scales.xScale0;
var label = xScale.getLabelForIndex(0, 0);
expect(label).toEqual('Jan 8, 2018 5:14:23 am');
});

it('should get the correct label for a timestamp representing a date', function() {
var chart = window.acquireChart({
type: 'line',
data: {
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}
]
}],
},
options: {
scales: {
xAxes: [{
id: 'xScale0',
type: 'time',
position: 'bottom'
}],
}
}
});

var xScale = chart.scales.xScale0;
var label = xScale.getLabelForIndex(0, 0);
expect(label).toEqual('Jan 8, 2018');
expect(label).toEqual('Jan 8, 2018, 5:14:23 am');
});

it('should get the correct pixel for only one data in the dataset', function() {
Expand Down Expand Up @@ -1532,6 +1476,7 @@ describe('Time scale tests', function() {

// NOTE: built-in adapter uses moment
var expected = {
datetime: 'MMM D, YYYY, h:mm:ss a',
millisecond: 'h:mm:ss.SSS a',
second: 'h:mm:ss a',
minute: 'h:mm a',
Expand Down Expand Up @@ -1570,6 +1515,7 @@ describe('Time scale tests', function() {

// NOTE: built-in adapter uses moment
var expected = {
datetime: 'MMM D, YYYY, h:mm:ss a',
millisecond: 'foo',
second: 'h:mm:ss a',
minute: 'h:mm a',
Expand Down

0 comments on commit 3e18708

Please sign in to comment.