From b53b54334c5cd75a8ef542f56f0f652fd94d02e4 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Tue, 23 Feb 2021 17:08:18 -0800 Subject: [PATCH 01/10] Use null for skipped values instead of NaN --- docs/docs/charts/line.mdx | 4 ++-- docs/docs/charts/radar.mdx | 4 ++-- docs/docs/configuration/tooltip.md | 2 +- docs/docs/developers/axes.md | 7 ------- src/controllers/controller.bar.js | 2 +- src/controllers/controller.bubble.js | 2 +- src/controllers/controller.doughnut.js | 4 ++-- src/core/core.datasetController.js | 18 +++++++++++------- src/scales/scale.linear.js | 2 +- src/scales/scale.linearbase.js | 4 ++-- src/scales/scale.logarithmic.js | 2 +- src/scales/scale.time.js | 8 ++++---- 12 files changed, 28 insertions(+), 31 deletions(-) diff --git a/docs/docs/charts/line.mdx b/docs/docs/charts/line.mdx index e8b6b682294..0536a1cead7 100644 --- a/docs/docs/charts/line.mdx +++ b/docs/docs/charts/line.mdx @@ -137,7 +137,7 @@ The style of the line can be controlled with the following properties: | `fill` | How to fill the area under the line. See [area charts](area.md). | `tension` | Bezier curve tension of the line. Set to 0 to draw straightlines. This option is ignored if monotone cubic interpolation is used. | `showLine` | If false, the line is not drawn for this dataset. -| `spanGaps` | If true, lines will be drawn between points with no or null data. If false, points with `NaN` data will create a break in the line. Can also be a number specifying the maximum gap length to span. The unit of the value depends on the scale used. +| `spanGaps` | If true, lines will be drawn between points with no or null data. If false, points with `null` data will create a break in the line. Can also be a number specifying the maximum gap length to span. The unit of the value depends on the scale used. If the value is `undefined`, `showLine` and `spanGaps` fallback to the associated [chart configuration options](#configuration-options). The rest of the values fallback to the associated [`elements.line.*`](../configuration/elements.md#line-configuration) options. @@ -184,7 +184,7 @@ The line chart defines the following configuration options. These options are me | Name | Type | Default | Description | ---- | ---- | ------- | ----------- | `showLine` | `boolean` | `true` | If false, the lines between points are not drawn. -| `spanGaps` | `boolean`\|`number` | `false` | If true, lines will be drawn between points with no or null data. If false, points with `NaN` data will create a break in the line. Can also be a number specifying the maximum gap length to span. The unit of the value depends on the scale used. +| `spanGaps` | `boolean`\|`number` | `false` | If true, lines will be drawn between points with no or null data. If false, points with `null` data will create a break in the line. Can also be a number specifying the maximum gap length to span. The unit of the value depends on the scale used. ## Default Options diff --git a/docs/docs/charts/radar.mdx b/docs/docs/charts/radar.mdx index 689a66b40b9..414f49daf70 100644 --- a/docs/docs/charts/radar.mdx +++ b/docs/docs/charts/radar.mdx @@ -149,7 +149,7 @@ The style of the line can be controlled with the following properties: | `borderWidth` | The line width (in pixels). | `fill` | How to fill the area under the line. See [area charts](area.md). | `tension` | Bezier curve tension of the line. Set to 0 to draw straight lines. -| `spanGaps` | If true, lines will be drawn between points with no or null data. If false, points with `NaN` data will create a break in the line. +| `spanGaps` | If true, lines will be drawn between points with no or null data. If false, points with `null` data will create a break in the line. If the value is `undefined`, `spanGaps` fallback to the associated [chart configuration options](#configuration-options). The rest of the values fallback to the associated [`elements.line.*`](../configuration/elements.md#line-configuration) options. @@ -170,7 +170,7 @@ The radar chart defines the following configuration options. These options are m | Name | Type | Default | Description | ---- | ---- | ------- | ----------- -| `spanGaps` | `boolean` | `false` | If false, NaN data causes a break in the line. +| `spanGaps` | `boolean` | `false` | If false, `null` data causes a break in the line. ## Scale Options diff --git a/docs/docs/configuration/tooltip.md b/docs/docs/configuration/tooltip.md index a4b1a67da65..07dc6717a0f 100644 --- a/docs/docs/configuration/tooltip.md +++ b/docs/docs/configuration/tooltip.md @@ -139,7 +139,7 @@ var chart = new Chart(ctx, { if (label) { label += ': '; } - if (!isNaN(context.parsed.y)) { + if (context.parsed.y !== null) { label += new Intl.NumberFormat('en-US', { style: 'currency', currency: 'USD' }).format(context.parsed.y); } return label; diff --git a/docs/docs/developers/axes.md b/docs/docs/developers/axes.md index 8a60ecb1052..2762c38dded 100644 --- a/docs/docs/developers/axes.md +++ b/docs/docs/developers/axes.md @@ -132,13 +132,6 @@ The Core.Scale base class also has some utility functions that you may find usef // Returns true if the scale instance is horizontal isHorizontal: function() {}, - // Get the correct value from the value from this.chart.data.datasets[x].data[] - // If dataValue is an object, returns .x or .y depending on the return of isHorizontal() - // If the value is undefined, returns NaN - // Otherwise returns the value. - // Note that in all cases, the returned value is not guaranteed to be a number - getRightValue: function(dataValue) {}, - // Returns the scale tick objects ({label, major}) getTicks: function() {} } diff --git a/src/controllers/controller.bar.js b/src/controllers/controller.bar.js index 1782d912798..94d7a5313ee 100644 --- a/src/controllers/controller.bar.js +++ b/src/controllers/controller.bar.js @@ -492,7 +492,7 @@ export default class BarController extends DatasetController { clipArea(chart.ctx, chart.chartArea); for (; i < ilen; ++i) { - if (!isNaN(me.getParsed(i)[vScale.axis])) { + if (me.getParsed(i)[vScale.axis] !== null) { rects[i].draw(me._ctx); } } diff --git a/src/controllers/controller.bubble.js b/src/controllers/controller.bubble.js index 348ea8866d7..ca25170cbc8 100644 --- a/src/controllers/controller.bubble.js +++ b/src/controllers/controller.bubble.js @@ -83,7 +83,7 @@ export default class BubbleController extends DatasetController { const properties = { x, y, - skip: isNaN(x) || isNaN(y) + skip: x === null || y === null }; if (includeOptions) { diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index 2f013c0cff8..e2431c2fc1c 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -161,7 +161,7 @@ export default class DoughnutController extends DatasetController { const opts = me.options; const meta = me._cachedMeta; const circumference = me._getCircumference(); - return reset && opts.animation.animateRotate ? 0 : this.chart.getDataVisibility(i) ? me.calculateCircumference(meta._parsed[i] * circumference / TAU) : 0; + return reset && opts.animation.animateRotate ? 0 : this.chart.getDataVisibility(i) ? (meta._parsed[i] === null ? 0 : me.calculateCircumference(meta._parsed[i]) * circumference / TAU) : 0; } updateElements(arcs, start, count, mode) { @@ -216,7 +216,7 @@ export default class DoughnutController extends DatasetController { for (i = 0; i < metaData.length; i++) { const value = meta._parsed[i]; - if (!isNaN(value) && this.chart.getDataVisibility(i)) { + if (value !== null && this.chart.getDataVisibility(i)) { total += Math.abs(value); } } diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js index 07b1795ba95..08af5e583b9 100644 --- a/src/core/core.datasetController.js +++ b/src/core/core.datasetController.js @@ -1,6 +1,6 @@ import Animations from './core.animations'; import defaults from './core.defaults'; -import {isObject, isArray, valueOrDefault, resolveObjectKey, defined} from '../helpers/helpers.core'; +import {isArray, isFinite, isObject, valueOrDefault, resolveObjectKey, defined} from '../helpers/helpers.core'; import {listenArrayEvents, unlistenArrayEvents} from '../helpers/helpers.collection'; import {sign} from '../helpers/helpers.math'; @@ -70,6 +70,10 @@ function applyStack(stack, value, dsIndex, allOther) { const keys = stack.keys; let i, ilen, datasetIndex, otherValue; + if (value === null) { + return; + } + for (i = 0, ilen = keys.length; i < ilen; ++i) { datasetIndex = +keys[i]; if (datasetIndex === dsIndex) { @@ -79,7 +83,7 @@ function applyStack(stack, value, dsIndex, allOther) { break; } otherValue = stack.values[datasetIndex]; - if (!isNaN(otherValue) && (value === 0 || sign(value) === sign(otherValue))) { + if (otherValue !== null && !isNaN(otherValue) && (value === 0 || sign(value) === sign(otherValue))) { value += otherValue; } } @@ -393,7 +397,7 @@ export default class DatasetController { parsed = me.parsePrimitiveData(meta, data, start, count); } - const isNotInOrderComparedToPrev = () => isNaN(cur[iAxis]) || (prev && cur[iAxis] < prev[iAxis]); + const isNotInOrderComparedToPrev = () => cur[iAxis] === null || (prev && cur[iAxis] < prev[iAxis]); for (i = 0; i < count; ++i) { meta._parsed[i + start] = cur = parsed[i]; if (sorted) { @@ -528,7 +532,7 @@ export default class DatasetController { * @protected */ updateRangeFromParsed(range, scale, parsed, stack) { - let value = parsed[scale.axis]; + let value = parsed[scale.axis] === null ? NaN : parsed[scale.axis]; const values = stack && parsed._stacks[scale.axis]; if (stack && values) { stack.values = values; @@ -536,7 +540,7 @@ export default class DatasetController { // in addition to the stacked value range.min = Math.min(range.min, value); range.max = Math.max(range.max, value); - value = applyStack(stack, value, this._cachedMeta.index, true); + value = applyStack(stack, parsed[scale.axis], this._cachedMeta.index, true); } range.min = Math.min(range.min, value); range.max = Math.max(range.max, value); @@ -561,7 +565,7 @@ export default class DatasetController { parsed = _parsed[i]; value = parsed[scale.axis]; otherValue = parsed[otherScale.axis]; - return (isNaN(value) || isNaN(otherValue) || otherMin > otherValue || otherMax < otherValue); + return (!isFinite(value) || !isFinite(otherValue) || otherMin > otherValue || otherMax < otherValue); } for (i = 0; i < ilen; ++i) { @@ -594,7 +598,7 @@ export default class DatasetController { for (i = 0, ilen = parsed.length; i < ilen; ++i) { value = parsed[i][scale.axis]; - if (!isNaN(value)) { + if (value !== null && !isNaN(value)) { values.push(value); } } diff --git a/src/scales/scale.linear.js b/src/scales/scale.linear.js index ece759902fb..e9617fb1922 100644 --- a/src/scales/scale.linear.js +++ b/src/scales/scale.linear.js @@ -31,7 +31,7 @@ export default class LinearScale extends LinearScaleBase { // Utils getPixelForValue(value) { - return this.getPixelForDecimal((value - this._startValue) / this._valueRange); + return value === null ? NaN : this.getPixelForDecimal((value - this._startValue) / this._valueRange); } getValueForPixel(pixel) { diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index 9c669aa621f..fe439156850 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -115,10 +115,10 @@ export default class LinearScaleBase extends Scale { parse(raw, index) { // eslint-disable-line no-unused-vars if (isNullOrUndef(raw)) { - return NaN; + return null; } if ((typeof raw === 'number' || raw instanceof Number) && !isFinite(+raw)) { - return NaN; + return null; } return +raw; diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index 5dccaf7926c..f88b02c2d55 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -64,7 +64,7 @@ export default class LogarithmicScale extends Scale { this._zero = true; return undefined; } - return isFinite(value) && value > 0 ? value : NaN; + return isFinite(value) && value > 0 ? value : null; } determineDataLimits() { diff --git a/src/scales/scale.time.js b/src/scales/scale.time.js index 52ead35238e..04df0ae1d20 100644 --- a/src/scales/scale.time.js +++ b/src/scales/scale.time.js @@ -47,7 +47,7 @@ function sorter(a, b) { */ function parse(scale, input) { if (isNullOrUndef(input)) { - return NaN; + return null; } const adapter = scale._adapter; @@ -67,7 +67,7 @@ function parse(scale, input) { } if (value === null) { - return NaN; + return null; } if (round) { @@ -244,7 +244,7 @@ export default class TimeScale extends Scale { */ parse(raw, index) { // eslint-disable-line no-unused-vars if (raw === undefined) { - return NaN; + return null; } return parse(this, raw); } @@ -489,7 +489,7 @@ export default class TimeScale extends Scale { */ getDecimalForValue(value) { const me = this; - return (value - me.min) / (me.max - me.min); + return value === null ? NaN : (value - me.min) / (me.max - me.min); } /** From 0d9cddc904c8f1febe52f2e8e3d01693794d8645 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 24 Feb 2021 05:36:31 -0800 Subject: [PATCH 02/10] Document skipped values when parsing is false --- docs/docs/general/data-structures.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/docs/general/data-structures.md b/docs/docs/general/data-structures.md index 8bc65bf1801..49c863b6fd0 100644 --- a/docs/docs/general/data-structures.md +++ b/docs/docs/general/data-structures.md @@ -19,7 +19,7 @@ When the `data` is an array of numbers, values from `labels` array at the same i ## Object[] ```javascript -data: [{x: 10, y: 20}, {x: 15, y: 10}] +data: [{x: 10, y: 20}, {x: 15, y: null}, {x: 20, y: 10}] ``` ```javascript @@ -32,7 +32,7 @@ data: [{x:'Sales', y:20}, {x:'Revenue', y:10}] This is also the internal format used for parsed data. In this mode, parsing can be disabled by specifying `parsing: false` at chart options or dataset. If parsing is disabled, data must be sorted and in the formats the associated chart type and scales use internally. -The values provided must be parsable by the associated scales or in the internal format of the associated scales. A common mistake would be to provide integers for the `category` scale, which uses integers as an internal format, where each integer represents an index in the labels array. +The values provided must be parsable by the associated scales or in the internal format of the associated scales. A common mistake would be to provide integers for the `category` scale, which uses integers as an internal format, where each integer represents an index in the labels array. `null` can be used for skipped values. ## Object[] using custom properties From fed3333a72f87ea537c833f73c529c87ca47b112 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 24 Feb 2021 05:49:47 -0800 Subject: [PATCH 03/10] Update src/core/core.datasetController.js Co-authored-by: Jukka Kurkela --- src/core/core.datasetController.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js index 08af5e583b9..84abde2718c 100644 --- a/src/core/core.datasetController.js +++ b/src/core/core.datasetController.js @@ -532,7 +532,8 @@ export default class DatasetController { * @protected */ updateRangeFromParsed(range, scale, parsed, stack) { - let value = parsed[scale.axis] === null ? NaN : parsed[scale.axis]; + const parsedValue = parsed[scale.axis]; + let value = parsedValue === null ? NaN : parsedValue; const values = stack && parsed._stacks[scale.axis]; if (stack && values) { stack.values = values; From 09b4d35c29b5d6f9c2592287b7f18e79d3c57439 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 24 Feb 2021 05:50:03 -0800 Subject: [PATCH 04/10] Update src/core/core.datasetController.js Co-authored-by: Jukka Kurkela --- src/core/core.datasetController.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js index 84abde2718c..2139615a5ab 100644 --- a/src/core/core.datasetController.js +++ b/src/core/core.datasetController.js @@ -541,7 +541,7 @@ export default class DatasetController { // in addition to the stacked value range.min = Math.min(range.min, value); range.max = Math.max(range.max, value); - value = applyStack(stack, parsed[scale.axis], this._cachedMeta.index, true); + value = applyStack(stack, parsedValue, this._cachedMeta.index, true); } range.min = Math.min(range.min, value); range.max = Math.max(range.max, value); From c00d82c013a819370ea8018b0756ba6b10a62489 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 24 Feb 2021 05:59:48 -0800 Subject: [PATCH 05/10] fix lint issue --- src/core/core.datasetController.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js index 2139615a5ab..9db87fd4b59 100644 --- a/src/core/core.datasetController.js +++ b/src/core/core.datasetController.js @@ -533,7 +533,7 @@ export default class DatasetController { */ updateRangeFromParsed(range, scale, parsed, stack) { const parsedValue = parsed[scale.axis]; - let value = parsedValue === null ? NaN : parsedValue; + let value = parsedValue === null ? NaN : parsedValue; const values = stack && parsed._stacks[scale.axis]; if (stack && values) { stack.values = values; From 2f1c3279df6819e289d0a55c2973909b6e9d72b7 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 24 Feb 2021 06:11:20 -0800 Subject: [PATCH 06/10] use isFinite --- src/core/core.datasetController.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js index 9db87fd4b59..7413ec954fe 100644 --- a/src/core/core.datasetController.js +++ b/src/core/core.datasetController.js @@ -83,7 +83,7 @@ function applyStack(stack, value, dsIndex, allOther) { break; } otherValue = stack.values[datasetIndex]; - if (otherValue !== null && !isNaN(otherValue) && (value === 0 || sign(value) === sign(otherValue))) { + if (isFinite(otherValue) && (value === 0 || sign(value) === sign(otherValue))) { value += otherValue; } } @@ -599,7 +599,7 @@ export default class DatasetController { for (i = 0, ilen = parsed.length; i < ilen; ++i) { value = parsed[i][scale.axis]; - if (value !== null && !isNaN(value)) { + if (isFinite(value)) { values.push(value); } } From 78505bb931dfb8c2ab6bc25291ceff9a4dad4485 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 24 Feb 2021 06:21:10 -0800 Subject: [PATCH 07/10] revert change checking for pixel values --- src/controllers/controller.bubble.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/controller.bubble.js b/src/controllers/controller.bubble.js index ca25170cbc8..348ea8866d7 100644 --- a/src/controllers/controller.bubble.js +++ b/src/controllers/controller.bubble.js @@ -83,7 +83,7 @@ export default class BubbleController extends DatasetController { const properties = { x, y, - skip: x === null || y === null + skip: isNaN(x) || isNaN(y) }; if (includeOptions) { From cda00acb3894b3e74bf15e10b16ee5c5af67ae62 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 24 Feb 2021 06:31:31 -0800 Subject: [PATCH 08/10] ternary readability --- src/controllers/controller.doughnut.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index e2431c2fc1c..02cca2aef05 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -154,14 +154,17 @@ export default class DoughnutController extends DatasetController { } /** - * @private - */ + * @private + */ _circumference(i, reset) { const me = this; const opts = me.options; const meta = me._cachedMeta; const circumference = me._getCircumference(); - return reset && opts.animation.animateRotate ? 0 : this.chart.getDataVisibility(i) ? (meta._parsed[i] === null ? 0 : me.calculateCircumference(meta._parsed[i]) * circumference / TAU) : 0; + if ((reset && opts.animation.animateRotate) || !this.chart.getDataVisibility(i) || meta._parsed[i] === null) { + return 0; + } + return me.calculateCircumference(meta._parsed[i]) * circumference / TAU; } updateElements(arcs, start, count, mode) { From d7bbc573f1dddbb93bf06e2581023effdd5f79ab Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 24 Feb 2021 06:32:33 -0800 Subject: [PATCH 09/10] revert accidental paren movement --- src/controllers/controller.doughnut.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index 02cca2aef05..68f3801af89 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -164,7 +164,7 @@ export default class DoughnutController extends DatasetController { if ((reset && opts.animation.animateRotate) || !this.chart.getDataVisibility(i) || meta._parsed[i] === null) { return 0; } - return me.calculateCircumference(meta._parsed[i]) * circumference / TAU; + return me.calculateCircumference(meta._parsed[i] * circumference / TAU); } updateElements(arcs, start, count, mode) { From 53ee322ba349b706bace43913c40e60d3f924b81 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 24 Feb 2021 13:20:48 -0800 Subject: [PATCH 10/10] test with parsing: false --- test/fixtures/element.line/skip/middle-span.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/fixtures/element.line/skip/middle-span.js b/test/fixtures/element.line/skip/middle-span.js index 8abfa4c34d8..0df72aca97b 100644 --- a/test/fixtures/element.line/skip/middle-span.js +++ b/test/fixtures/element.line/skip/middle-span.js @@ -1,10 +1,11 @@ module.exports = { config: { type: 'line', + parsing: false, data: { datasets: [ { - data: [{x: 0, y: 10}, {x: 5, y: 0}, {x: NaN, y: -10}, {x: 19, y: -5}], + data: [{x: 0, y: 10}, {x: 5, y: 0}, {x: null, y: -10}, {x: 19, y: -5}], borderColor: 'red', fill: true, spanGaps: true,