[charts] Render axis title within axis size #16730

bernardobelchior · 2025-02-25T13:49:21Z

Render axis title within axis size. Precursor to #16709.

At the moment we hardcode the render position of the axis title regardless of its size or axis height.
The problem of this approach is that increasing the axis height does not change the position of the axis title. This makes it hard to solve issues with overflowing labels that are rotated, since the only way to change the axis title position is to apply a translateY() transformation.

With this PR, we'll position the axis title at the outer edge of the axis container, i.e., bottom edge for an axis with position: 'bottom', left edge for an axis with position: 'left'.

This provides the maximum space for tick labels to be shown.

Changelog

The axis label default position is now set to the outermost bound of the axis size (introduced in v8).
Instead of increasing the chart margin, and translating the label, you can now increase the axis size to get a similar result.

 <LineChart
-   margin: { bottom: 30 } // Add 10px to the bottom margin
    xAxis={[{
-     labelStyle: { transform: `translateY(10px)` }, // translate vertically the label
+    height: 40,  // Add 10px to the default axis height
   }]}
 />

A consequence of this change is that the specificity of some styles changed, so make sure you check your axis label position and appearance after upgrading.

mui-bot · 2025-02-25T13:54:03Z

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

Updated pages:

Generated by 🚫 dangerJS against 72656c3

codspeed-hq · 2025-02-25T13:56:51Z

CodSpeed Performance Report

Merging #16730 will not alter performance

_{Comparing bernardobelchior:label-inside-axis-size (72656c3) with master (7746150)}

Summary

✅ 6 untouched benchmarks

github-actions · 2025-02-25T14:51:16Z

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

bernardobelchior · 2025-02-25T16:27:50Z

@JCQuintas @alexfauquette here a point I'd like your opinion on:

Increase default y-axis width

From the demos that I've updated in this PR, setting width: 60 is the most common when there is an axis label in the y-axis.

I'd suggest updating that default to 60px. We can discuss whether to increase the DEFAULT_AXIS_SIZE_WIDTH or the AXIS_LABEL_DEFAULT_HEIGHT, but I really think we should increase DEFAULT_AXIS_SIZE_WIDTH and maybe reduce the margin.

Here's why: the default label has a font size of 14px and the default tick size is 6px wide. This means that the remaining space must fit the tick label plus some spacing to look good.

With our default configuration, the tick labels "100" and "1,000" are 21.5px and 31.5px wide respectively. The default axis label has a font size of 14px and the default tick size is 6px.

This means that we need at least 41.5px to render an axis label, a tick label with the text "100" and the default tick size. When displaying "1,000", we'll need 51.5px, and this isn't taking into account any spacing we might want to have to ensure an appealing look.

As such, I'd suggest a default of 60px, since I feel like looks good for 1-character labels and 3-character labels. For numbers over 999, it might be still acceptable, but for more digits the axis width would need to be increased.

I propose that we split this increase by setting the default axis width to 40 and the default label width to 20.
To combat the decrease of real estate of the chart content, we could consider reducing the margin.

Here's how it would look like if we reduced the margin to 16px:

Here's an example with 12px:

We'd need to then update some demos, such as this one (margin = 12px):

What do you think? Is it fine to update the axis width? What about the margin?

JCQuintas · 2025-02-25T20:01:05Z

I'm ok with the margin being decreased to 16px. We need a decent margin size in order to acomodate for small overflows

From the screens it doesn't look like we are using all the area we can.

It seems like we could better align the text at the end of the available area?

bernardobelchior · 2025-02-26T06:44:29Z

I'm ok with the margin being decreased to 16px. We need a decent margin size in order to acomodate for small overflows

From the screens it doesn't look like we are using all the area we can.

It seems like we could better align the text at the end of the available area?

I've reduced line-height to 1, which should move the axis labels closer to the edge. From looking at the x position of the text, it seems they're correctly positioned. However, if the text is lowercase it might take less vertical space (before rotation) which makes it look like there's some space left.

Do you know where I can find that last example so I can test it as well?

JCQuintas · 2025-02-26T10:10:56Z

Do you know where I can find that last example so I can test it as well?

I've just updated the demo at with the default values http://localhost:3001/x/react-charts/styling/#placement

JCQuintas · 2025-02-26T10:14:31Z

packages/x-charts/src/ChartsXAxis/ChartsXAxis.tsx

@@ -118,6 +120,7 @@ const defaultProps = {
  disableTicks: false,
  tickSize: 6,
  tickLabelMinGap: 4,
+  height: DEFAULT_AXIS_SIZE_HEIGHT,


This doesn't seem necessary, the height gets initialised elsewhere

Yeah, there seems to be a TypeScript issue because if I remove this I'll get an error.

Our types are a bit complex, so I'm having a hard time understanding how to fix this, but basically there's no place where we're defining that height is actually not undefined.

Do you have any suggestion on how to fix this?

We can do something like this:

export type AxisDefaultized< S extends ScaleName = ScaleName, V = any, AxisProps extends ChartsAxisProps = ChartsXAxisProps | ChartsYAxisProps, > = MakeRequired<Omit<AxisConfig<S, V, AxisProps>, 'scaleType'>, 'offset'> & AxisScaleConfig[S] & AxisSideConfig<AxisProps> & AxisScaleComputedConfig[S] & { /** * An indication of the expected number of ticks. */ tickNumber: number; } & AxisProps extends ChartsXAxisProps ? { height: number } : AxisProps extends ChartsYAxisProps ? { width: number } : never;

However, I'm thinking if it makes sense to separate AxisDefaultized into XAxisDefaultized and YAxisDefaultized. We already have ZAxisDefaultized and it would simplify types by not needing to use extends in some places.

However, I'm thinking if it makes sense to separate AxisDefaultized into XAxisDefaultized and YAxisDefaultized. We already have ZAxisDefaultized and it would simplify types by not needing to use extends in some places.

I thought the same previously, so probably we should try it at least.

You can also probably just mark the entire AxisSideConfig as Required Required<AxisSideConfig<AxisProps>>

export type AxisDefaultized< S extends ScaleName = ScaleName, V = any, AxisProps extends ChartsAxisProps = ChartsXAxisProps | ChartsYAxisProps, > = MakeRequired<Omit<AxisConfig<S, V, AxisProps>, 'scaleType'>, 'offset'> & AxisScaleConfig[S] & Required<AxisSideConfig<AxisProps>> & AxisScaleComputedConfig[S] & { /** * An indication of the expected number of ticks. */ tickNumber: number; };

I don't think I can do Required<AxisSideConfig<AxisProps>> because position can be undefined for axes that aren't the first (source), but I'll do something similar.

MakeOptional<Required<AxisSideConfig<AxisProps>>, 'position'> 🤣

Apparently it breaks types in computeAxisValue 🤔

Also, we might want to add the position from line 48 into the sharedConfig

not sure why I didn't do that, might have side effects

Yeah, there seem to be an error in our TypeScript types.

It seems defaultizeAxis is called before computeAxisValue, but here in computeAxisValue we're stating that the type is AxisConfig instead of AxisDefaultized. This is problematic because I'm getting an error because height/width aren't defined, but they are. It's just that the types are wrong.

I can fix this by adding height: 0 before spreading axis, but it seems to me that our store has incorrect types.

@alexfauquette @JCQuintas are you aware of this? Is there any reason for it?

JCQuintas · 2025-02-26T10:16:17Z

packages/x-charts/src/internals/plugins/featurePlugins/useChartCartesianAxis/defaultizeAxis.ts

+    const height =
+      axisName === 'x'
+        ? DEFAULT_AXIS_SIZE_HEIGHT + (axisConfig.label ? AXIS_LABEL_DEFAULT_HEIGHT : 0)
+        : 0;
+    const width =
+      axisName === 'y'
+        ? DEFAULT_AXIS_SIZE_WIDTH + (axisConfig.label ? AXIS_LABEL_DEFAULT_HEIGHT : 0)
+        : 0;


This makes me think if we should instead of having a fixed height/width. We define it based on the axis extremums? 🤔 We would probably need to move the computation elsewhere, to the cartesianaxis plugin probably.

From what I see the decision is between:

The axis size is fixed, and labels get elpises or wrapped to fit in

The axis size get updated according to its content

The first one seems easier to do and customize than the second one

Yes, but the second one would be better for the user.

Yes, but the second one would be better for the user.

It does not look that obvious. The two behaviors have advantages and issues, depending on the context (how much control you have on the data, labels, and sizing)

Since we are in beta phase, I would be in favor of doing as few last breaking changes on axes as possible. We might go with:

Fix axis sizes with a best effort default (taking into consideration the presence or not of the axis title)

Move the axis title position from fixed point to the end of the axis sizes

Have tick labels cut if they overflow a given dimension

From that point, we would have a significant improvement, and could continue working toward point 2 as an optional configuration.
It would not be breaking changes, because enabling this feature will require to pass some options

I agree that a height: 'auto' or width: 'auto' would be useful, but then we get into other problems (e.g., how much of a chart should a label take up? What happens if a label is larger than the total chart width?).

I agree with Alex that we should keep changes incremental, but I'd also like to eventually have fully responsive axes (potentially with a minSize/maxSize prop). When I was searching for a charting library in the past, this was something that I felt was missing from many libraries and that I ended up having to implement myself, so I think there's value in providing this by default.

alexfauquette

The logic seems good. I just left comment about duplicated places where theme and lineHeight are propagated. Othe than that it's good for me.

I added a changelog in the description. I should probably be added to the migration guide

packages/x-charts/src/ChartsXAxis/ChartsXAxis.tsx

packages/x-charts/src/constants/index.ts

alexfauquette · 2025-02-28T10:07:56Z

packages/x-charts/src/internals/components/AxisSharedComponents.tsx

+     * measurements to be off. As such, it is recommended to apply those properties through the `labelStyle` prop. */
    ...theme.typography.body1,
+    lineHeight: 1,


Suggested change

* measurements to be off. As such, it is recommended to apply those properties through the `labelStyle` prop. */

...theme.typography.body1,

lineHeight: 1,

* measurements to be off. As such, it is recommended to apply those properties through the `labelStyle` prop. */

See #16730 (comment)

The idea of this proposal is to remove the theme.typography.body1 and lineHeight from axis labels since they are already specified in the default style of ChartXAxis and ChartYAxis.

Here you're adding styling to .Mui-ChartAxis-root .Mui-ChartAxis-label but the component will get a styled props that override them with the same value

Sure, I think that works 👍

alexfauquette

Maybe just wait for the relesae before merging 😇

… default values for Y axes.

bernardobelchior added the component: charts This is the name of the generic UI component, not the React module! label Feb 25, 2025

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 25, 2025

bernardobelchior force-pushed the label-inside-axis-size branch from 41164c9 to ad6067b Compare February 25, 2025 14:53

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 25, 2025

JCQuintas reviewed Feb 26, 2025

View reviewed changes

bernardobelchior force-pushed the label-inside-axis-size branch from 4b44a7c to 936112e Compare February 26, 2025 13:52

bernardobelchior requested review from alexfauquette and JCQuintas February 26, 2025 14:24

bernardobelchior force-pushed the label-inside-axis-size branch from 34cfc91 to 7a9cb8b Compare February 27, 2025 09:46

bernardobelchior marked this pull request as ready for review February 27, 2025 14:41

alexfauquette reviewed Feb 28, 2025

View reviewed changes

bernardobelchior force-pushed the label-inside-axis-size branch from a41de1c to 316e6b7 Compare February 28, 2025 10:59

bernardobelchior requested a review from alexfauquette February 28, 2025 11:31

alexfauquette approved these changes Feb 28, 2025

View reviewed changes

bernardobelchior added 9 commits February 28, 2025 17:35

[charts] Render axis title within axis size

01c506e

Remove translates from demos to correctly position axis labels. Add…

3c9f6e8

… default values for Y axes.

Fix demos

07ee590

Reduce line-height. Fix styles on being applied in Chart Y Axis

b4826f0

Remove redundant default. Fix default types.

e13e7d3

Gen demos

94f9b51

Fix types

b2314e7

Accept proposal

6622242

Add migration guide

72656c3

bernardobelchior force-pushed the label-inside-axis-size branch from 33d2701 to 72656c3 Compare February 28, 2025 16:36

bernardobelchior merged commit 824047f into mui:master Feb 28, 2025
19 checks passed

bernardobelchior deleted the label-inside-axis-size branch February 28, 2025 17:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[charts] Render axis title within axis size #16730

[charts] Render axis title within axis size #16730

bernardobelchior commented Feb 25, 2025 •

edited

Loading

mui-bot commented Feb 25, 2025 •

edited

Loading

codspeed-hq bot commented Feb 25, 2025 •

edited

Loading

github-actions bot commented Feb 25, 2025

bernardobelchior commented Feb 25, 2025

JCQuintas commented Feb 25, 2025

bernardobelchior commented Feb 26, 2025

JCQuintas commented Feb 26, 2025

JCQuintas Feb 26, 2025

bernardobelchior Feb 26, 2025

JCQuintas Feb 26, 2025

bernardobelchior Feb 26, 2025

JCQuintas Feb 26, 2025

bernardobelchior Feb 26, 2025

JCQuintas Feb 26, 2025

JCQuintas Feb 26, 2025

bernardobelchior Feb 26, 2025

JCQuintas Feb 26, 2025

alexfauquette Feb 26, 2025

JCQuintas Feb 26, 2025

alexfauquette Feb 26, 2025

bernardobelchior Feb 26, 2025

alexfauquette left a comment •

edited

Loading

alexfauquette Feb 28, 2025

bernardobelchior Feb 28, 2025

alexfauquette Feb 28, 2025

bernardobelchior Feb 28, 2025

alexfauquette left a comment

[charts] Render axis title within axis size #16730

[charts] Render axis title within axis size #16730

Conversation

bernardobelchior commented Feb 25, 2025 • edited Loading

Changelog

mui-bot commented Feb 25, 2025 • edited Loading

codspeed-hq bot commented Feb 25, 2025 • edited Loading

Merging #16730 will not alter performance

Summary

github-actions bot commented Feb 25, 2025

bernardobelchior commented Feb 25, 2025

Increase default y-axis width

JCQuintas commented Feb 25, 2025

bernardobelchior commented Feb 26, 2025

JCQuintas commented Feb 26, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexfauquette left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexfauquette left a comment

Choose a reason for hiding this comment

bernardobelchior commented Feb 25, 2025 •

edited

Loading

mui-bot commented Feb 25, 2025 •

edited

Loading

codspeed-hq bot commented Feb 25, 2025 •

edited

Loading

alexfauquette left a comment •

edited

Loading