Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use custom scale defaults and dataset axis ID options to determine the axis #11134

Merged
merged 8 commits into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .size-limit.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module.exports = [
},
{
path: 'dist/chart.js',
limit: '34 KB',
limit: '34.5 KB',
import: '{ Chart }',
running: false,
modifyWebpackConfig
Expand Down
41 changes: 31 additions & 10 deletions src/core/core.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ function getDefaultScaleIDFromAxis(axis, indexAxis) {
return axis === indexAxis ? '_index_' : '_value_';
}

function idMatchesAxis(id) {
if (id === 'x' || id === 'y' || id === 'r') {
return id;
}
}

function axisFromPosition(position) {
if (position === 'top' || position === 'bottom') {
return 'x';
Expand All @@ -31,20 +37,35 @@ function axisFromPosition(position) {
}
}

export function determineAxis(id, scaleOptions) {
if (id === 'x' || id === 'y' || id === 'r') {
export function determineAxis(id, ...scaleOptions) {
if (idMatchesAxis(id)) {
return id;
etimberg marked this conversation as resolved.
Show resolved Hide resolved
}
for (const opts of scaleOptions) {
const axis = opts.axis
|| axisFromPosition(opts.position)
|| id.length > 1 && idMatchesAxis(id[0].toLowerCase());
if (axis) {
return axis;
}
}
throw new Error(`Cannot determine type of '${id}' axis. Please provide 'axis' or 'position' option.`);
}

id = scaleOptions.axis
|| axisFromPosition(scaleOptions.position)
|| id.length > 1 && determineAxis(id[0].toLowerCase(), scaleOptions);

if (id) {
return id;
function getAxisFromDataset(id, axis, dataset) {
if (dataset[axis + 'AxisID'] === id) {
return {axis};
}
}

throw new Error(`Cannot determine type of '${name}' axis. Please provide 'axis' or 'position' option.`);
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) {
return getAxisFromDataset(id, 'x', boundDs[0]) || getAxisFromDataset(id, 'y', boundDs[0]);
}
}
return {};
}

function mergeScaleConfig(config, options) {
Expand All @@ -62,7 +83,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, 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]]);
Expand Down
86 changes: 86 additions & 0 deletions test/specs/core.scale.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
Expand Down Expand Up @@ -514,6 +543,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',
Expand Down