DmitryBaranovskiy / g.raphael

Charts for Raphaël
http://g.raphaeljs.com/
1.52k stars 435 forks source link

Allow users to preserve the order of values for pie charts #186

Closed stephenbinns closed 8 years ago

stephenbinns commented 11 years ago

Added an option to the piechart named preserveOrder to allow users to preserve the order of the values provided so that the pie section colors are always consistent.

This is a non breaking change as the default value is falsy, whereas https://github.com/DmitryBaranovskiy/g.raphael/pull/176 and #79 will break existing functionality

gshutler commented 11 years ago

:+1:

stephenbinns commented 11 years ago

Use case: http://jsfiddle.net/gCB7D/1/ and with fix http://jsfiddle.net/t9gf5/1/

MaffooBristol commented 11 years ago

:+1: Glad this has been implemented :)

emiliok commented 11 years ago

If you disable sorting this way you will end in collapsing "small" sectors that are not actually "small". Try for example with this data [0, 30, 10].

gshutler commented 11 years ago

What would you expect to happen in that situation @emiliok? It not collapse anything?

emiliok commented 11 years ago

Hi @gshutler,

if you try some data in this fiddle (unordered pie charts) http://jsfiddle.net/t9gf5/1/ (just copy and paste my data and re-run) you will see that every value after the zero will collapse into "Others". Try this: var values = [30, 0, 10, 40, 20]; var legend = ["A", "B", "C", "D", "E"]; and then this: var values = [30, 20, 10, 40, 0]; var legend = ["A", "B", "C", "D", "E"]; and you will see what I mean.

In the above two cases I would expect to see the same number of sectors in the resulting pie charts.

I do agree that unsorted pie charts are useful, but I think that this implementation needs to be improved.

gshutler commented 11 years ago

I'll have a chat with @stephenbinns (we work together). I think preserving the order and disabling the collapsing are interlinked (as you've demonstrated) and so should probably be done through one parameter. But that means preserveOrder isn't the best name for it any more.

Thanks for the feedback.

stephenbinns commented 11 years ago

I've made a change which handles that case @emiliok we're no longer allowing for the cutting of values when preserving the order, this seems sensible to me as you'd reasonably expect to see a label for each color you've specified.

Working example: http://jsfiddle.net/t9gf5/5/

emiliok commented 11 years ago

:+1: