From 8ff853c60e6cb9617fdadbfad415f68760844e96 Mon Sep 17 00:00:00 2001 From: Dan Onoshko Date: Fri, 2 Sep 2022 08:52:48 +0400 Subject: [PATCH] feat: remove default axis override when custom id is given (#10643) * feat: remove default axis override when custom id is given * docs: add info into migration guide * test: fix tests for the feat * docs: add info into migration guide * test: fix tests for the feat * feat: review fixes * feat: review fixes --- docs/migration/v4-migration.md | 1 + src/core/core.config.js | 17 +++- .../controller.bar/stacking/issue-9105.js | 10 +- .../fixtures/core.scale/ticks/rotated-long.js | 4 +- .../core.scale/ticks/rotated-multi-line.js | 4 +- test/fixtures/mixed/bar+line.js | 4 +- test/specs/core.controller.tests.js | 1 + test/specs/core.scale.tests.js | 98 +++++++++++++++++++ types/index.d.ts | 4 +- 9 files changed, 125 insertions(+), 18 deletions(-) diff --git a/docs/migration/v4-migration.md b/docs/migration/v4-migration.md index 566f2e080..c2700528b 100644 --- a/docs/migration/v4-migration.md +++ b/docs/migration/v4-migration.md @@ -7,6 +7,7 @@ Chart.js 4.0 introduces a number of breaking changes. We tried keeping the amoun ### Charts * Charts don't override the default tooltip callbacks, so all chart types have the same-looking tooltips. +* Default scale override has been removed if the configured scale starts with `x`/`y`. Defining `xAxes` in your config will now create a second scale instead of overriding the default `x` axis. ### Options diff --git a/src/core/core.config.js b/src/core/core.config.js index 028266678..630c21e89 100644 --- a/src/core/core.config.js +++ b/src/core/core.config.js @@ -32,17 +32,25 @@ function axisFromPosition(position) { } export function determineAxis(id, scaleOptions) { - if (id === 'x' || id === 'y') { + if (id === 'x' || id === 'y' || id === 'r') { return id; } - return scaleOptions.axis || axisFromPosition(scaleOptions.position) || id.charAt(0).toLowerCase(); + + id = scaleOptions.axis + || axisFromPosition(scaleOptions.position) + || id.length > 1 && determineAxis(id[0].toLowerCase(), scaleOptions); + + if (id) { + return id; + } + + throw new Error(`Cannot determine type of '${name}' axis. Please provide 'axis' or 'position' option.`); } function mergeScaleConfig(config, options) { const chartDefaults = overrides[config.type] || {scales: {}}; const configScales = options.scales || {}; const chartIndexAxis = getIndexAxis(config.type, options); - const firstIDs = Object.create(null); const scales = Object.create(null); // First figure out first scale id's per axis. @@ -57,7 +65,6 @@ function mergeScaleConfig(config, options) { const axis = determineAxis(id, scaleConf); const defaultId = getDefaultScaleIDFromAxis(axis, chartIndexAxis); const defaultScaleOptions = chartDefaults.scales || {}; - firstIDs[axis] = firstIDs[axis] || id; scales[id] = mergeIf(Object.create(null), [{axis}, scaleConf, defaultScaleOptions[axis], defaultScaleOptions[defaultId]]); }); @@ -69,7 +76,7 @@ function mergeScaleConfig(config, options) { const defaultScaleOptions = datasetDefaults.scales || {}; Object.keys(defaultScaleOptions).forEach(defaultID => { const axis = getAxisFromDefaultScaleID(defaultID, indexAxis); - const id = dataset[axis + 'AxisID'] || firstIDs[axis] || axis; + const id = dataset[axis + 'AxisID'] || axis; scales[id] = scales[id] || Object.create(null); mergeIf(scales[id], [{axis}, configScales[id], defaultScaleOptions[defaultID]]); }); diff --git a/test/fixtures/controller.bar/stacking/issue-9105.js b/test/fixtures/controller.bar/stacking/issue-9105.js index 200ce4d30..992a245ac 100644 --- a/test/fixtures/controller.bar/stacking/issue-9105.js +++ b/test/fixtures/controller.bar/stacking/issue-9105.js @@ -10,31 +10,31 @@ module.exports = { label: 'Dataset 1', data: [12, 19, 3, 5, 2, 3], stack: '0', - yAxisID: 'y-axis-1' + yAxisID: 'y' }, { backgroundColor: 'rgba(54,162,235,0.8)', label: 'Dataset 2', data: [13, 19, 3, 5, 8, 3], stack: '0', - yAxisID: 'y-axis-1' + yAxisID: 'y' }, { backgroundColor: 'rgba(75,192,192,0.8)', label: 'Dataset 3', data: [13, 19, 3, 5, 8, 3], stack: '0', - yAxisID: 'y-axis-1' + yAxisID: 'y' } ] }, options: { plugins: false, scales: { - xaxis: { + x: { display: false, }, - 'y-axis-1': { + y: { display: false } } diff --git a/test/fixtures/core.scale/ticks/rotated-long.js b/test/fixtures/core.scale/ticks/rotated-long.js index 3009d4cda..b8bed15a4 100644 --- a/test/fixtures/core.scale/ticks/rotated-long.js +++ b/test/fixtures/core.scale/ticks/rotated-long.js @@ -17,11 +17,11 @@ module.exports = { title: false }, scales: { - bottom: { + x: { type: 'category', position: 'bottom' }, - top: { + x2: { type: 'category', position: 'top' } diff --git a/test/fixtures/core.scale/ticks/rotated-multi-line.js b/test/fixtures/core.scale/ticks/rotated-multi-line.js index 271c0b312..82fec478a 100644 --- a/test/fixtures/core.scale/ticks/rotated-multi-line.js +++ b/test/fixtures/core.scale/ticks/rotated-multi-line.js @@ -17,11 +17,11 @@ module.exports = { title: false }, scales: { - bottom: { + x: { type: 'category', position: 'bottom' }, - top: { + x2: { type: 'category', position: 'top' } diff --git a/test/fixtures/mixed/bar+line.js b/test/fixtures/mixed/bar+line.js index f6bbfe8cb..ca6e81548 100644 --- a/test/fixtures/mixed/bar+line.js +++ b/test/fixtures/mixed/bar+line.js @@ -19,10 +19,10 @@ module.exports = { options: { indexAxis: 'y', scales: { - horz: { + x: { position: 'top' }, - vert: { + y: { axis: 'y', labels: ['a', 'b', 'c', 'd'] } diff --git a/test/specs/core.controller.tests.js b/test/specs/core.controller.tests.js index e30da0fe9..a75267b8d 100644 --- a/test/specs/core.controller.tests.js +++ b/test/specs/core.controller.tests.js @@ -443,6 +443,7 @@ describe('Chart', function() { options: { scales: { foo: { + axis: 'x', type: 'logarithmic', _jasmineCheckC: 'c2', _jasmineCheckD: 'd2' diff --git a/test/specs/core.scale.tests.js b/test/specs/core.scale.tests.js index 04ce097ed..e5e94926d 100644 --- a/test/specs/core.scale.tests.js +++ b/test/specs/core.scale.tests.js @@ -571,4 +571,102 @@ describe('Core.scale', function() { expect(chart.scales.y.max).toEqual(10); }); }); + + describe('overrides', () => { + it('should create new scale', () => { + const chart = window.acquireChart({ + type: 'scatter', + data: { + datasets: [{ + data: [{x: 100, y: 100}, {x: -100, y: -100}] + }, { + data: [{x: 10, y: 10}, {x: -10, y: -10}] + }] + }, + options: { + scales: { + x2: { + type: 'linear', + min: -20, + max: 20 + } + } + } + }); + + expect(Object.keys(chart.scales).sort()).toEqual(['x', 'x2', 'y']); + }); + + it('should create new scale with custom name', () => { + const chart = window.acquireChart({ + type: 'scatter', + data: { + datasets: [{ + data: [{x: 100, y: 100}, {x: -100, y: -100}] + }, { + data: [{x: 10, y: 10}, {x: -10, y: -10}] + }] + }, + options: { + scales: { + scaleX: { + axis: 'x', + type: 'linear', + min: -20, + max: 20 + } + } + } + }); + + expect(Object.keys(chart.scales).sort()).toEqual(['scaleX', 'x', 'y']); + }); + + it('should throw error on scale with custom name without axis type', () => { + expect(() => window.acquireChart({ + type: 'scatter', + data: { + datasets: [{ + data: [{x: 100, y: 100}, {x: -100, y: -100}] + }, { + data: [{x: 10, y: 10}, {x: -10, y: -10}] + }] + }, + options: { + scales: { + scaleX: { + type: 'linear', + min: -20, + max: 20 + } + } + } + })).toThrow(); + }); + + it('should read options first to determine axis', () => { + const chart = window.acquireChart({ + type: 'scatter', + data: { + datasets: [{ + data: [{x: 100, y: 100}, {x: -100, y: -100}] + }, { + data: [{x: 10, y: 10}, {x: -10, y: -10}] + }] + }, + options: { + scales: { + xavier: { + axis: 'y', + type: 'linear', + min: -20, + max: 20 + } + } + } + }); + + expect(chart.scales.xavier.axis).toBe('y'); + }); + }); }); diff --git a/types/index.d.ts b/types/index.d.ts index 06803e8b1..a992e62f1 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -3039,9 +3039,9 @@ export interface CartesianScaleOptions extends CoreScaleOptions { stackWeight?: number; /** - * Which type of axis this is. Possible values are: 'x', 'y'. If not set, this is inferred from the first character of the ID which should be 'x' or 'y'. + * Which type of axis this is. Possible values are: 'x', 'y', 'r'. If not set, this is inferred from the first character of the ID which should be 'x', 'y' or 'r'. */ - axis: 'x' | 'y'; + axis: 'x' | 'y' | 'r'; /** * User defined minimum value for the scale, overrides minimum value from data. -- 2.47.3