[charts] Allow series.label property to receive a function with the "location" it is going to be displayed on

[JCQuintas]

• Allow series.label property to receive a function with the "location" it is going to be displayed on
• location options are legend | tooltip for Series, Scatter and Line
• And legend | tooltip | arc for Pie
• Small typos fixed
• Added label documentation as a standalone page

resolves #12482

Todo:

• Example usage
• Typings documentation

[JCQuintas]

@alexfauquette what do you think about having a labelFormatter on the series in order to solve #12482?

https://codesandbox.io/p/sandbox/12482-labelformatter-dj3d9c?file=%2Fsrc%2Fdemo.tsx%3A8%2C32

[mui-bot]

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

Updated pages:

• docs/data/charts/label/label.md

Generated by 🚫 dangerJS against 40eb3e9

[alexfauquette]

what do you think about having a labelFormatter on the series in order to solve #12482?

IMHO the formatter is useful to define a rendering once and use it multiple times.

For example with valueFormatter, you define it once for the series, and it's applied to all the values.

Since series have one label that can be displayed in the tooltip or the legend, you end up with only two cases.

From a DX point of view, this syntax seems easier to read and document:

label: 'Duration',
- labelFormatter: (v, { location }) => location === 'tooltip' ? `${v} (HH:MM:SS)` : v,
+ labelTooltip: 'Duration (HH:MM:SS)",

[JCQuintas]

From a DX point of view, this syntax seems easier to read and document:

I kind of agree, but we have Pie charts which would mean we need to add labelTooltip to the data as well.

So pretty much series.labelTooltip and when type=pie series[].data[].labelTooltip. If that seems better, should I also change arcLabel to accept 'labelTooltip'?

[alexfauquette]

I kind of agree, but we have Pie charts which would mean we need to add labelTooltip to the data as well.

Always a pain to support pie charts :)

What about having tooltipLabel and legendLabel (on the same idea as arcLabel).

It would be two formatter with type ((item: DefaultizedPieValueType) => string). For those two new ones, we don't have to add 'formattedValue' | 'label' | 'value' because only 'label' makes sense for the tooltip/legend.

[JCQuintas]

Yeah, Pie charts are also odd that we use their dataset as "series" and that messes with my feelings a lot 😛

[JCQuintas]

So thought a bit more about this, and I don't see a lot of value in having all these different formatters when one with a context would do 🤔

This PR implements

series[].label: string
series[].labelFormatter: ((context: SeriesLabelFormatterContext) => string)

series[type=pie].data[].label = string
series[type=pie].data[].arcLabel = 'formattedValue' | 'label' | 'value'| ((item: DefaultizedPieValueType) => string)

Your suggestion would be

series[].legend = 'string'
series[].tooltipLabel = (item: DefaultizedPieValueType) => string
series[].legendLabel = (item: DefaultizedPieValueType) => string

series[type=pie].data[].label = 'string'
series[type=pie].data[].arcLabel = 'formattedValue' | 'label' | 'value'| ((item: DefaultizedPieValueType) => string)
series[type=pie].data[].tooltipLabel = (item: DefaultizedPieValueType) => string
series[type=pie].data[].legendLabel = (item: DefaultizedPieValueType) => string

Is this correct?

I think we could build on the first case to make it smaller, like, if formatting the label is important, rather than having label be a a string, we could also allow functions

series[].label: string | ((context: SeriesLabelFormatterContext) => string);

series[type=pie].data[].label = string | ((context: SeriesLabelFormatterContext) => string)
series[type=pie].data[].arcLabel = 'formattedValue' | 'label' | 'value'| ((item: DefaultizedPieValueType) => string)

Would this be a good approach?

[alexfauquette]

Would this be a good approach?

Yes, that looks even better 👍

What about replacing the context with just the location. and we will add the context if necessary latter as a second argument:

- series[].label: string | ((context: SeriesLabelFormatterContext) => string);
+ series[].label: string | (location: "tooltip" | "axis") => string;

[JCQuintas]

What about replacing the context with just the location. and we will add the context if necessary latter as a second argument:

Sure that can work

[alexfauquette]

As said in our weekly, this looks pretty nice. I'm still wondering if we could improve the DX on the pie chart

About the docs it looks good. Maybe adding links from the components overviw pages to the one you created, like we do for pickers

The pie chart page will also require a particular update depending on the choice made
https://mui.com/x/react-charts/pie/#labels

[alexfauquette]

Not sure about why we need to do the computation of the label here. This value is just here to let the user format the value

[JCQuintas]

Else the user receives a function as the label, which they will then have to check if it is a function before calling, and has to call with the correct params. We just streamline the process, now they can always expect a string there.

[alexfauquette]

Feels weird to have two levels of resolving function.

A solution could be to keep label as a string, and have a labelFormatter: (item, context) => string

The demo would then be labelFormatter: (item, context) => `${context.location}+${item.label}` assuming items labels are A, B, and C.
In that case, we should deprecate the arcLabel and remove it in the next major version to keep only on way for formatting arc labels.

The other option is to create tooltipLabel and legendLabel properties. It's less elegant, but do the job

[JCQuintas]

Do you mean we should have a labelFormatter for the other series types as well?

I think the issue with arc is that its inner text can actually represent values, not only the labels.

[JCQuintas]

On a related note, it would probably be very useful for us to unify the pie-charts data structure in relation to the other data structures. Though I'm not sure it is possible...

[github-actions]

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

[LukasTy]

Looks like a really nice improvement for label formatting. 👍
The API seems elegant and powerful. 💯

[alexfauquette]

Looks nice. Only few proposals to make the docs page more straight forward

[alexfauquette]

Looks good 👍