pie chart labels #12174

ppisljar · 2017-06-05T16:50:53Z

Release Notes: Users can now add labels to pie charts.

Resolves #1702
partly addresses: #3686

summary: pie charts can now have labels on them to increase readability

ppisljar · 2017-06-09T07:32:21Z

jenkins, test this

thomasneirynck · 2017-06-13T04:06:40Z

src/ui/public/vislib/visualizations/pie_chart.js

+            };
+
+            const conflicts = [];
+            let conflictIterations = 0;


this algo is a little dense for me. might help to go over with me in person.

thomasneirynck

Below some initial comments already:

this is genuinely a nice addition. It makes the pie charts a lot more usable.

I particularly like how readable it is, and I love the use of the "elbowed" connector lines. The left-right alignment makes the labels very readable. Pretty great approach IMO!

Most of the below is mainly minor comments. Biggest suggestion is to simplify the conflict detection. FWIW, I'd prefer fewer labels on screen with a more clean consistent layout of the connector-lines, than more labels but with a more complicated connector-lines. Perhaps others have different opinion.

positioning for a single label doesn't work right. It overlaps with the connector.

the double-rind pie chart is difficult visually to begin with. But for consitency, I would diplay labels on both sides, not just the right side.

can we make the labels and connecting lines the color of the pie-slice?
conflict detection:
- the conflict detection discards entire swats of labels. e.g. notice how labels on the entire left side are dropped.
- Is it possible to have just a greedy algorithm that places each label in order. If there is room, it puts the label, if it does not, it does not place it. The current implementation seems to do more complex things.
- Right now, by flipping the orientation the elbow of the connecting-lines, we get overlaps that hurt legibility. IMHO, this complexity isn't necessary. I think part of the cleanliness of the connectors is that because of the elbows, lines going out from all four quadrants never overlap. e.g.: overlapping connector-lines make it look more of a spaghetti
the truncation parameter is a little bit voodoo. I think mainly because we are using pixels, which isn't all that intuitive.
- can we use char-count iso pixels?
- can we truncate the actual label, instead of replacing it with ... So e.g. foobar would become fo... and not ...
- perhaps we should get rid of the truncation option anyway (?)

ppisljar · 2017-06-14T11:03:39Z

thanks for looking into this @thomas ... answers to some of your comments

fixed the bug with single label
missing labels on the right side were not related to conflict resolution, but were simply pushed out too far to the left (and then hidden as they would go out of the vis bounds) ... i updated this to show labels closer to the pie itself, however you can still get into that scenario (on dashboard for example you should not make your container square .... but 'widescreen' so it can fit the labels as well.
colors of lines and texts: we loose legibility quite fast (think of light yellow slices)
truncation .... i would leave it as it is in this PR for consistency reasons (its like this everywhere else we have it). but i agree we should update this (everywhere).

ppisljar · 2017-06-14T11:17:28Z

flipping of elbows: we don't flip them ... thats the thing ... elbow is always drawn thru 3 points:

centroid of the slice
middle point where x = centroid of outer arc and y = label y
label position

doesn't work well in all scenarios ... but not sure its worth complicating, it will mostly be used for simple charts probably.

thomasneirynck · 2017-06-14T14:02:38Z

I didn't explain well enough about the "elbows". I mean, the elbow flips when you move labels that connect one quadrant of the pie to another quadrant. that's how you get that overlap.

e.g.: when there's a pie slice in the bottom-left quadrant, but position the label in the top-left quadrant, the "elbow" flips, and the connector-line crosses the chart.

agreed on preserving the black color, doing otherwise would make it worse

ppisljar · 2017-06-15T08:49:22Z

@thomas i simplified conflict resolution a bit ... now elbows wont 'break' ... simply if label cant be positioned without conflict its skipped ....

this will render less labels, and won't prioritize labels with bigger slice size over the labels with smaller slices ... let me know what you think

thomasneirynck

the below looks really really slick IMO

so lgtm from me, just on look&feel alone.

I would add a textual description of how the algorithm is supposed to work (and does work). That is probably one of the densest, low level pieces of code in Visualize, so it should be documented.

I am running into some issues where the algorithm can be slow and cause the visualization to "hang up" for a minute or so. Granted, these were for some more "academic" layouts (ie. 250 slices), that may not arise in practice.

I haven't tested this on Reporting yet.

manninge · 2017-10-16T18:34:10Z

So I see this has been solved, but I do not see the 'how to'. I am new to Kibana, so a little green. Picking up a project dumped by someone else and I need to add labels to the pie slices. Thanks.

thomasneirynck · 2017-11-13T22:30:46Z

Created #14926 to solve merge issues.

timroes · 2017-11-14T17:11:44Z

Close this in favor of #14926

ppisljar · 2017-11-16T09:19:44Z

@timroes all your comments were addressed (from #14926)
@alexfrancoeur almoust all your comments were addressed ... for now i didn't add another checkbox to actually show the value in labels (instead of percentage) ... i guess we can do that as a follow up, when somebody opens an issue for it.

timroes

LGTM once the test passes

ppisljar · 2017-11-16T10:45:44Z

CI failed on fail: "management import objects should allow for cancelling overrides" which i think is unrelated ... restarting

jenkins, test this

ppisljar · 2017-11-16T12:10:54Z

CI failed on fail: "visualize app tile map visualize app tile map chart should save with zoom level and load, take screenshot" which is also unrelated ....

you just have to love our CI :)

jenkins, test this

thomasneirynck · 2017-11-17T00:49:43Z

when merging, could you also add a small summary paragraph to the description? it'll add it to the release-notes, thx!

* pie labels * add simple unit test * fixing dashboard test * fixing basedo on review * simplifying conflict resolution * removing unused code * cleanup code * minor changes based on review * updating option templates to match new design * updating truncate_labels to work with chars instead pixels

ppisljar · 2017-11-17T11:13:39Z

backport 6.x: 0b5052a

alexfrancoeur · 2017-11-17T18:17:06Z

yes @ppisljar!!! 🍻

gui-f · 2018-01-17T18:22:09Z

Great news!

However, I believed that on this version the # of counts of documents within legends of certain chart types (like bar and pie) would be available again.

Is there any way to include these informations on charts with terms aggregation?

ppisljar · 2018-01-18T15:54:13Z

you are right, currently you can only show the percentage value, but not the actual document count.
could you open a separate issue for that ? thanks!

cawoodm · 2020-05-29T06:02:15Z

I'm still seeing no values on my Pie chart if I disable Labels (which I don't want) but enable Values (which I do want):

ppisljar added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) v6.0.0 v6.0.0-beta1 labels Jun 5, 2017

ppisljar requested a review from thomasneirynck June 5, 2017 16:50

ppisljar force-pushed the enh/pieLabels branch from 6d23d0a to eaf5fb9 Compare June 6, 2017 07:38

ppisljar changed the title ~~[WIP] pie chart labels~~ pie chart labels Jun 6, 2017

ppisljar requested a review from marius-dr June 6, 2017 07:40

ppisljar force-pushed the enh/pieLabels branch from 509e805 to 7720cdf Compare June 6, 2017 10:22

thomasneirynck reviewed Jun 13, 2017

View reviewed changes

thomasneirynck self-requested a review June 26, 2017 13:42

ppisljar force-pushed the enh/pieLabels branch from 2f9d6cf to 91b11e2 Compare June 26, 2017 14:07

thomasneirynck reviewed Jun 27, 2017

View reviewed changes

thomasneirynck added roadmap v6.1.0 and removed v6.0.0 v6.0.0-beta1 labels Jun 28, 2017

thomasneirynck self-requested a review July 17, 2017 12:55

ppisljar force-pushed the enh/pieLabels branch from 8aa8cfb to 5267d26 Compare September 21, 2017 11:00

ppisljar requested a review from timroes November 9, 2017 13:14

thomasneirynck mentioned this pull request Nov 13, 2017

pie chart labels #14926

Closed

timroes closed this Nov 14, 2017

ppisljar added 3 commits November 16, 2017 09:49

simplifying conflict resolution

62ae083

removing unused code

8825b16

cleanup code

625add6

ppisljar force-pushed the enh/pieLabels branch from 5267d26 to 625add6 Compare November 16, 2017 08:50

ppisljar added 3 commits November 16, 2017 10:10

minor changes based on review

8020fa5

updating option templates to match new design

f250746

updating truncate_labels to work with chars instead pixels

908d85c

timroes approved these changes Nov 16, 2017

View reviewed changes

thomasneirynck approved these changes Nov 17, 2017

View reviewed changes

thomasneirynck added the v7.0.0 label Nov 17, 2017

ppisljar merged commit 5793410 into elastic:master Nov 17, 2017

stacey-gammon mentioned this pull request Nov 17, 2017

[dashboard/functionalTests] stabilize initialization ordering #15031

Merged

ppisljar added the release_note:enhancement label Dec 7, 2017

This was referenced Dec 7, 2017

Optional data labels on charts or legends #4599

Closed

Series labels #3686

Closed

fixes pie label conflict resolution #15410

Merged

alexfrancoeur mentioned this pull request Mar 7, 2018

[Maps] Optional Data Labels in a Shape #17022

Closed

aphelionz mentioned this pull request May 7, 2018

Accessibility: Kibana 6.3 Meta Issue #18866

Closed

48 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pie chart labels #12174

pie chart labels #12174

ppisljar commented Jun 5, 2017 •

edited by thomasneirynck

Loading

ppisljar commented Jun 9, 2017

thomasneirynck Jun 13, 2017

thomasneirynck left a comment •

edited

Loading

ppisljar commented Jun 14, 2017

ppisljar commented Jun 14, 2017

thomasneirynck commented Jun 14, 2017

ppisljar commented Jun 15, 2017

thomasneirynck left a comment

manninge commented Oct 16, 2017

thomasneirynck commented Nov 13, 2017

timroes commented Nov 14, 2017

ppisljar commented Nov 16, 2017

timroes left a comment

ppisljar commented Nov 16, 2017

ppisljar commented Nov 16, 2017

thomasneirynck commented Nov 17, 2017 •

edited

Loading

ppisljar commented Nov 17, 2017

alexfrancoeur commented Nov 17, 2017

gui-f commented Jan 17, 2018

ppisljar commented Jan 18, 2018

cawoodm commented May 29, 2020

pie chart labels #12174

pie chart labels #12174

Conversation

ppisljar commented Jun 5, 2017 • edited by thomasneirynck Loading

ppisljar commented Jun 9, 2017

thomasneirynck Jun 13, 2017

Choose a reason for hiding this comment

thomasneirynck left a comment • edited Loading

Choose a reason for hiding this comment

ppisljar commented Jun 14, 2017

ppisljar commented Jun 14, 2017

thomasneirynck commented Jun 14, 2017

ppisljar commented Jun 15, 2017

thomasneirynck left a comment

Choose a reason for hiding this comment

manninge commented Oct 16, 2017

thomasneirynck commented Nov 13, 2017

timroes commented Nov 14, 2017

ppisljar commented Nov 16, 2017

timroes left a comment

Choose a reason for hiding this comment

ppisljar commented Nov 16, 2017

ppisljar commented Nov 16, 2017

thomasneirynck commented Nov 17, 2017 • edited Loading

ppisljar commented Nov 17, 2017

alexfrancoeur commented Nov 17, 2017

gui-f commented Jan 17, 2018

ppisljar commented Jan 18, 2018

cawoodm commented May 29, 2020

ppisljar commented Jun 5, 2017 •

edited by thomasneirynck

Loading

thomasneirynck left a comment •

edited

Loading

thomasneirynck commented Nov 17, 2017 •

edited

Loading