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

Pie charts that are 100% a single value have a line that shouldn't be there #4385

Open
1 task done
joshgalvan opened this issue Apr 3, 2024 · 5 comments
Open
1 task done
Labels
breaking change Use this label to indicate a breaking change in the Recharts API bug General bug label enhancement Enhancement to a current API

Comments

@joshgalvan
Copy link

  • I have searched the issues of this repository and believe that this is not a duplicate.

What problem does this feature solve?

Pie charts that are filled with only a single value will be whole circles and not have an unnecessary split in them. A whole pie chart should not have any split in it.

What does the proposed API look like?

I don't believe there should be any new API exposed to the user; by default pie charts that are 100% a single value should show a single solid circle. If this feature was to be optional there could be an attribute passed to <Pie /> like splitOnSingleValue or wholeCircleOnSingleValue, etc.

@joshgalvan
Copy link
Author

This is doable with blendStroke on the <Pie /> component, however you have to conditionally make this true on if your data set has a single value in it or not, and removing/adding blendStroke removes/adds a border around the pie chart, making the pie chart not be consistent between different selections.

I've also found that blendStroke causes the last slice to be slightly offset from the first slice, but using stroke="none" fixes this specific problem and removes the need for blendStroke at all, which doesn't exist in the documentation for pie anyways.

You must choose to always eliminate spaces between different slices if your chart potentially has a single value, which seems like an unnecessary limitation.

@joshgalvan joshgalvan changed the title Pie charts that are 100% a single value have a line that is not possible to get rid of Pie charts that are 100% a single value have a line that shouldn't be there Apr 3, 2024
@ckifer
Copy link
Member

ckifer commented Apr 3, 2024

This is what you mean yeah?
Edit two-simple-pie-chart (forked)

Makes sense to me, the default should probably be no line there.

removing/adding blendStroke removes/adds a border around the pie chart

It does? Can you give an example?

blendStroke causes the last slice to be slightly offset from the first slice

Same, can you give an example?

which doesn't exist in the documentation for pie anyways

blendStroke exists in the storybook https://master--63da8268a0da9970db6992aa.chromatic.com/?path=/docs/api-polar-pie--docs

Right now we're at an in-between where the storybook has more accurate documentation than the website due to the website being hard to maintain. This does need added there and feel free to make a PR here https://github.com/recharts/recharts.org if you'd like to see it there.

You must choose to always eliminate spaces between different slices if your chart potentially has a single value, which seems like an unnecessary limitation.

For sure! This is a legacy thing, I have no idea why it is like that and I agree with you. We can just default stroke to none when there are no slices, but this is a breaking change and we'll have to do it in 3.x which we're working on

@joshgalvan
Copy link
Author

This is what you mean yeah?

Yes.

Can you give an example?

blendStroke={true}

blendStroke={true}
blendStroke={false}

blendStroke={false}

You can observe the added white border around the pie chart, however there is probably a way to make this stroke color the same color as my background, making this a non-issue, but perhaps it should default transparent?

You can also see the slight offset of the pink slice in the first picture at the center of the pie chart.

stroke="none"

image

Here you can see the pink slice has no offset and and its most central point is flush with the rest of the other slice's most central point.

@ckifer
Copy link
Member

ckifer commented Apr 3, 2024

Got it.

I can see the case for making the stroke transparent or none by default for sure. I'm not really sure what the logic of blendStroke does to cause that offset, but that should be fixed as well if it should be considered a prop. I'll take a look at it and decide if blendStroke makes sense to keep or not. If all it does is remove the stroke then removing the stroke is much easier and less complicated.

Changing the stroke default can come in 3.x. Fixing blendStroke, if applicable, could come as a bug fix in the next patch. @joshgalvan I probably won't be able to get to it but if you'd like to contribute feel free to take a look and I can point you in the right direction

@ckifer ckifer added bug General bug label enhancement Enhancement to a current API breaking change Use this label to indicate a breaking change in the Recharts API labels Apr 3, 2024
@ckifer
Copy link
Member

ckifer commented Apr 3, 2024

All blendStroke does is match the stroke to the current fill 😅

https://github.com/recharts/recharts/blob/3.x/src/polar/Pie.tsx#L517. This could easily be replaced with stroke: 'none' I think and solve the problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Use this label to indicate a breaking change in the Recharts API bug General bug label enhancement Enhancement to a current API
Projects
None yet
Development

No branches or pull requests

2 participants