Fix labels for non-linearly sized units

Months, quarters and years have non-constant numbers of seconds. A scale that's linear WRT milliseconds produces incorrect tick labels due to the label formatting losing precision (eg year labels lose month and day so a label of 2016-12-32 displays as 2016 instead of 2017).
chartjs · Feb 16, 2017 · e5cd916 · e5cd916
1 parent a3d35fc
commit e5cd916
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 28 deletions.
diff --git a/src/scales/scale.time.js b/src/scales/scale.time.js
@@ -142,7 +142,7 @@ module.exports = function(Chart) {
 	 * @param min {Number} the scale minimum
 	 * @param max {Number} the scale maximum
 	 * @param unit {String} the unit determined by the {@see determineUnit} method
-	 * @return {Number} the axis step size in milliseconds
+	 * @return {Number} the axis step size as a multiple of unit
 	 */
 	function determineStepSize(min, max, unit) {
 		// Using our unit, figoure out what we need to scale as
@@ -165,7 +165,7 @@ module.exports = function(Chart) {
 			}
 		}
 
-		return unitSizeInMilliSeconds * multiplier;
+		return multiplier;
 	}
 
 	/**
@@ -176,47 +176,32 @@ module.exports = function(Chart) {
 	 */
 	Chart.Ticks.generators.time = function(generationOptions, dataRange) {
 		var ticks = [];
-		var spacing = generationOptions.stepSize;
-		var baseSpacing = generationOptions.baseSize;
+		var stepSize = generationOptions.stepSize;
 
 		var niceMin;
 		var niceMax;
 		var isoWeekday = generationOptions.isoWeekday;
 		if (generationOptions.unit === 'week' && isoWeekday !== false) {
 			niceMin = moment(dataRange.min).startOf('isoWeek').isoWeekday(isoWeekday).valueOf();
-			niceMax = moment(dataRange.max).startOf('isoWeek').isoWeekday(isoWeekday).valueOf();
+			niceMax = moment(dataRange.max).startOf('isoWeek').isoWeekday(isoWeekday);
 			if (dataRange.max - niceMax > 0) {
-				niceMax += baseSpacing;
+				niceMax.add(1, 'week');
 			}
+			niceMax = niceMax.valueOf();
 		} else {
 			niceMin = moment(dataRange.min).startOf(generationOptions.unit).valueOf();
-			niceMax = moment(dataRange.max).startOf(generationOptions.unit).valueOf();
+			niceMax = moment(dataRange.max).startOf(generationOptions.unit);
 			if (dataRange.max - niceMax > 0) {
-				niceMax += baseSpacing;
+				niceMax.add(1, generationOptions.unit);
 			}
-		}
-
-		// If min, max and stepSize is set and they make an evenly spaced scale use it.
-		if (generationOptions.min && generationOptions.max && generationOptions.stepSize) {
-			var minMaxDeltaDivisableByStepSize = ((generationOptions.max - generationOptions.min) % generationOptions.stepSize) === 0;
-			if (minMaxDeltaDivisableByStepSize) {
-				niceMin = generationOptions.min;
-				niceMax = generationOptions.max;
-			}
-		}
-
-		var numSpaces = (niceMax - niceMin) / spacing;
-		// If very close to our rounded value, use it.
-		if (helpers.almostEquals(numSpaces, Math.round(numSpaces), spacing / 1000)) {
-			numSpaces = Math.round(numSpaces);
-		} else {
-			numSpaces = Math.ceil(numSpaces);
+			niceMax = niceMax.valueOf();
 		}
 
 		// Put the values into the ticks array
 		ticks.push(generationOptions.min !== undefined ? generationOptions.min : niceMin);
-		for (var j = 1; j < numSpaces; ++j) {
-			ticks.push(niceMin + (j * spacing));
+		var cur = moment(niceMin);
+		while (cur.add(stepSize, generationOptions.unit).valueOf() < niceMax) {
+			ticks.push(cur.valueOf());
 		}
 		ticks.push(generationOptions.max !== undefined ? generationOptions.max : niceMax);
 
@@ -340,7 +325,6 @@ module.exports = function(Chart) {
 				max: maxTimestamp,
 				stepSize: stepSize,
 				unit: unit,
-				baseSize: time[unit].size,
 				isoWeekday: timeOpts.isoWeekday
 			};
 			var ticks = me.ticks = Chart.Ticks.generators.time(timeGeneratorOptions, {

diff --git a/test/scale.time.tests.js b/test/scale.time.tests.js
@@ -360,6 +360,56 @@ describe('Time scale tests', function() {
 		});
 	});
 
+	describe('when rendering several years', function() {
+		var chart = window.acquireChart({
+			type: 'line',
+			data: {
+				labels: ['2005-07-04', '2017-01-20'],
+			},
+			options: {
+				scales: {
+					xAxes: [{
+						id: 'xScale0',
+						type: 'time',
+						position: 'bottom'
+					}],
+				}
+			}
+		});
+
+		var xScale = chart.scales.xScale0;
+
+		it('should be bounded by nearest year starts', function() {
+			expect(xScale.getValueForPixel(xScale.left)).toBeCloseToTime({
+				value: moment(chart.data.labels[0]).startOf('year'),
+				unit: 'hour',
+			});
+			expect(xScale.getValueForPixel(xScale.right)).toBeCloseToTime({
+				value: moment(chart.data.labels[chart.data.labels - 1]).endOf('year'),
+				unit: 'hour',
+			});
+		});
+
+		it('should build the correct ticks', function() {
+			// Where 'correct' is a two year spacing, except the last tick which is the year end of the last point.
+			expect(xScale.ticks).toEqual(['2005', '2007', '2009', '2011', '2013', '2015', '2017', '2018']);
+		});
+
+		it('should have ticks with accurate labels', function() {
+			var ticks = xScale.ticks;
+			var pixelsPerYear = xScale.width / 13;
+
+			for (var i = 0; i < ticks.length - 1; i++) {
+				var offset = 2 * pixelsPerYear * i;
+				expect(xScale.getValueForPixel(xScale.left + offset)).toBeCloseToTime({
+					value: moment(ticks[i] + '-01-01'),
+					unit: 'day',
+					threshold: 0.5,
+				});
+			}
+		});
+	});
+
 	it('should get the correct label for a data value', function() {
 		var chart = window.acquireChart({
 			type: 'line',