Merge pull request #3065 from chartjs/fix/3056

Fix 2 line drawing issues and add new tests for these cases
chartjs · Jul 30, 2016 · 1731620 · 1731620
2 parents 7628d4c + eb6124f
commit 1731620
Show file tree

Hide file tree

Showing 2 changed files with 290 additions and 3 deletions.
diff --git a/src/elements/element.line.js b/src/elements/element.line.js
@@ -117,7 +117,7 @@ module.exports = function(Chart) {
 				}
 
 				if (!loop) {
-					ctx.lineTo(points[points.length - 1]._view.x, scaleZero);
+					ctx.lineTo(points[lastDrawnIndex]._view.x, scaleZero);
 				}
 
 				ctx.fillStyle = vm.backgroundColor || globalDefaults.defaultColor;
@@ -160,7 +160,7 @@ module.exports = function(Chart) {
 					previous = lastDrawnIndex === -1 ? previous : points[lastDrawnIndex];
 
 					if (!currentVM.skip) {
-						if (lastDrawnIndex !== (index - 1) && !spanGaps) {
+						if ((lastDrawnIndex !== (index - 1) && !spanGaps) || lastDrawnIndex === -1) {
 							// There was a gap and this is the first point after the gap
 							ctx.moveTo(currentVM.x, currentVM.y);
 						} else {

diff --git a/test/element.line.tests.js b/test/element.line.tests.js
@@ -960,6 +960,151 @@ describe('Line element tests', function() {
 		expect(mockContext.getCalls()).toEqual(expected);
 	});
 
+	it('should skip the first point correctly when spanGaps is true', function() {
+		var mockContext = window.createMockContext();
+
+		// Create our points
+		var points = [];
+		points.push(new Chart.elements.Point({
+			_datasetindex: 2,
+			_index: 0,
+			_view: {
+				x: 0,
+				y: 10,
+				controlPointNextX: 0,
+				controlPointNextY: 10,
+				skip: true
+			}
+		}));
+		points.push(new Chart.elements.Point({
+			_datasetindex: 2,
+			_index: 1,
+			_view: {
+				x: 5,
+				y: 0,
+				controlPointPreviousX: 5,
+				controlPointPreviousY: 0,
+				controlPointNextX: 5,
+				controlPointNextY: 0
+			}
+		}));
+		points.push(new Chart.elements.Point({
+			_datasetindex: 2,
+			_index: 2,
+			_view: {
+				x: 15,
+				y: -10,
+				controlPointPreviousX: 15,
+				controlPointPreviousY: -10,
+				controlPointNextX: 15,
+				controlPointNextY: -10
+			}
+		}));
+		points.push(new Chart.elements.Point({
+			_datasetindex: 2,
+			_index: 3,
+			_view: {
+				x: 19,
+				y: -5,
+				controlPointPreviousX: 19,
+				controlPointPreviousY: -5,
+				controlPointNextX: 19,
+				controlPointNextY: -5
+			}
+		}));
+
+		var line = new Chart.elements.Line({
+			_datasetindex: 2,
+			_chart: {
+				ctx: mockContext,
+			},
+			_children: points,
+			// Need to provide some settings
+			_view: {
+				fill: true,
+				scaleZero: 2, // for filling lines
+				tension: 0.0, // no bezier curve for now
+				spanGaps: true
+			}
+		});
+
+		line.draw();
+
+		var expected = [{
+			name: 'save',
+			args: []
+		}, {
+			name: 'beginPath',
+			args: []
+		}, {
+			name: 'moveTo',
+			args: [0, 2]
+		}, {
+			name: 'lineTo',
+			args: [5, 2]
+		}, {
+			name: 'lineTo',
+			args: [5, 0]
+		}, {
+			name: 'bezierCurveTo',
+			args: [5, 0, 15, -10, 15, -10]
+		}, {
+			name: 'bezierCurveTo',
+			args: [15, -10, 19, -5, 19, -5]
+		}, {
+			name: 'lineTo',
+			args: [19, 2]
+		}, {
+			name: 'setFillStyle',
+			args: ['rgba(0,0,0,0.1)']
+		}, {
+			name: 'closePath',
+			args: []
+		}, {
+			name: 'fill',
+			args: []
+		}, {
+			name: 'setLineCap',
+			args: ['butt']
+		}, {
+			name: 'setLineDash',
+			args: [
+				[]
+			]
+		}, {
+			name: 'setLineDashOffset',
+			args: [0]
+		}, {
+			name: 'setLineJoin',
+			args: ['miter']
+		}, {
+			name: 'setLineWidth',
+			args: [3]
+		}, {
+			name: 'setStrokeStyle',
+			args: ['rgba(0,0,0,0.1)']
+		}, {
+			name: 'beginPath',
+			args: []
+		}, {
+			name: 'moveTo',
+			args: [5, 0]
+		}, {
+			name: 'bezierCurveTo',
+			args: [5, 0, 15, -10, 15, -10]
+		}, {
+			name: 'bezierCurveTo',
+			args: [15, -10, 19, -5, 19, -5]
+		}, {
+			name: 'stroke',
+			args: []
+		}, {
+			name: 'restore',
+			args: []
+		}];
+		expect(mockContext.getCalls()).toEqual(expected);
+	});
+
 	it('should skip the last point correctly', function() {
 		var mockContext = window.createMockContext();
 
@@ -1052,7 +1197,149 @@ describe('Line element tests', function() {
 			args: [15, 2]
 		}, {
 			name: 'lineTo',
-			args: [19, 2]
+			args: [15, 2]
+		}, {
+			name: 'setFillStyle',
+			args: ['rgba(0,0,0,0.1)']
+		}, {
+			name: 'closePath',
+			args: []
+		}, {
+			name: 'fill',
+			args: []
+		}, {
+			name: 'setLineCap',
+			args: ['butt']
+		}, {
+			name: 'setLineDash',
+			args: [
+				[]
+			]
+		}, {
+			name: 'setLineDashOffset',
+			args: [0]
+		}, {
+			name: 'setLineJoin',
+			args: ['miter']
+		}, {
+			name: 'setLineWidth',
+			args: [3]
+		}, {
+			name: 'setStrokeStyle',
+			args: ['rgba(0,0,0,0.1)']
+		}, {
+			name: 'beginPath',
+			args: []
+		}, {
+			name: 'moveTo',
+			args: [0, 10]
+		}, {
+			name: 'bezierCurveTo',
+			args: [0, 10, 5, 0, 5, 0]
+		}, {
+			name: 'bezierCurveTo',
+			args: [5, 0, 15, -10, 15, -10]
+		}, {
+			name: 'stroke',
+			args: []
+		}, {
+			name: 'restore',
+			args: []
+		}];
+		expect(mockContext.getCalls()).toEqual(expected);
+	});
+
+	it('should skip the last point correctly when spanGaps is true', function() {
+		var mockContext = window.createMockContext();
+
+		// Create our points
+		var points = [];
+		points.push(new Chart.elements.Point({
+			_datasetindex: 2,
+			_index: 0,
+			_view: {
+				x: 0,
+				y: 10,
+				controlPointNextX: 0,
+				controlPointNextY: 10
+			}
+		}));
+		points.push(new Chart.elements.Point({
+			_datasetindex: 2,
+			_index: 1,
+			_view: {
+				x: 5,
+				y: 0,
+				controlPointPreviousX: 5,
+				controlPointPreviousY: 0,
+				controlPointNextX: 5,
+				controlPointNextY: 0
+			}
+		}));
+		points.push(new Chart.elements.Point({
+			_datasetindex: 2,
+			_index: 2,
+			_view: {
+				x: 15,
+				y: -10,
+				controlPointPreviousX: 15,
+				controlPointPreviousY: -10,
+				controlPointNextX: 15,
+				controlPointNextY: -10
+			}
+		}));
+		points.push(new Chart.elements.Point({
+			_datasetindex: 2,
+			_index: 3,
+			_view: {
+				x: 19,
+				y: -5,
+				controlPointPreviousX: 19,
+				controlPointPreviousY: -5,
+				controlPointNextX: 19,
+				controlPointNextY: -5,
+				skip: true
+			}
+		}));
+
+		var line = new Chart.elements.Line({
+			_datasetindex: 2,
+			_chart: {
+				ctx: mockContext,
+			},
+			_children: points,
+			// Need to provide some settings
+			_view: {
+				fill: true,
+				scaleZero: 2, // for filling lines
+				tension: 0.0, // no bezier curve for now
+				spanGaps: true
+			}
+		});
+
+		line.draw();
+
+		var expected = [{
+			name: 'save',
+			args: []
+		}, {
+			name: 'beginPath',
+			args: []
+		}, {
+			name: 'moveTo',
+			args: [0, 2]
+		}, {
+			name: 'lineTo',
+			args: [0, 10]
+		}, {
+			name: 'bezierCurveTo',
+			args: [0, 10, 5, 0, 5, 0]
+		}, {
+			name: 'bezierCurveTo',
+			args: [5, 0, 15, -10, 15, -10]
+		}, {
+			name: 'lineTo',
+			args: [15, 2]
 		}, {
 			name: 'setFillStyle',
 			args: ['rgba(0,0,0,0.1)']