Line hide (#7612)

* Add support for description in fixtures
* Update datasetController to draw active(s) last
* Handle hiding of line by borderWidth
* Disable filling of scatter charts by default
* Make radar chart consistent with line

src/controllers/controller.line.js

@@ -5,23 +5,15 @@ import {resolve} from '../helpers/helpers.options';
 
 export default class LineController extends DatasetController {
 
-	constructor(chart, datasetIndex) {
-		super(chart, datasetIndex);
-
-		this._showLine = false;
-	}
-
 	update(mode) {
 		const me = this;
 		const meta = me._cachedMeta;
 		const line = meta.dataset;
 		const points = meta.data || [];
-		const options = me.chart.options;
-		const config = me._config;
-		const showLine = me._showLine = valueOrDefault(config.showLine, options.showLines);
 
 		// Update Line
-		if (showLine && mode !== 'resize') {
+		// In resize mode only point locations change, so no need to set the points or options.
+		if (mode !== 'resize') {
 			const properties = {
 				points,
 				options: me.resolveDatasetElementOptions()
@@ -80,6 +72,7 @@ export default class LineController extends DatasetController {
 		const options = me.chart.options;
 		const lineOptions = options.elements.line;
 		const values = super.resolveDatasetElementOptions(active);
+		const showLine = valueOrDefault(config.showLine, options.showLines);
 
 		// The default behavior of lines is to break at null values, according
 		// to https://github.com/chartjs/Chart.js/issues/2435#issuecomment-216718158
@@ -88,6 +81,10 @@ export default class LineController extends DatasetController {
 		values.tension = valueOrDefault(config.lineTension, lineOptions.tension);
 		values.stepped = resolve([config.stepped, lineOptions.stepped]);
 
+		if (!showLine) {
+			values.borderWidth = 0;
+		}
+
 		return values;
 	}
 
@@ -97,7 +94,7 @@ export default class LineController extends DatasetController {
 	getMaxOverflow() {
 		const me = this;
 		const meta = me._cachedMeta;
-		const border = me._showLine && meta.dataset.options.borderWidth || 0;
+		const border = meta.dataset.options.borderWidth || 0;
 		const data = meta.data || [];
 		if (!data.length) {
 			return border;
@@ -106,36 +103,6 @@ export default class LineController extends DatasetController {
 		const lastPoint = data[data.length - 1].size();
 		return Math.max(border, firstPoint, lastPoint) / 2;
 	}
-
-	draw() {
-		const me = this;
-		const ctx = me._ctx;
-		const chart = me.chart;
-		const meta = me._cachedMeta;
-		const points = meta.data || [];
-		const area = chart.chartArea;
-		const active = [];
-		let ilen = points.length;
-		let i, point;
-
-		if (me._showLine) {
-			meta.dataset.draw(ctx, area);
-		}
-
-
-		// Draw the points
-		for (i = 0; i < ilen; ++i) {
-			point = points[i];
-			if (point.active) {
-				active.push(point);
-			} else {
-				point.draw(ctx, area);
-			}
-		}
-		for (i = 0, ilen = active.length; i < ilen; ++i) {
-			active[i].draw(ctx, area);
-		}
-	}
 }
 
 LineController.id = 'line';

src/controllers/controller.radar.js

@@ -23,19 +23,22 @@ export default class RadarController extends DatasetController {
 		const line = meta.dataset;
 		const points = meta.data || [];
 		const labels = meta.iScale.getLabels();
-		const properties = {
-			points,
-			_loop: true,
-			_fullLoop: labels.length === points.length,
-			options: me.resolveDatasetElementOptions()
-		};
 
-		me.updateElement(line, undefined, properties, mode);
+		// Update Line
+		// In resize mode only point locations change, so no need to set the points or options.
+		if (mode !== 'resize') {
+			const properties = {
+				points,
+				_loop: true,
+				_fullLoop: labels.length === points.length,
+				options: me.resolveDatasetElementOptions()
+			};
+
+			me.updateElement(line, undefined, properties, mode);
+		}
 
 		// Update Points
 		me.updateElements(points, 0, mode);
-
-		line.updateControlPoints(me.chart.chartArea);
 	}
 
 	updateElements(points, start, mode) {
@@ -75,10 +78,15 @@ export default class RadarController extends DatasetController {
 		const config = me._config;
 		const options = me.chart.options;
 		const values = super.resolveDatasetElementOptions(active);
+		const showLine = valueOrDefault(config.showLine, options.showLines);
 
 		values.spanGaps = valueOrDefault(config.spanGaps, options.spanGaps);
 		values.tension = valueOrDefault(config.lineTension, options.elements.line.tension);
 
+		if (!showLine) {
+			values.borderWidth = 0;
+		}
+
 		return values;
 	}
 }

src/controllers/controller.scatter.js

@@ -20,7 +20,8 @@ ScatterController.defaults = {
 	},
 
 	datasets: {
-		showLine: false
+		showLine: false,
+		fill: false
 	},
 
 	tooltips: {

src/core/core.datasetController.js

@@ -620,18 +620,30 @@ export default class DatasetController {
 	update(mode) {} // eslint-disable-line no-unused-vars
 
 	draw() {
-		const ctx = this._ctx;
-		const meta = this._cachedMeta;
+		const me = this;
+		const ctx = me._ctx;
+		const chart = me.chart;
+		const meta = me._cachedMeta;
 		const elements = meta.data || [];
-		const ilen = elements.length;
-		let i = 0;
+		const area = chart.chartArea;
+		const active = [];
+		let i, ilen;
 
 		if (meta.dataset) {
-			meta.dataset.draw(ctx);
+			meta.dataset.draw(ctx, area);
 		}
 
-		for (; i < ilen; ++i) {
-			elements[i].draw(ctx);
+		for (i = 0, ilen = elements.length; i < ilen; ++i) {
+			const element = elements[i];
+			if (element.active) {
+				active.push(element);
+			} else {
+				element.draw(ctx, area);
+			}
+		}
+
+		for (i = 0, ilen = active.length; i < ilen; ++i) {
+			active[i].draw(ctx, area);
 		}
 	}
 

src/elements/element.line.js

@@ -192,7 +192,6 @@ export default class Line extends Element {
 		this.options = undefined;
 		this._loop = undefined;
 		this._fullLoop = undefined;
-		this._controlPointsUpdated = undefined;
 		this._points = undefined;
 		this._segments = undefined;
 
@@ -203,9 +202,6 @@ export default class Line extends Element {
 
 	updateControlPoints(chartArea) {
 		const me = this;
-		if (me._controlPointsUpdated) {
-			return;
-		}
 		const options = me.options;
 		if (options.tension && !options.stepped) {
 			const loop = options.spanGaps ? me._loop : me._fullLoop;
@@ -323,19 +319,20 @@ export default class Line extends Element {
 	 * @param {CanvasRenderingContext2D} ctx
 	 */
 	draw(ctx) {
-		const me = this;
+		const options = this.options || {};
+		const points = this.points || [];
 
-		if (!me.points.length) {
+		if (!points.length || !options.borderWidth) {
 			return;
 		}
 
 		ctx.save();
 
-		setStyle(ctx, me.options);
+		setStyle(ctx, options);
 
 		ctx.beginPath();
 
-		if (me.path(ctx)) {
+		if (this.path(ctx)) {
 			ctx.closePath();
 		}
 

test/fixture.js

@@ -42,9 +42,10 @@ function specFromFixture(description, inputs) {
 	var input = inputs.js || inputs.json;
 	it(input, function(done) {
 		loadConfig(input, function(json) {
+			var descr = json.description || (json.description = description);
 			var chart = utils.acquireChart(json.config, json.options);
 			if (!inputs.png) {
-				fail('Missing PNG comparison file for ' + input);
+				fail(descr + '\r\nMissing PNG comparison file for ' + input);
 				done();
 			}
 

test/fixtures/controller.line/borderWidth/zero.js

@@ -0,0 +1,49 @@
+module.exports = {
+	config: {
+		type: 'line',
+		data: {
+			labels: [0, 1, 2, 3, 4, 5],
+			datasets: [
+				{
+					// option in dataset
+					data: [0, 5, 10, null, -10, -5],
+					backgroundColor: '#0000ff',
+					borderColor: '#0000ff',
+					borderWidth: 0
+				},
+				{
+					// option in element (fallback)
+					data: [4, -5, -10, null, 10, 5],
+				}
+			]
+		},
+		options: {
+			legend: false,
+			title: false,
+			elements: {
+				line: {
+					borderColor: '#00ff00',
+					borderWidth: 3,
+					fill: false,
+				},
+				point: {
+					backgroundColor: '#00ff00',
+					radius: 10,
+				}
+			},
+			layout: {
+				padding: 32
+			},
+			scales: {
+				x: {display: false},
+				y: {display: false}
+			}
+		}
+	},
+	options: {
+		canvas: {
+			height: 256,
+			width: 512
+		}
+	}
+};

test/fixtures/controller.line/showLines/dataset.js

@@ -0,0 +1,36 @@
+module.exports = {
+	description: 'should draw all elements except lines turned off per dataset',
+	config: {
+		type: 'line',
+		data: {
+			datasets: [{
+				data: [10, 15, 0, -4],
+				label: 'dataset1',
+				borderColor: 'red',
+				backgroundColor: 'green',
+				showLine: false,
+				fill: false
+			}],
+			labels: ['label1', 'label2', 'label3', 'label4']
+		},
+		options: {
+			legend: false,
+			title: false,
+			showLines: true,
+			scales: {
+				x: {
+					display: false
+				},
+				y: {
+					display: false
+				}
+			}
+		}
+	},
+	options: {
+		canvas: {
+			width: 512,
+			height: 512
+		}
+	}
+};

test/fixtures/controller.line/showLines/false.js

@@ -0,0 +1,28 @@
+module.exports = {
+	description: 'should draw all elements except lines',
+	config: {
+		type: 'line',
+		data: {
+			datasets: [{
+				data: [10, 15, 0, -4],
+				label: 'dataset1',
+				borderColor: 'red',
+				backgroundColor: 'green'
+			}],
+			labels: ['label1', 'label2', 'label3', 'label4']
+		},
+		options: {
+			legend: false,
+			title: false,
+			showLines: false,
+			scales: {
+				x: {
+					display: false
+				},
+				y: {
+					display: false
+				}
+			}
+		}
+	}
+};

test/fixtures/controller.radar/borderWidth/zero.js

@@ -0,0 +1,46 @@
+module.exports = {
+	config: {
+		type: 'radar',
+		data: {
+			labels: [0, 1, 2, 3, 4, 5],
+			datasets: [
+				{
+					// option in dataset
+					data: [0, 5, 10, null, -10, -5],
+					backgroundColor: '#0000ff',
+					borderColor: '#0000ff',
+					borderWidth: 0,
+				},
+				{
+					// option in element (fallback)
+					data: [4, -5, -10, null, 10, 5]
+				}
+			]
+		},
+		options: {
+			legend: false,
+			title: false,
+			elements: {
+				line: {
+					borderColor: '#00ff00',
+					borderWidth: 1,
+					fill: false
+				},
+				point: {
+					backgroundColor: '#00ff00',
+					radius: 10
+				}
+			},
+			scale: {
+				display: false,
+				min: -15
+			}
+		}
+	},
+	options: {
+		canvas: {
+			height: 512,
+			width: 512
+		}
+	}
+};

test/fixtures/controller.radar/showLines/value.js

@@ -0,0 +1,48 @@
+module.exports = {
+	config: {
+		type: 'radar',
+		data: {
+			labels: [0, 1, 2, 3, 4, 5],
+			datasets: [
+				{
+					// option in dataset
+					data: [0, 5, 10, null, -10, -5],
+					backgroundColor: '#ff0000',
+					fill: false,
+					showLine: true
+				},
+				{
+					// option in element (fallback)
+					data: [4, -5, -10, null, 10, 5]
+				},
+				{
+					data: [1, 1, 1, 1, 1, 1],
+					showLine: true,
+					backgroundColor: 'rgba(0,0,255,0.5)'
+				}
+			]
+		},
+		options: {
+			legend: false,
+			title: false,
+			showLines: false,
+			elements: {
+				line: {
+					borderColor: '#ff0000',
+					backgroundColor: 'rgba(0,255,0,0.5)',
+					fill: true
+				}
+			},
+			scale: {
+				display: false,
+				min: -15
+			}
+		}
+	},
+	options: {
+		canvas: {
+			height: 512,
+			width: 512
+		}
+	}
+};

test/fixtures/controller.scatter/showLines/true.js

@@ -0,0 +1,33 @@
+module.exports = {
+	description: 'showLines option should draw a line if true',
+	config: {
+		type: 'scatter',
+		data: {
+			datasets: [{
+				data: [{x: 10, y: 15}, {x: 15, y: 10}],
+				pointRadius: 10,
+				backgroundColor: 'red',
+				showLine: true,
+				label: 'dataset1'
+			}],
+		},
+		options: {
+			legend: false,
+			title: false,
+			scales: {
+				x: {
+					display: false
+				},
+				y: {
+					display: false
+				}
+			}
+		}
+	},
+	options: {
+		canvas: {
+			width: 256,
+			height: 256
+		}
+	}
+};

test/fixtures/controller.scatter/showLines/undefined.js

@@ -0,0 +1,32 @@
+module.exports = {
+	description: 'showLines option should not draw a line if undefined',
+	config: {
+		type: 'scatter',
+		data: {
+			datasets: [{
+				data: [{x: 10, y: 15}, {x: 15, y: 10}],
+				pointRadius: 10,
+				backgroundColor: 'red',
+				label: 'dataset1'
+			}],
+		},
+		options: {
+			legend: false,
+			title: false,
+			scales: {
+				x: {
+					display: false
+				},
+				y: {
+					display: false
+				}
+			}
+		}
+	},
+	options: {
+		canvas: {
+			width: 256,
+			height: 256
+		}
+	}
+};

test/matchers.js

@@ -20,9 +20,10 @@ function canvasFromImageData(data) {
 	return canvas;
 }
 
-function buildPixelMatchPreview(actual, expected, diff, threshold, tolerance, count) {
+function buildPixelMatchPreview(actual, expected, diff, threshold, tolerance, count, description) {
 	var ratio = count / (actual.width * actual.height);
 	var wrapper = document.createElement('div');
+	wrapper.appendChild(document.createTextNode(description));
 
 	wrapper.style.cssText = 'display: flex; overflow-y: auto';
 
@@ -195,7 +196,7 @@ function toEqualImageData() {
 				ratio = count / (w * h);
 
 				if ((ratio > tolerance) || debug) {
-					message = buildPixelMatchPreview(idata, expected, ddata, threshold, tolerance, count);
+					message = buildPixelMatchPreview(idata, expected, ddata, threshold, tolerance, count, opts.description);
 				}
 			} else {
 				message = 'Input value is not a valid image source.';

test/specs/controller.line.tests.js

@@ -93,69 +93,6 @@ describe('Chart.controllers.line', function() {
 		expect(meta.data[3].draw.calls.count()).toBe(1);
 	});
 
-	it('should draw all elements except lines', function() {
-		var chart = window.acquireChart({
-			type: 'line',
-			data: {
-				datasets: [{
-					data: [10, 15, 0, -4],
-					label: 'dataset1'
-				}],
-				labels: ['label1', 'label2', 'label3', 'label4']
-			},
-			options: {
-				showLines: false
-			}
-		});
-
-		var meta = chart.getDatasetMeta(0);
-		spyOn(meta.dataset, 'draw');
-		spyOn(meta.data[0], 'draw');
-		spyOn(meta.data[1], 'draw');
-		spyOn(meta.data[2], 'draw');
-		spyOn(meta.data[3], 'draw');
-
-		chart.update();
-
-		expect(meta.dataset.draw.calls.count()).toBe(0);
-		expect(meta.data[0].draw.calls.count()).toBe(1);
-		expect(meta.data[1].draw.calls.count()).toBe(1);
-		expect(meta.data[2].draw.calls.count()).toBe(1);
-		expect(meta.data[3].draw.calls.count()).toBe(1);
-	});
-
-	it('should draw all elements except lines turned off per dataset', function() {
-		var chart = window.acquireChart({
-			type: 'line',
-			data: {
-				datasets: [{
-					data: [10, 15, 0, -4],
-					label: 'dataset1',
-					showLine: false
-				}],
-				labels: ['label1', 'label2', 'label3', 'label4']
-			},
-			options: {
-				showLines: true
-			}
-		});
-
-		var meta = chart.getDatasetMeta(0);
-		spyOn(meta.dataset, 'draw');
-		spyOn(meta.data[0], 'draw');
-		spyOn(meta.data[1], 'draw');
-		spyOn(meta.data[2], 'draw');
-		spyOn(meta.data[3], 'draw');
-
-		chart.update();
-
-		expect(meta.dataset.draw.calls.count()).toBe(0);
-		expect(meta.data[0].draw.calls.count()).toBe(1);
-		expect(meta.data[1].draw.calls.count()).toBe(1);
-		expect(meta.data[2].draw.calls.count()).toBe(1);
-		expect(meta.data[3].draw.calls.count()).toBe(1);
-	});
-
 	it('should update elements when modifying data', function() {
 		var chart = window.acquireChart({
 			type: 'line',

test/specs/controller.radar.tests.js

@@ -131,17 +131,13 @@ describe('Chart.controllers.radar', function() {
 		}));
 
 		[
-			{x: 256, y: 260, cppx: 256, cppy: 260, cpnx: 256, cpny: 260},
-			{x: 256, y: 260, cppx: 256, cppy: 260, cpnx: 256, cpny: 260},
-			{x: 256, y: 260, cppx: 256, cppy: 260, cpnx: 256, cpny: 260},
-			{x: 256, y: 260, cppx: 256, cppy: 260, cpnx: 256, cpny: 260},
+			{x: 256, y: 260},
+			{x: 256, y: 260},
+			{x: 256, y: 260},
+			{x: 256, y: 260},
 		].forEach(function(expected, i) {
 			expect(meta.data[i].x).toBeCloseToPixel(expected.x);
 			expect(meta.data[i].y).toBeCloseToPixel(expected.y);
-			expect(meta.data[i].controlPointPreviousX).toBeCloseToPixel(expected.cppx);
-			expect(meta.data[i].controlPointPreviousY).toBeCloseToPixel(expected.cppy);
-			expect(meta.data[i].controlPointNextX).toBeCloseToPixel(expected.cpnx);
-			expect(meta.data[i].controlPointNextY).toBeCloseToPixel(expected.cpny);
 			expect(meta.data[i].options).toEqual(jasmine.objectContaining({
 				backgroundColor: Chart.defaults.color,
 				borderWidth: 1,

test/specs/controller.scatter.tests.js

@@ -1,4 +1,6 @@
 describe('Chart.controllers.scatter', function() {
+	describe('auto', jasmine.fixture.specs('controller.scatter'));
+
 	it('should be registered as dataset controller', function() {
 		expect(typeof Chart.controllers.scatter).toBe('function');
 	});
@@ -29,51 +31,4 @@ describe('Chart.controllers.scatter', function() {
 
 		jasmine.triggerMouseEvent(chart, 'mousemove', point);
 	});
-
-	describe('showLines option', function() {
-		it('should not draw a line if undefined', function() {
-			var chart = window.acquireChart({
-				type: 'scatter',
-				data: {
-					datasets: [{
-						data: [{x: 10, y: 15}],
-						label: 'dataset1'
-					}],
-				},
-				options: {}
-			});
-
-			var meta = chart.getDatasetMeta(0);
-			spyOn(meta.dataset, 'draw');
-			spyOn(meta.data[0], 'draw');
-
-			chart.update();
-
-			expect(meta.dataset.draw.calls.count()).toBe(0);
-			expect(meta.data[0].draw.calls.count()).toBe(1);
-		});
-
-		it('should draw a line if true', function() {
-			var chart = window.acquireChart({
-				type: 'scatter',
-				data: {
-					datasets: [{
-						data: [{x: 10, y: 15}],
-						showLine: true,
-						label: 'dataset1'
-					}],
-				},
-				options: {}
-			});
-
-			var meta = chart.getDatasetMeta(0);
-			spyOn(meta.dataset, 'draw');
-			spyOn(meta.data[0], 'draw');
-
-			chart.update();
-
-			expect(meta.dataset.draw.calls.count()).toBe(1);
-			expect(meta.data[0].draw.calls.count()).toBe(1);
-		});
-	});
 });