New pie chart layout in 0.38 causes overlap and text truncation

[bryanck]

Screenshot

Here's a pie chart in 0.37.2:

Here is the same chart in 0.38:

Description

The new pie chart layout causes the legend to overlap with the chart, labels to get cut off, especially when mousing over and the font enlarges, labels also overlap with the legend. The old chart has a much cleaner layout that is much easier to read.

[issue-label-bot]

Issue-Label Bot is automatically applying the label #bug to this issue, with a confidence of 0.86. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

[junlincc]

A recent PR should have fixed it. but I just tested on latest master and still seeing this overlap issue.
@villebro I am gonna assign this to Kasia, so her team can slowly take over fixing the cosmetic issues on charts. @kkucharc please reach out to Ville if you have any questions. 🙏 thank you both!

[mistercrunch]

This seems like an echarts issue. Personally I'm not big on using the interactive(toggle) legend on a pie chart, why toggle things here? I feel like we should disable the legend by default, labels + tooltips are a lot more useful.

[mistercrunch]

Unless there's an easy option in echarts as something like dontOverlapLegend: true, I wouldn't spend time on this.

[junlincc]

https://echarts.apache.org/examples/en/editor.html?c=pie-legend

in one of Echarts Pie example, they have legend listed on the right side, user can click through the pages if there are too many slices. I think that's a decent solution.

[mistercrunch]

Oh nice! This looks but better. I still would recommend to not show the legend by default. It breaks down quickly as the color palette cycles. In the example above, there's no good way to map the dark red from the legend to the right slice in the pie. You have to map them by either toggling or looking up the label, which defeats the purpose.

[villebro]

The legend overlapping problem is a known issue in the current ECharts version, so the scrollable legend is probably a good option for now. Perhaps even introduce an option for legend orientation. However, I agree that once legend overlapping becomes a real problem, the real reason is probably too many

Many of the stylistic problems in the picture here are actually fixed in the version that is currently on master branch. @bryanck are you able to try out with master to see what your charts look like with the latest version?

[bryanck]

I wasn't able to get the latest to work with the 0.38 backend, I was getting Request is incorrect: {'queries': {0: {'annotation_layers': ['Unknown field.'], 'url_params': ['Unknown field.'], 'applied_time_extras': ['Unknown field.']}}} errors. So it seem there was an API change I'd also need to update.

I did test master, and it looks like it resolves the issues I'm seeing with the label zoom/truncation/overlap and legend overlap. Though I'll miss the slight zoom animation of the slice with mouse over that was removed and which the old one had (apache-superset/superset-ui#816).

[bryanck]

IMO the 0.15.11 echart update and related API change should be merged to 0.38

[villebro]

I wasn't able to get the latest to work with the 0.38 backend, I was getting Request is incorrect: {'queries': {0: {'annotation_layers': ['Unknown field.'], 'url_params': ['Unknown field.'], 'applied_time_extras': ['Unknown field.']}}} errors. So it seem there was an API change I'd also need to update.

@bryanck the reason you're getting that error is you also need to pull in #11393 . There are some unrelated changes to superset-ui/core that were pulled in due to how plugin dependencies are setup. That PR also bumps the plugin dependencies, so picking that cherry should fix your problem.

I did test master, and it looks like it resolves the issues I'm seeing with the label zoom/truncation/overlap and legend overlap. Though I'll miss the slight zoom animation of the slice with mouse over that was removed and which the old one had (apache-superset/superset-ui#816).

Ok, the animation was removed as it was deemed unnecessary. We can probably add it back in as an optional flag if there is demand for it.

IMO the 0.15.11 echart update and related API change should be merged to 0.38

Let me check how easily we can pick that cherry into 0.38.0. If it doesn't mess up stuff too much we can probably get it in. This is my bad, I thought the Pie fixes were included in the RC, but apparently they were not.

[junlincc]

closing this issue @villebro will include the fix in next release.