[charts] Decouple margin and axis-size

[JCQuintas]

Fixes #16387
Fixes #16383

Changelog

Breaking change

• Replace topAxis, rightAxis, bottomAxis, and leftAxis props by position in the axis config

  The following props have been removed from the axis config: topAxis, rightAxis, bottomAxis, and leftAxis.

  Replaced the topAxis, rightAxis, bottomAxis and leftAxis props by a position prop in the axis config that accepts 'top' | 'right' | 'bottom' | 'left' | 'none'.

  If you were previously disabling an axis by setting it to null, you should now set its position to 'none'.

   <LineChart
     yAxis={[
       {
         scaleType: 'linear',
  +      position: 'right',
       },
     ]}
     series={[{ data: [1, 10, 30, 50, 70, 90, 100], label: 'linear' }]}
     height={400}
  -  rightAxis={{}}
   />

• Remove position prop from ChartsXAxis and ChartsYAxis

  The position prop has been removed from the ChartsXAxis and ChartsYAxis components. Configure it directly in the axis config.

   <ChartContainer
     yAxis={[
       {
         id: 'my-axis',
  +      position: 'right',
       },
     ]}
   >
  -  <ChartsYAxis axisId="my-axis" position="right" />
  +  <ChartsYAxis axisId="my-axis" />
   </ChartContainer>

[mui-bot]

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

Updated pages:

• docs/data/charts/axis/axis.md
• docs/data/charts/composition/composition.md
• docs/data/migration/migration-charts-v7/migration-charts-v7.md

Generated by 🚫 dangerJS against acbc0eb

[codspeed-hq]

CodSpeed Performance Report

Merging #16418 will not alter performance

_{Comparing JCQuintas:axis-size-poc (acbc0eb) with master (15f5a84)}

Summary

✅ 6 untouched benchmarks

[JCQuintas]

@alexfauquette having the config in either the plugin or in the component cause a re-render which slows up the chart.

An alternative could be to get rid of left/right/top/bottomAxis properties, move all the config into the x/yAxis, which will then process everything before they render as far as I'm aware. 🤔

eg:

<LineChart
  xAxis={[
    {
      data: ['A', 'B', 'C', 'D'],
      scaleType: 'point',
      position: 'right',
    },
  ]}
  series={[{ data: [40, 30, 20, 10] }]}
  height={200}
/>

[alexfauquette]

An alternative could be to get rid of left/right/top/bottomAxis properties, move all the config into the x/yAxis, which will then process everything before they render as far as I'm aware. 🤔

Sounds reasonable. Wwe will need to find a hack to let the selectorChartDrawingArea accept to support this new notion of axis dimensions depending on the existence of the cartesian axis plugging or not.

You would still have the plugin in the following order "dimensions > series > axes" but the dimensions could have a selector that uses xAxis and yAxis config if available. The selectorChartDrawingArea would depend on two selectors selectorChartMargin and selectorChartAxisSpace.

If later we want to adapt the size according the real space an axis takes we will also need to take into account the zoom and the tick we render.

I assume the idea is then to have something like

xAxis={[
    {
      data: ['A', 'B', 'C', 'D'],
      scaleType: 'point',
      position: 'right',
      width: 95 // specify the space taken by the axis.
    },
  ]}

[alexfauquette]

By the way, the removal of leftAxis, ... can be done in a non breaking way.

We can deprecate the props and keep default the same

[JCQuintas]

@mui/xcharts what do you think about this API? I still have to make height/width count towards the margins, but it was trivial to allow multiple axis on a side in this implementation 😄

import * as React from 'react';
import { BarChart } from '@mui/x-charts/BarChart';

export default function BasicBars() {
  return (
    <BarChart
      xAxis={[
        { scaleType: 'band', data: ['group A', 'group B', 'group C'], position: 'top' },
        { scaleType: 'band', data: ['group 1', 'group 2', 'group 3'], position: 'top' }
      ]}
      series={[{ data: [4, 3, 5] }, { data: [1, 6, 3] }, { data: [2, 5, 6] }]}
      margin={60}
      width={500}
      height={300}
    />
  );
}

[alexfauquette]

I was effectively thinking of this issue #16387 about multiple axis as a nice example of what this new axis config could fix :)

I'm good with the API, and adding width and height as properties that get defaulted on x/y axes.
I would be against something like { position: 'top', size: { width: 50, height: 150 } } because for x-axis user will only care about the height, and with y-axis only about width.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[JCQuintas]

@mui/xcharts one possible disadvantage of the current approach is that the axis is being counted for sizing purposes even if ChartsAxis is not present in the document.

Since we have "defaults", I guess it would be difficult to prevent this kind of behavior :/

[alexfauquette]

You have an error in one of the screen shots (the axis/HidingAxis.tsx) All the other looks good

About the sparkling, it's not an issue for me. It's part of the tradeoff we discussed. If someone does not render the axes (like us in the sparkling) or do a custom rendering. They should update the position and sizing of the axis.

[alexfauquette]

It looks a bit duplicated with the title. Maybe something more like

The following props have been removed topAxis, rightAxis, bottomAxis, and leftAxis.
To modify the axis placement, use the new position property in the axis config.
It accepts 'top' | 'right' | 'bottom' | 'left' | 'none'.

[alexfauquette]

Maybe also at the end a small sentence to highlight why doing that:

Notice this new API allows you to [stack multiple axes on the same side of the chart](link to the demo)

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[alexfauquette]

🚀

[bernardobelchior]

Left a comment, not a blocker, but probably something we want to tackle post-merge.

[bernardobelchior]

Is this up to date? The axes are no longer part of the margin, are they?

Also, we now also accept a single number, right?