Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graphs with nested values ignore "label" property #6529

Closed
1 of 2 tasks
JonathanG-DC opened this issue Jul 16, 2024 · 8 comments · Fixed by #6536
Closed
1 of 2 tasks

Graphs with nested values ignore "label" property #6529

JonathanG-DC opened this issue Jul 16, 2024 · 8 comments · Fixed by #6536

Comments

@JonathanG-DC
Copy link

Dependencies check up

  • I have verified that I use latest version of all @mantine/* packages

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

  • I would be willing to implement a fix for this issue
@Sergio16T
Copy link
Contributor

Beginning work on fix for this issue.

@JonathanG-DC
Copy link
Author

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
Copy link
Contributor

Sergio16T commented Aug 13, 2024

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Member

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

@JonathanG-DC
Copy link
Author

@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
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants