praekeltfoundation / diamondash

Dashboard for how we use Graphite
BSD 3-Clause "New" or "Revised" License
6 stars 0 forks source link

Add pie chart widget #127

Closed justinvdm closed 10 years ago

justinvdm commented 10 years ago

Ready for review. This is just the basics for the pie chart, planning on adding a legend in a separate PR, since that might involve refactoring out the graph widget's legend view.

justinvdm commented 10 years ago

Oh, this PR depends on #130.

justinvdm commented 10 years ago

Ready for review.

hodgestar commented 10 years ago

Other than the question about the scale settings, looks good.

justinvdm commented 10 years ago

Ready for re-review.

I ended up removing margins from chart dimension calculations, these just complicate things, and would be simpler to just do these with css instead of adding more things to think about for dimension calculations.

The issue with using margins for scaling the pie chart inside the svg and keeping the width and height equal is that we will end up with large spaces for height. If we use css padding and margins for this, we can avoid that alltogether, since what we actually need to do is limit the amount of space the svg can take up.

At the moment, the svg for pie charts takes up the entire widget, but this will change once we have legends in the pie chart widget and have a better idea how to allocate space for the svg.

Sorry for the essay, hope this makes sense.

hodgestar commented 10 years ago

:+1: