From 3a5bc7753208fdb9be3d8b0beb7b9c0783b5452f Mon Sep 17 00:00:00 2001 From: stockiNail Date: Sat, 11 Feb 2023 20:03:35 +0100 Subject: [PATCH 1/8] Use custom scale defaults to determine the axis --- src/core/core.config.js | 16 +++++++--- test/specs/core.scale.tests.js | 57 ++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/core/core.config.js b/src/core/core.config.js index b7362b328c4..7d04398c5f2 100644 --- a/src/core/core.config.js +++ b/src/core/core.config.js @@ -31,20 +31,28 @@ function axisFromPosition(position) { } } -export function determineAxis(id, scaleOptions) { +function getAxis(id, scaleOptions) { if (id === 'x' || id === 'y' || id === 'r') { return id; } id = scaleOptions.axis || axisFromPosition(scaleOptions.position) - || id.length > 1 && determineAxis(id[0].toLowerCase(), scaleOptions); + || id.length > 1 && getAxis(id[0].toLowerCase(), scaleOptions); if (id) { return id; } +} - throw new Error(`Cannot determine type of '${name}' axis. Please provide 'axis' or 'position' option.`); +export function determineAxis(id, ...scaleOptions) { + for (let i = 0, n = scaleOptions.length; i < n; i++) { + const axis = getAxis(id, scaleOptions[i]); + if (axis) { + return axis; + } + } + throw new Error(`Cannot determine type of '${id}' axis. Please provide 'axis' or 'position' option.`); } function mergeScaleConfig(config, options) { @@ -62,7 +70,7 @@ function mergeScaleConfig(config, options) { if (scaleConf._proxy) { return console.warn(`Ignoring resolver passed as options for scale: ${id}`); } - const axis = determineAxis(id, scaleConf); + const axis = determineAxis(id, scaleConf, defaults.scales[scaleConf.type]); const defaultId = getDefaultScaleIDFromAxis(axis, chartIndexAxis); const defaultScaleOptions = chartDefaults.scales || {}; scales[id] = mergeIf(Object.create(null), [{axis}, scaleConf, defaultScaleOptions[axis], defaultScaleOptions[defaultId]]); diff --git a/test/specs/core.scale.tests.js b/test/specs/core.scale.tests.js index d0aa77f60ad..cf5f4f409b7 100644 --- a/test/specs/core.scale.tests.js +++ b/test/specs/core.scale.tests.js @@ -514,6 +514,63 @@ describe('Core.scale', function() { expect(scale._layers()[0].z).toEqual(20); }); + it('should default to one layer for custom scales for axis', function() { + class CustomScale1 extends Chart.Scale { + draw() {} + convertTicksToLabels() { + return ['tick']; + } + } + CustomScale1.id = 'customScale1'; + CustomScale1.defaults = {axis: 'x'}; + Chart.register(CustomScale1); + + var chart = window.acquireChart({ + type: 'line', + options: { + scales: { + my: { + type: 'customScale1', + grid: { + z: 10 + }, + ticks: { + z: 20 + } + } + } + } + }); + + var scale = chart.scales.my; + expect(scale._layers().length).toEqual(1); + expect(scale._layers()[0].z).toEqual(20); + }); + + it('should fail for custom scales without any axis or position', function() { + class CustomScale2 extends Chart.Scale { + draw() {} + } + CustomScale2.id = 'customScale2'; + CustomScale2.defaults = {}; + Chart.register(CustomScale2); + + function createChart() { + return window.acquireChart({ + type: 'line', + options: { + scales: { + my: { + type: 'customScale2' + } + } + } + }); + } + + expect(createChart).toThrow(new Error('Cannot determine type of \'my\' axis. Please provide \'axis\' or \'position\' option.')); + }); + it('should return 3 layers when z is not equal between ticks and grid', function() { var chart = window.acquireChart({ type: 'line', From 5c676887d10d3186a620344fde006f67d71d6048 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Sat, 11 Feb 2023 21:02:59 +0100 Subject: [PATCH 2/8] add check of datasets x/yAxisID options --- src/core/core.config.js | 18 +++++++++++++++++- test/specs/core.scale.tests.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/core/core.config.js b/src/core/core.config.js index 7d04398c5f2..90465eb1d4b 100644 --- a/src/core/core.config.js +++ b/src/core/core.config.js @@ -55,6 +55,22 @@ export function determineAxis(id, ...scaleOptions) { throw new Error(`Cannot determine type of '${id}' axis. Please provide 'axis' or 'position' option.`); } +function getScaleFromDataset(id, axis, dataset) { + if (dataset[axis + 'AxisID'] === id) { + return {axis}; + } +} + +function searchScaleIDs(id, config) { + if (config.data && config.data.datasets) { + const boundDs = config.data.datasets.filter((d) => d.xAxisID === id || d.yAxisID === id); + if (boundDs.length) { + return getScaleFromDataset(id, 'x', boundDs[0]) || getScaleFromDataset(id, 'y', boundDs[0]); + } + } + return {}; +} + function mergeScaleConfig(config, options) { const chartDefaults = overrides[config.type] || {scales: {}}; const configScales = options.scales || {}; @@ -70,7 +86,7 @@ function mergeScaleConfig(config, options) { if (scaleConf._proxy) { return console.warn(`Ignoring resolver passed as options for scale: ${id}`); } - const axis = determineAxis(id, scaleConf, defaults.scales[scaleConf.type]); + const axis = determineAxis(id, scaleConf, searchScaleIDs(id, config), defaults.scales[scaleConf.type]); const defaultId = getDefaultScaleIDFromAxis(axis, chartIndexAxis); const defaultScaleOptions = chartDefaults.scales || {}; scales[id] = mergeIf(Object.create(null), [{axis}, scaleConf, defaultScaleOptions[axis], defaultScaleOptions[defaultId]]); diff --git a/test/specs/core.scale.tests.js b/test/specs/core.scale.tests.js index cf5f4f409b7..caa3c8f32ea 100644 --- a/test/specs/core.scale.tests.js +++ b/test/specs/core.scale.tests.js @@ -481,6 +481,35 @@ describe('Core.scale', function() { expect(scale._layers().length).toEqual(3); }); + it('should create the chart with custom scale ids without axis or position options', function() { + function createChart() { + return window.acquireChart({ + type: 'scatter', + data: { + datasets: [{ + data: [{x: 0, y: 0}, {x: 1, y: 1}, {x: 2, y: 2}], + xAxisID: 'customIDx', + yAxisID: 'customIDy' + }] + }, + options: { + scales: { + customIDx: { + type: 'linear', + display: false + }, + customIDy: { + type: 'linear', + display: false + } + } + } + }); + } + + expect(createChart).not.toThrow(); + }); + it('should default to one layer for custom scales', function() { class CustomScale extends Chart.Scale { draw() {} From 358772dd741156972117ecdeed6ecf5b96cb1ec6 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Sat, 11 Feb 2023 21:10:21 +0100 Subject: [PATCH 3/8] try fix size limit --- .size-limit.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.cjs b/.size-limit.cjs index 2f4e5e4c67d..2b421ffcb09 100644 --- a/.size-limit.cjs +++ b/.size-limit.cjs @@ -13,7 +13,7 @@ module.exports = [ }, { path: 'dist/chart.js', - limit: '34 KB', + limit: '34.5 KB', import: '{ Chart }', running: false, modifyWebpackConfig From a9880d6da5ac41cf326c9bfc75a9cb8c7050b302 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Sat, 11 Feb 2023 21:32:21 +0100 Subject: [PATCH 4/8] rename functions --- src/core/core.config.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/core.config.js b/src/core/core.config.js index 90465eb1d4b..29944a0ab5a 100644 --- a/src/core/core.config.js +++ b/src/core/core.config.js @@ -55,17 +55,17 @@ export function determineAxis(id, ...scaleOptions) { throw new Error(`Cannot determine type of '${id}' axis. Please provide 'axis' or 'position' option.`); } -function getScaleFromDataset(id, axis, dataset) { +function getAxisFromDataset(id, axis, dataset) { if (dataset[axis + 'AxisID'] === id) { return {axis}; } } -function searchScaleIDs(id, config) { +function retrieveAxisOnDatasets(id, config) { if (config.data && config.data.datasets) { const boundDs = config.data.datasets.filter((d) => d.xAxisID === id || d.yAxisID === id); if (boundDs.length) { - return getScaleFromDataset(id, 'x', boundDs[0]) || getScaleFromDataset(id, 'y', boundDs[0]); + return getAxisFromDataset(id, 'x', boundDs[0]) || getAxisFromDataset(id, 'y', boundDs[0]); } } return {}; @@ -86,7 +86,7 @@ function mergeScaleConfig(config, options) { if (scaleConf._proxy) { return console.warn(`Ignoring resolver passed as options for scale: ${id}`); } - const axis = determineAxis(id, scaleConf, searchScaleIDs(id, config), defaults.scales[scaleConf.type]); + const axis = determineAxis(id, scaleConf, retrieveAxisOnDatasets(id, config), defaults.scales[scaleConf.type]); const defaultId = getDefaultScaleIDFromAxis(axis, chartIndexAxis); const defaultScaleOptions = chartDefaults.scales || {}; scales[id] = mergeIf(Object.create(null), [{axis}, scaleConf, defaultScaleOptions[axis], defaultScaleOptions[defaultId]]); From 0f535655bc44c22d835f5f6b49a6928bb9e87da8 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Sat, 11 Feb 2023 21:51:28 +0100 Subject: [PATCH 5/8] some improvements --- src/core/core.config.js | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/core/core.config.js b/src/core/core.config.js index 29944a0ab5a..4c80fc382ea 100644 --- a/src/core/core.config.js +++ b/src/core/core.config.js @@ -22,6 +22,12 @@ function getDefaultScaleIDFromAxis(axis, indexAxis) { return axis === indexAxis ? '_index_' : '_value_'; } +function axisFromId(id) { + if (id === 'x' || id === 'y' || id === 'r') { + return id; + } +} + function axisFromPosition(position) { if (position === 'top' || position === 'bottom') { return 'x'; @@ -31,23 +37,15 @@ function axisFromPosition(position) { } } -function getAxis(id, scaleOptions) { - if (id === 'x' || id === 'y' || id === 'r') { - return id; - } - - id = scaleOptions.axis - || axisFromPosition(scaleOptions.position) - || id.length > 1 && getAxis(id[0].toLowerCase(), scaleOptions); - - if (id) { +export function determineAxis(id, ...scaleOptions) { + if (axisFromId(id)) { return id; } -} - -export function determineAxis(id, ...scaleOptions) { for (let i = 0, n = scaleOptions.length; i < n; i++) { - const axis = getAxis(id, scaleOptions[i]); + const opts = scaleOptions[i]; + const axis = opts.axis + || axisFromPosition(opts.position) + || id.length > 1 && axisFromId(id[0].toLowerCase()); if (axis) { return axis; } @@ -61,7 +59,7 @@ function getAxisFromDataset(id, axis, dataset) { } } -function retrieveAxisOnDatasets(id, config) { +function retrieveAxisFromDatasets(id, config) { if (config.data && config.data.datasets) { const boundDs = config.data.datasets.filter((d) => d.xAxisID === id || d.yAxisID === id); if (boundDs.length) { @@ -86,7 +84,7 @@ function mergeScaleConfig(config, options) { if (scaleConf._proxy) { return console.warn(`Ignoring resolver passed as options for scale: ${id}`); } - const axis = determineAxis(id, scaleConf, retrieveAxisOnDatasets(id, config), defaults.scales[scaleConf.type]); + const axis = determineAxis(id, scaleConf, retrieveAxisFromDatasets(id, config), defaults.scales[scaleConf.type]); const defaultId = getDefaultScaleIDFromAxis(axis, chartIndexAxis); const defaultScaleOptions = chartDefaults.scales || {}; scales[id] = mergeIf(Object.create(null), [{axis}, scaleConf, defaultScaleOptions[axis], defaultScaleOptions[defaultId]]); From 6fe0e2e742d1189098276c00c02b8b3f0acd3e62 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Sun, 12 Feb 2023 11:20:53 +0100 Subject: [PATCH 6/8] apply review --- src/core/core.config.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/core/core.config.js b/src/core/core.config.js index 4c80fc382ea..45ebaa50033 100644 --- a/src/core/core.config.js +++ b/src/core/core.config.js @@ -22,7 +22,7 @@ function getDefaultScaleIDFromAxis(axis, indexAxis) { return axis === indexAxis ? '_index_' : '_value_'; } -function axisFromId(id) { +function idMatchesAxis(id) { if (id === 'x' || id === 'y' || id === 'r') { return id; } @@ -38,14 +38,15 @@ function axisFromPosition(position) { } export function determineAxis(id, ...scaleOptions) { - if (axisFromId(id)) { - return id; + let axis = idMatchesAxis(id); + if (axis) { + return axis; } for (let i = 0, n = scaleOptions.length; i < n; i++) { const opts = scaleOptions[i]; - const axis = opts.axis + axis = opts.axis || axisFromPosition(opts.position) - || id.length > 1 && axisFromId(id[0].toLowerCase()); + || id.length > 1 && idMatchesAxis(id[0].toLowerCase()); if (axis) { return axis; } From cf95747704ddfb059320c0a4e32924641b58f2d3 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Sun, 12 Feb 2023 11:31:24 +0100 Subject: [PATCH 7/8] back idMatchesAxis --- src/core/core.config.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/core/core.config.js b/src/core/core.config.js index 45ebaa50033..e412668b30e 100644 --- a/src/core/core.config.js +++ b/src/core/core.config.js @@ -38,13 +38,12 @@ function axisFromPosition(position) { } export function determineAxis(id, ...scaleOptions) { - let axis = idMatchesAxis(id); - if (axis) { - return axis; + if (idMatchesAxis(id)) { + return id; } for (let i = 0, n = scaleOptions.length; i < n; i++) { const opts = scaleOptions[i]; - axis = opts.axis + const axis = opts.axis || axisFromPosition(opts.position) || id.length > 1 && idMatchesAxis(id[0].toLowerCase()); if (axis) { From f0ac2b87d7838a0f31ee7d8ff4d56b5ea4ab0df1 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Mon, 13 Feb 2023 14:18:37 +0100 Subject: [PATCH 8/8] apply review --- src/core/core.config.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/core.config.js b/src/core/core.config.js index e412668b30e..209e87291ab 100644 --- a/src/core/core.config.js +++ b/src/core/core.config.js @@ -41,8 +41,7 @@ export function determineAxis(id, ...scaleOptions) { if (idMatchesAxis(id)) { return id; } - for (let i = 0, n = scaleOptions.length; i < n; i++) { - const opts = scaleOptions[i]; + for (const opts of scaleOptions) { const axis = opts.axis || axisFromPosition(opts.position) || id.length > 1 && idMatchesAxis(id[0].toLowerCase());