Adjust pie chart offset to avoid overlapping labels

[pnachum]

To reduce the chance of overlapping labels, the pie chart's startOffset defaults to Math.PI / 4 so that the smaller labels are likely to appear towards the right side of the chart, where there is more space.

Also, redraw the chart when startOffset changes. This makes it easy to specify and change the rotation of the chart.

Before:

After:

@Addepar/fire

[xudaniel11work]

I prefer the original Pie Chart version for regular charts. Would it be possible to detect when an "overlap" occurs and then rotate the pie chart with the new offset? This would prevent all of our current Pie Charts from breaking and rotating, except the ones that are "broken" already.

I have some ideas if you want to experiment!

[xudaniel11work]

Checked in with Karpas. He said that UI 1 actually used to have Y-Axis aligned pie charts until they had the same problem as we do now. They then switched it so that the chart would no longer be Y-Axis aligned, so my previous concern is moot.

That being said, I want to thoroughly test this one out just in case we might miss something. One concern is where there could be a concentration of labels in the bottom (slight right/left) of the the chart.

Try testing out data where {A = 50%, B = 5%, C = 5%, D = 4%, E = 4%, F = 4%, Other = remaining percent}. Will that setup potentially break the chart?

[akhileshgupta]

Some tips on how to create pie chart visualizations: http://visage.co/data-visualization-101-pie-charts/ Looks like the older design was following the Tip #4 for ordering pies.

[pnachum]

@xudaniel11work Yeah, we'll definitely BTR this. It's unlikely that this won't ever break for any set of data without adding a bunch of gross logic to detect overlaps.

Let me know if you can find test cases that break this badly.

[pnachum]

@Addepar/fire Any other thoughts on this?

[billy-addepar]

This LGTM if CS agree on this solution.

[thangdinh]

This one still has problems if the biggest pie is about 50% of the total, right?

[pnachum]

Why?

[thangdinh]

Because all of the small pies will still be at the bottom in that case

[pnachum]

The small slices should always be in the same place

[thangdinh]

I think my example is not correct. If the largest pie is 58%-62.5%, the small pies should be at the bottom, right?

[pnachum]

No, the small slices should always be to the right of the largest slice, and the largest slice always begins at a fixed position.

[xudaniel11work]

This is a step in the right direction. Although rotating the pie does help with the labels, it doesn't solve it perfectly 100% of the time. At some point, we will have to change the offset amount from the pie itself, it can't always be a default distance

I think this is also what Thang was alluding to.

Or

[pnachum]

At some point, we will have to change the offset amount from the pie itself, it can't always be a default distance

I don't understand.

I think this is also what Thang was alluding to. This problem will occur again if we don't solve properly for it

That problem occurs because the "Other" slice is drawn after the small slices. I feel like that's incorrect behavior, because it means that the slices aren't in order of largest to smallest. The size of the "Other" slice should be computed, and then drawn in the correct order. Then the small slices will always be rendered in the same area of the chart.

[xudaniel11work]

I think that even if we rendered the Other piece before the smaller pieces, there will still be situations where the labels could intersect. We can experiment tomorrow!

Correction: It actually handles it quite well since we limit the number of slices in a pie chart to 7. However, if we want to increase the number of slices sometime in the future (which we probably will), we'll probably run into problems with intersections again

With the entire Pie being sorted "properly" ("Other" slice coming after Label 1)

If we have more than 7 slices and have it properly sorted, still intersections

[xudaniel11work]

Another potential issue :( , labels that are close to 0% to 1% will tend to intersect with one another. This was one of the core problems identified here in ADPR-12487 -

https://addepar.atlassian.net/browse/ADPR-12487?jql=text%20~%20%22pie%20chart%20intersection%22

[pnachum]

I think that even if we rendered the Other piece before the smaller pieces, there will still be situations where the labels could intersect.

For sure. It's unlikely that there's any reasonable solution that's completely perfect. But we can make it a lot better, especially given reasonable values of minSlicePercent and maxNumberOfSlices.

if we want to increase the number of slices sometime in the future (which we probably will), we'll probably run into problems with intersections again

I'd imagine that we're more likely to let users control it themselves than to change it for all users without adding a control.

In the example where there are a ton of small slices (ie let's say the entire chart is made up of a ton of small slices), I don't really know where we can place them such that there isn't an intersection.

In the example where there's a 0% slice, it should probably be handled minSlicePercent, or we just shouldn't render slices which get rounded to 0 or something.

[pnachum]

Closing for now, since the related bug was downgraded.