[charts] Refactor axis band scaleType check

[JCQuintas]

• Move "scale type check" to its own function so it is easier to reason with
• Added tests in order to properly refactor code

This was done as part of #12537 to prepare for extended checks

[mui-bot]

Deploy preview: https://deploy-preview-13295--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 5b3d3f3

[alexfauquette]

Nice refactoring. Just left nitpick comments

[alexfauquette]

For logic that impacts the rendering, I agree we can test the logic function independently because the difficulty of maintaining a test that checks element coordinates is annoying

But for errors/warnings, we could test directly the <BarChart /> such that test breaks if for example, we delete the call to checkScaleErrors

[JCQuintas]

yeah I just didn't invest into setting up react testing, as I didn't find any example in the charts package 😅

[JCQuintas]

Do you think we should go straight with playwright for this? Or run them in mocha?

[alexfauquette]

Suggested change

	const discreteAxisKey = verticalLayout ? xAxisKey : yAxisKey;
	const continuousAxisKey = verticalLayout ? yAxisKey : xAxisKey;
	const discreteAxisDirection = verticalLayout ? 'x' : 'y';
	const continuousAxisDirection = verticalLayout ? 'y' : 'x';

[alexfauquette]

Same adaptation would be needed on the two other lines

Suggested change

	`MUI X Charts: ${getAxisMessage(verticalLayout, discreteAxisKey)} should be of type "band" to display the bar series of id "${seriesId}".`,
	`MUI X Charts: ${getAxisMessage(discreteAxisDirection)} should be of type "band" to display the bar series of id "${seriesId}".`,

[alexfauquette]

Suggested change

	const getAxisMessage = (isVertical: boolean, axisKey: string, isContinuous: boolean = false) => {
	const axisName = getXOrY(isVertical, isContinuous, 'x-axis', 'y-axis');
	const axisKeyName = getXOrY(isVertical, isContinuous, 'xAxis', 'yAxis');
	const axisDefaultKey = getXOrY(isVertical, isContinuous, DEFAULT_X_AXIS_KEY, DEFAULT_Y_AXIS_KEY);
	return axisKey === axisDefaultKey
	? `The first \`${axisKeyName}\``
	: `The ${axisName} with id "${axisKey}"`;
	};
	const getAxisMessage = (axisDirection: 'x' | 'y') => {
	const axisName = `${axisDirection}-axis`;
	const axisKeyName = `${axisDirection}Axis`;
	const axisDefaultKey = axisDirection === 'x'? DEFAULT_X_AXIS_KEY : DEFAULT_Y_AXIS_KEY;
	return axisKey === axisDefaultKey
	? `The first \`${axisKeyName}\``
	: `The ${axisName} with id "${axisKey}"`;
	};

[JCQuintas]

Issue is that we have both discrete and continuous, and one is the inverse of the other. We also need the proper axisKey for the error message.

[alexfauquette]

The idea was to move this logic in a single place

const discreteAxisKey = verticalLayout ? xAxisKey : yAxisKey;
const continuousAxisKey = verticalLayout ? yAxisKey : xAxisKey;
+ const discreteAxisDirection = verticalLayout ? 'x' : 'y';
+ const continuousAxisDirection = verticalLayout ? 'y' : 'x';

And then if you do a test on the discreteAxisKey you call the function with discreteAxisDirection. And if you do a test on continuousAxisKey you call the error message with continuousAxisDirection

[JCQuintas]

🤦