mantinedev / mantine

A fully featured React components library
https://mantine.dev
MIT License
26.02k stars 1.85k forks source link

Graphs with nested values ignore "label" property #6529

Closed JonathanG-DC closed 4 weeks ago

JonathanG-DC commented 1 month ago

Dependencies check up

What version of @mantine/* packages do you have in package.json?

7.11.2

What package has an issue?

@mantine/charts

What framework do you use?

Vite

In which browsers you can reproduce the issue?

All

Describe the bug

Graphs that are using nested values now ignore the "label" property for tooltips and legends and instead use the last part of the "name" property. Image from 7.11.1: image

Image from 7.11.2: image

The included code sandbox shows the issue and can be fixed by reverting the version of all mantine packages to 7.11.1.

I believe this is related to #5885 and its associated PR #5886.

If possible, include a link to a codesandbox with a minimal reproduction

https://codesandbox.io/p/sandbox/mantine-react-template-forked-cxqmf8

Possible fix

The following files had new functions added as part of the mentioned PR: packages/@mantine/charts/src/ChartLegend/ChartLegend.tsx updateChartTooltipPayload packages/@mantine/charts/src/ChartTooltip/ChartTooltip.tsx updateChartLegendPayload

These override the name with the nested property name, I assume these should instead be using label property with the fallback being the nested property name.

Self-service

Sergio16T commented 1 month ago

Beginning work on fix for this issue.

JonathanG-DC commented 3 weeks ago

Happy to raise this as a new issue if need be, but unfortunately this fix (using all 7.12.1 packages) doesn't work if nested values are more than one level deep. See example here: https://codesandbox.io/p/sandbox/mantine-react-template-forked-cxqmf8

image

The AppleLabel is not displayed in the legend and the value is not shown in the tooltip.

Sergio16T commented 3 weeks ago

Hi @JonathanG-DC, Is this a common use-case with Line Chat Graphs? My understanding of original ask was only 1 level deep. If so, additional dev work is needed to support this.

JonathanG-DC commented 3 weeks ago

Hi @Sergio16T all good, I do appreciate your work in the initial issue. My goal with this request was to get feature parity with 7.11.1 which supports multiple levels of nesting on all graphs. The graph itself seems to be fine regardless of version and nesting level, this is just a issue with the Tooltip and Legend.

Sergio16T commented 3 weeks ago

Hi @JonathanG-DC, Thank you for providing more information about the original feature request.

I can create a subsequent PR to address this feature request to attain feature parity with 7.11.1 to enable multi-level nesting of graph data for the Tooltip and Legend.

We can re-open this issue.

FYI @rtivital

rtivital commented 3 weeks ago

If you want to submit a PR for this, I do not mind adding this feature

JonathanG-DC commented 2 weeks ago

@Sergio16T I've found another issue with the fix unfortunately. If you have the same nested prop name the last ones label is used for all of them. This occurs even when sticking within the existing single level of nesting.

image image

https://codesandbox.io/p/sandbox/mantine-react-template-forked-cxqmf8?file=%2Fsrc%2Fdata.ts%3A5%2C12

Sergio16T commented 2 weeks ago

@JonathanG-DC great QA. Appreciate the steps to replicate and details. I haven't had bandwidth to begin work on a fix. I plan to begin work on this in the near future.