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

withBarValueLabel not displaying in correct positions in BarChart #6991

Closed
1 of 2 tasks
namakshenas opened this issue Oct 17, 2024 · 12 comments · Fixed by #7042
Closed
1 of 2 tasks

withBarValueLabel not displaying in correct positions in BarChart #6991

namakshenas opened this issue Oct 17, 2024 · 12 comments · Fixed by #7042

Comments

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

What package has an issue?

@mantine/charts

What framework do you use?

Other, I will specify in the bug description

In which browsers you can reproduce the issue?

Chrome

Describe the bug

Hi,

I'm trying to use the withBarValueLabel property of BarChart. The value labels are not placed correctly. Here is the snapshot of the issue:

Screenshot 2024-10-17 at 16 08 52

Here is the working example in @mantine/charts:

<BarChart
          h={300}
          data={[
            { dmu: 'dmu1', 'Efficiency score': 100, Status: 'Efficient', color: '#32CD32' },
            { dmu: 'dmu2', 'Efficiency score': 78, Status: 'InEfficient', color: '#FFA500' },
            { dmu: 'dmu3', 'Efficiency score': 100, Status: 'Efficient', color: '#32CD32' },
            { dmu: 'dmu4', 'Efficiency score': 100, Status: 'Efficient', color: '#32CD32' },
            { dmu: 'dmu5', 'Efficiency score': 89, Status: 'InEfficient', color: '#FFA500' },
            { dmu: 'dmu6', 'Efficiency score': 95, Status: 'InEfficient', color: '#FFA500' },
          ]}
          withBarValueLabel
          orientation="vertical"
          gridAxis="y"
          barProps={{ radius: 5 }}
          dataKey="dmu"
          series={[{ name: 'Efficiency score' }]}
          tickLine="none"
/>

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

No response

Possible fix

No response

Self-service

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

I'd like to work on this. Please assign if not picked up.

@rtivital
Copy link
Member

You are welcome to submit a PR with a fix

@fsd-niraj
Copy link
Contributor

image

Please let me know if this looks good? I'll quickly raise my PR.

@namakshenas
Copy link
Author

image

Please let me know if this looks good? I'll quickly raise my PR.

Thanks. But they should be aligned right, where just after each bar ends in the right hand side.

@fsd-niraj
Copy link
Contributor

fsd-niraj commented Oct 19, 2024

image
image

How about this?

@namakshenas
Copy link
Author

image

image

How about this?

Looks great. But consider another scenario: what if the user chooses the bar width to be thin. In that case, the values should be placed outside of the bar.

@fsd-niraj
Copy link
Contributor

image

image

image

Hey, Let me know if this looks good to you.

@namakshenas
Copy link
Author

image

image

image

Hey, Let me know if this looks good to you.

Well done!
I think you can now suggest your PR.

@fsd-niraj
Copy link
Contributor

PR: #7011

@fsd-niraj
Copy link
Contributor

Hey, I saw that the test case failed, is it still mergeable? I don't see any other problem than linting and prettier.

@rtivital
Copy link
Member

To merge the PR, all tests must pass. To continue working on your PR, add more commits to the same branch and push them. You can run tests locally with npm test command.

@fsd-niraj
Copy link
Contributor

Hey @rtivital , heres my new PR, please check.
#7042

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