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

fix(pie chart): fix overlapping labels #2599

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

calum-atkinson
Copy link

Fix based on the solution found in the following issue: #1378

Effectively, the solution recalculates the points on the label when segments of the pie are small (π/10 radians chosen as a useful value) so that more labels are able to be displayed without interference with one another.

It's not an exhaustive solution as when the segments become very small (<1% of the pie) there can still be overlapping, but this solution should improve the general case. A more complete solution may need an awareness of the neighbouring segments and the space around the pie.

Copy link

vercel bot commented May 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nivo ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 30, 2024 2:18pm

@brian-codes
Copy link

Also issue #143

@MartinHutyra
Copy link

@calum-atkinson is it working as expected? If so, @plouc can you please do the review?

@plouc
Copy link
Owner

plouc commented Jul 3, 2024

This could be a solution, however I think it should be optional, if you compare this https://nivo-git-fork-raven-controls-fix-pie-cha-15a04b-ploucs-projects.vercel.app/pie/ to the current implementation https://nivo.rocks/pie/, and try a few different datasets, you'll see that it's not always better, and sometimes there are no overlaps to fix.

Also it would be good to add some tests for this.

@brian-codes
Copy link

@plouc do you have an example dataset it's worse with?

@plouc
Copy link
Owner

plouc commented Jul 24, 2024

@brian-codes, I just played with the deployed demo, and generated random dataset, see this screenshot for example:

CleanShot 2024-07-24 at 18 36 00@2x

IMO, it would be better if labels were all the same and personally, I wouldn't want this behavior on by default.

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 this pull request may close these issues.

4 participants