[charts] Add `minTickLabelGap` to x-axis #16548

bernardobelchior · 2025-02-12T09:40:13Z

Part of #10928.

Changelog

Add minTickLabelGap to x-axis, which allows users to define the minimum gap, in pixels, between two tick labels. The default value is 4px. Make sure to check your charts as the spacing between tick labels might have changed.

mui-bot · 2025-02-12T09:43:55Z

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

Updated pages:

docs/data/migration/migration-charts-v7/migration-charts-v7.md

Generated by 🚫 dangerJS against e701067

bernardobelchior · 2025-02-12T10:19:31Z

packages/x-charts/src/models/axis.ts

+  /**
+   * The minimum gap in pixels between two tick labels.
+   * @default 8
+   */
+  minTickLabelGap?: number;


Added this only for the x-axis because the y-axis doesn't accept discrete data.

what do you mean that the y-axis doesn't accept discrete data? 🤔

Wouldn't it depend on the chart type and orientation?

Wouldn't it depend on the chart type and orientation?

This gap only makes sense for cartesian axes, and I'm assuming that the Y axis represents the second coordinate, regardless of orientation. Is that assumption wrong?

Highcharts does the same. In this example, the x-axis on the left and the y-axis is at the bottom.

Do you think we should not follow this convention? I see some benefits, namely having correct types for the x- and y-axes: since the x-axis is the only one that can accept discrete data, we don't need props related to discrete data in the y-axis.

Let me know your thoughts, maybe I'm missing something or there are some less conventional charts where my assumption is wrong.

Is that assumption wrong?

Yes, and we have a nice counter example for that: the heatmap which is made of two band axes

If you only want to add something to some scale type (point and band in your case) you have the AxisScaleConfig.

Notice this property will only apply on the x-axis because it's the only one to hide some labels.
The reason behind that is not the continuouse/categorical data. It's because x-axis labels are more likely to overflow since they are impacted by labels length

Good points, hadn't thought of that.

So, in theory, the minTickLabelGap can also be applied to the y-axis. However, it seems unlikely we'll need it because of the reason you delineated, so I'm going to assume we're only going to implement this for the x-axis for now.

bernardobelchior · 2025-02-12T11:08:12Z

@alexfauquette @JCQuintas just realized that we have an issue in our measuring code and it doesn't properly measure text sizes when classes are applied.
I think we might need to change our approach because in AxisSharedComponents we're using CSS selectors to properly style elements, however the measurement span isn't a child of Axis, so even if we apply the tickLabel class, it won't be properly styled, causing the measurement to be off.
I'm thinking that we'll need to actually render the labels (maybe with visibility: hidden) and measure them inside the SVG, otherwise we won't be able to get a correct measurement.
Since we're already rendering all labels in SSR, would it make sense to render them with visibility: hidden, measure them, and then create a ResizeObserver (to listen to style changes)? On style change, we'd need to re-render and re-measure the labels.

What do you think?

JCQuintas · 2025-02-12T11:36:14Z

What do you think?

Are you suggesting that we should get the measurements from the actual tick label element instead of a mock element?

then create a ResizeObserver

Is ResizeObserver correct here? You might want a MutationObserver as well/instead

bernardobelchior · 2025-02-12T12:06:40Z

Are you suggesting that we should get the measurements from the actual tick label element instead of a mock element?

Yes.

Is ResizeObserver correct here? You might want a MutationObserver as well/instead

Yeah, I meant a ResizeObserver because we only care about the size of the element. It's fine if a label changes its color: as long as it doesn't impact it's size, we don't need to know about it.

I don't think a MutationObserver would work here because style changes in a class wouldn't be caught. We could only capture changes to a style attribute, as far as I understand.

JCQuintas · 2025-02-12T12:09:25Z

Are you suggesting that we should get the measurements from the actual tick label element instead of a mock element?

Yes.

Is ResizeObserver correct here? You might want a MutationObserver as well/instead

Yeah, I meant a ResizeObserver because we only care about the size of the element. It's fine if a label changes its color: as long as it doesn't impact it's size, we don't need to know about it.

I don't think a MutationObserver would work here because style changes in a class wouldn't be caught. We could only capture changes to a style attribute, as far as I understand.

Ok, I don't see an issue with using the elements themselves, but I didn't build that feature, so maybe @alexfauquette has other concerns

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

bernardobelchior · 2025-02-19T14:26:40Z

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

@@ -96,6 +100,7 @@ const defaultProps = {
  disableLine: false,
  disableTicks: false,
  tickSize: 6,
+  tickLabelMinGap: 4,


I've reduced this to 4px as it seems enough.

Examples with 4px:

Examples with 8px:

bernardobelchior · 2025-02-19T14:30:35Z

@alexfauquette @JCQuintas this is a breaking change in the sense that the previous behavior has been altered, but there's no way to get the exact previous behavior back. We can suggest setting tickLabelMinGap to 0, but the previous implementation always added 20% on top of the width of the tick labels.

As such, this isn't codemod-able, so where should I write documentation to inform users of this change? Is it enough to add a changelog entry or should I also write a section in migration-charts-v7.md about this change?

codspeed-hq · 2025-02-19T14:40:19Z

CodSpeed Performance Report

Merging #16548 will improve performances by 17.31%

_{Comparing bernardobelchior:chart-axis-min-gap (e701067) with master (68412f1)}

Summary

⚡ 2 improvements
✅ 4 untouched benchmarks

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
⚡	`BarChart with big data amount`	584 ms	497.8 ms	+17.31%
⚡	`BarChartPro with big data amount`	868.5 ms	767.4 ms	+13.17%

JCQuintas · 2025-02-19T14:44:53Z

@alexfauquette @JCQuintas this is a breaking change in the sense that the previous behavior has been altered, but there's no way to get the exact previous behavior back. We can suggest setting tickLabelMinGap to 0, but the previous implementation always added 20% on top of the width of the tick labels.

As such, this isn't codemod-able, so where should I write documentation to inform users of this change? Is it enough to add a changelog entry or should I also write a section in migration-charts-v7.md about this change?

Usually we add it on both. The ## Changelog is for information purposes, the migration-charts-v7.md is the actual doc

JCQuintas · 2025-02-19T16:34:52Z

packages/x-charts/src/models/axis.ts

+   * The minimum gap in pixels between two tick labels.
+   * @default 8


Probably good to mention that if two tick labels overlap, one of them will be hidden.

alexfauquette

Comments are about

A slightly less verbose default. But the way it's defined is less obvious, so feel free to ignore
The update of the @default (will require to run scripts again)
Other than that it looks ready to merge 👍

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

packages/x-charts/src/models/axis.ts

alexfauquette · 2025-02-20T09:34:13Z

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

@@ -39,10 +39,14 @@ function addLabelDimension(
  {
    tickLabelStyle: style,
    tickLabelInterval,
+    tickLabelMinGap,


Suggested change

tickLabelMinGap,

tickLabelMinGap = 4,

alexfauquette · 2025-02-20T09:34:40Z

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

@@ -39,10 +39,14 @@ function addLabelDimension(
  {
    tickLabelStyle: style,
    tickLabelInterval,
+    tickLabelMinGap,
    reverse,
    isMounted,
  }: Pick<ChartsXAxisProps, 'tickLabelInterval' | 'tickLabelStyle'> &


Suggested change

}: Pick<ChartsXAxisProps, 'tickLabelInterval' | 'tickLabelStyle'> &

}: Pick<ChartsXAxisProps, 'tickLabelInterval' | 'tickLabelStyle' | 'tickLabelMinGap'> &

alexfauquette · 2025-02-20T09:34:53Z

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

-    Pick<AxisDefaultized, 'reverse'> & { isMounted: boolean },
+    Pick<AxisDefaultized<ScaleName, any, ChartsXAxisProps>, 'reverse'> & {
+      isMounted: boolean;
+      tickLabelMinGap: NonNullable<ChartsXAxisProps['tickLabelMinGap']>;


Suggested change

tickLabelMinGap: NonNullable<ChartsXAxisProps['tickLabelMinGap']>;

alexfauquette · 2025-02-20T09:35:37Z

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

@@ -96,6 +101,7 @@ const defaultProps = {
  disableLine: false,
  disableTicks: false,
  tickSize: 6,
+  tickLabelMinGap: 4,


Suggested change

tickLabelMinGap: 4,

bernardobelchior · 2025-02-20T10:19:13Z

A slightly less verbose default. But the way it's defined is less obvious, so feel free to ignore

I had it this way before, but I changed it to be more in line with other defaults. It probably makes sense to have the defaults in the same place so it's easier to understand where they come from, or do you see any drawbacks with that?

github-actions · 2025-02-20T11:40:07Z

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

bernardobelchior force-pushed the chart-axis-min-gap branch from c5e1697 to 14d559b Compare February 12, 2025 10:13

bernardobelchior commented Feb 12, 2025

View reviewed changes

bernardobelchior force-pushed the chart-axis-min-gap branch 2 times, most recently from 52b38e3 to c549b8e Compare February 12, 2025 10:52

bernardobelchior force-pushed the chart-axis-min-gap branch from c549b8e to 511dcb8 Compare February 18, 2025 16:55

bernardobelchior commented Feb 19, 2025

View reviewed changes

packages/x-charts/src/ChartsXAxis/ChartsXAxis.tsx Outdated Show resolved Hide resolved

bernardobelchior mentioned this pull request Feb 19, 2025

[charts] Improve performance of rendering ticks in x-axis #16536

Merged

bernardobelchior force-pushed the chart-axis-min-gap branch 2 times, most recently from 044d0c5 to 4c8e50c Compare February 19, 2025 14:26

bernardobelchior commented Feb 19, 2025

View reviewed changes

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

bernardobelchior marked this pull request as ready for review February 19, 2025 16:26

bernardobelchior requested review from JCQuintas and alexfauquette February 19, 2025 16:26

bernardobelchior added the breaking change label Feb 19, 2025

JCQuintas reviewed Feb 19, 2025

View reviewed changes

bernardobelchior requested a review from JCQuintas February 20, 2025 07:09

alexfauquette reviewed Feb 20, 2025

View reviewed changes

bernardobelchior requested a review from alexfauquette February 20, 2025 10:21

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

bernardobelchior added 10 commits February 20, 2025 15:10

[charts] Add minTickLabelGap to x-axis

7a2c44a

Revert className in text measurement

4f0b6d1

Rename prop

607066c

Update prop name

1e9f5db

Create default for tickLabelMinGap. Reduce default to 4px.

9bd523b

Fix TS

90763c8

Add info to migration

baf3b0c

Address PR feedback

7f3334a

Update docs

5f0103a

Update default and docs.

e701067

bernardobelchior force-pushed the chart-axis-min-gap branch from d6ce934 to e701067 Compare February 20, 2025 14:13

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

alexfauquette approved these changes Feb 20, 2025

View reviewed changes

bernardobelchior merged commit ff10228 into mui:master Feb 20, 2025
19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[charts] Add `minTickLabelGap` to x-axis #16548

[charts] Add `minTickLabelGap` to x-axis #16548

bernardobelchior commented Feb 12, 2025 •

edited

Loading

mui-bot commented Feb 12, 2025 •

edited

Loading

bernardobelchior Feb 12, 2025

JCQuintas Feb 12, 2025

bernardobelchior Feb 18, 2025

alexfauquette Feb 18, 2025

bernardobelchior Feb 18, 2025

bernardobelchior commented Feb 12, 2025

JCQuintas commented Feb 12, 2025

bernardobelchior commented Feb 12, 2025

JCQuintas commented Feb 12, 2025

bernardobelchior Feb 19, 2025

bernardobelchior commented Feb 19, 2025

codspeed-hq bot commented Feb 19, 2025 •

edited

Loading

JCQuintas commented Feb 19, 2025

JCQuintas Feb 19, 2025

bernardobelchior Feb 20, 2025

alexfauquette left a comment

alexfauquette Feb 20, 2025

alexfauquette Feb 20, 2025

alexfauquette Feb 20, 2025

alexfauquette Feb 20, 2025

bernardobelchior commented Feb 20, 2025 •

edited

Loading

github-actions bot commented Feb 20, 2025

		* The minimum gap in pixels between two tick labels.
		* @default 8

	}: Pick<ChartsXAxisProps, 'tickLabelInterval' \| 'tickLabelStyle'> &
	}: Pick<ChartsXAxisProps, 'tickLabelInterval' \| 'tickLabelStyle' \| 'tickLabelMinGap'> &

[charts] Add minTickLabelGap to x-axis #16548

[charts] Add minTickLabelGap to x-axis #16548

Conversation

bernardobelchior commented Feb 12, 2025 • edited Loading

Changelog

mui-bot commented Feb 12, 2025 • 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

bernardobelchior commented Feb 12, 2025

JCQuintas commented Feb 12, 2025

bernardobelchior commented Feb 12, 2025

JCQuintas commented Feb 12, 2025

Choose a reason for hiding this comment

bernardobelchior commented Feb 19, 2025

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

Merging #16548 will improve performances by 17.31%

Summary

Benchmarks breakdown

JCQuintas commented Feb 19, 2025

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

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

bernardobelchior commented Feb 20, 2025 • edited Loading

github-actions bot commented Feb 20, 2025

[charts] Add `minTickLabelGap` to x-axis #16548

[charts] Add `minTickLabelGap` to x-axis #16548

bernardobelchior commented Feb 12, 2025 •

edited

Loading

mui-bot commented Feb 12, 2025 •

edited

Loading

codspeed-hq bot commented Feb 19, 2025 •

edited

Loading

bernardobelchior commented Feb 20, 2025 •

edited

Loading