From: Jukka Kurkela Date: Mon, 13 Jul 2020 17:20:05 +0000 (+0300) Subject: Line hide (#7612) X-Git-Tag: v3.0.0-beta.2~48 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=91164e8c66a4d767fc629d0a98ae5cce263efbbf;p=thirdparty%2FChart.js.git 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 --- diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index a44f81115..53083db0e 100644 --- a/src/controllers/controller.line.js +++ b/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'; diff --git a/src/controllers/controller.radar.js b/src/controllers/controller.radar.js index b7dca8f6a..1c30e0c0f 100644 --- a/src/controllers/controller.radar.js +++ b/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; } } diff --git a/src/controllers/controller.scatter.js b/src/controllers/controller.scatter.js index 75165a9d4..01200f86a 100644 --- a/src/controllers/controller.scatter.js +++ b/src/controllers/controller.scatter.js @@ -20,7 +20,8 @@ ScatterController.defaults = { }, datasets: { - showLine: false + showLine: false, + fill: false }, tooltips: { diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js index 551d4e8ba..fb15a4b51 100644 --- a/src/core/core.datasetController.js +++ b/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 = 0, ilen = elements.length; i < ilen; ++i) { + const element = elements[i]; + if (element.active) { + active.push(element); + } else { + element.draw(ctx, area); + } } - for (; i < ilen; ++i) { - elements[i].draw(ctx); + for (i = 0, ilen = active.length; i < ilen; ++i) { + active[i].draw(ctx, area); } } diff --git a/src/elements/element.line.js b/src/elements/element.line.js index 46b5ae6e8..55f3718e9 100644 --- a/src/elements/element.line.js +++ b/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(); } diff --git a/test/fixture.js b/test/fixture.js index d00eb3f7d..c6055d1cc 100644 --- a/test/fixture.js +++ b/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(); } diff --git a/test/fixtures/controller.line/borderWidth/zero.js b/test/fixtures/controller.line/borderWidth/zero.js new file mode 100644 index 000000000..35cfc7da0 --- /dev/null +++ b/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 + } + } +}; diff --git a/test/fixtures/controller.line/borderWidth/zero.png b/test/fixtures/controller.line/borderWidth/zero.png new file mode 100644 index 000000000..febae92e5 Binary files /dev/null and b/test/fixtures/controller.line/borderWidth/zero.png differ diff --git a/test/fixtures/controller.line/showLines/dataset.js b/test/fixtures/controller.line/showLines/dataset.js new file mode 100644 index 000000000..b1f47db14 --- /dev/null +++ b/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 + } + } +}; diff --git a/test/fixtures/controller.line/showLines/dataset.png b/test/fixtures/controller.line/showLines/dataset.png new file mode 100644 index 000000000..79e1628f2 Binary files /dev/null and b/test/fixtures/controller.line/showLines/dataset.png differ diff --git a/test/fixtures/controller.line/showLines/false.js b/test/fixtures/controller.line/showLines/false.js new file mode 100644 index 000000000..d88f9ebdd --- /dev/null +++ b/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 + } + } + } + } +}; diff --git a/test/fixtures/controller.line/showLines/false.png b/test/fixtures/controller.line/showLines/false.png new file mode 100644 index 000000000..2d2ac9a3a Binary files /dev/null and b/test/fixtures/controller.line/showLines/false.png differ diff --git a/test/fixtures/controller.radar/borderWidth/zero.js b/test/fixtures/controller.radar/borderWidth/zero.js new file mode 100644 index 000000000..dd1bb6555 --- /dev/null +++ b/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 + } + } +}; diff --git a/test/fixtures/controller.radar/borderWidth/zero.png b/test/fixtures/controller.radar/borderWidth/zero.png new file mode 100644 index 000000000..130348dab Binary files /dev/null and b/test/fixtures/controller.radar/borderWidth/zero.png differ diff --git a/test/fixtures/controller.radar/showLines/value.js b/test/fixtures/controller.radar/showLines/value.js new file mode 100644 index 000000000..eab88702b --- /dev/null +++ b/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 + } + } +}; diff --git a/test/fixtures/controller.radar/showLines/value.png b/test/fixtures/controller.radar/showLines/value.png new file mode 100644 index 000000000..fc6a9d94e Binary files /dev/null and b/test/fixtures/controller.radar/showLines/value.png differ diff --git a/test/fixtures/controller.scatter/showLines/true.js b/test/fixtures/controller.scatter/showLines/true.js new file mode 100644 index 000000000..a98262653 --- /dev/null +++ b/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 + } + } +}; diff --git a/test/fixtures/controller.scatter/showLines/true.png b/test/fixtures/controller.scatter/showLines/true.png new file mode 100644 index 000000000..78bd3ac4f Binary files /dev/null and b/test/fixtures/controller.scatter/showLines/true.png differ diff --git a/test/fixtures/controller.scatter/showLines/undefined.js b/test/fixtures/controller.scatter/showLines/undefined.js new file mode 100644 index 000000000..42b8bef9f --- /dev/null +++ b/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 + } + } +}; diff --git a/test/fixtures/controller.scatter/showLines/undefined.png b/test/fixtures/controller.scatter/showLines/undefined.png new file mode 100644 index 000000000..4e01c86f4 Binary files /dev/null and b/test/fixtures/controller.scatter/showLines/undefined.png differ diff --git a/test/matchers.js b/test/matchers.js index 146e80318..f7d41da2b 100644 --- a/test/matchers.js +++ b/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.'; diff --git a/test/specs/controller.line.tests.js b/test/specs/controller.line.tests.js index 3f46f145b..e66ed7101 100644 --- a/test/specs/controller.line.tests.js +++ b/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', diff --git a/test/specs/controller.radar.tests.js b/test/specs/controller.radar.tests.js index 8f3cf935b..10a08e6a0 100644 --- a/test/specs/controller.radar.tests.js +++ b/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, diff --git a/test/specs/controller.scatter.tests.js b/test/specs/controller.scatter.tests.js index ace955b6c..df5168361 100644 --- a/test/specs/controller.scatter.tests.js +++ b/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); - }); - }); });