Fixed bug handling 1 and 0 #1
Conversation
README.md
Outdated
| @@ -32,4 +32,4 @@ And if you need to, you can update the values: | |||
| }) | |||
|
|
|||
|
|
|||
| Have fun! | |||
| https://raw.githubusercontent.com/ismaell/js-donut-chart | |||
There was a problem hiding this comment.
Unintentional copy / paste error I should think. Totally unintentional
| this.spec = spec; | ||
| this.element = null; | ||
|
|
||
| this.update({}); |
There was a problem hiding this comment.
Please don't change the indentation in the same commit. Also, what's the rationale for changing it?
There was a problem hiding this comment.
I'm not aware that I changed the indentation at all. I did move some methods around in the page order but I don't think I changed the indentation at all....
| @@ -1,7 +1,7 @@ | |||
| <html> | |||
| <head> | |||
| <style> | |||
| .chart-item-bg {stroke: #ddd} | |||
| .chart-item-bg {stroke: #000000} | |||
There was a problem hiding this comment.
I am guessing this change slipped thru.
There was a problem hiding this comment.
This was intentional. Switching the stroke to black made it much clearer to debug at the very very small levels that I was dealing with where it would break trying to cover off the closest acceptable fraction close to 1 / 0. I wanted the highest possible contrast.
donut-chart.js
Outdated
| if (this.element) this.element.remove(); | ||
| this.element = this.parent.appendChild(code); | ||
| } else { | ||
| throw new Error("No parent defined for the chart.") |
There was a problem hiding this comment.
I would prefer the following style:
if (!this.parent)
throw new Error("...")
...
Re-added request for fun which I seem to have removed unintentionally
Hi,
I've added some documentation to this, applied a fix to handle a bug where a single item of a value of 1 or 0 would make the circle disappear and also added some error handling to make it easier to debug when it's not working.
Defaults will also apply if you don't provide radius for example so that the item fills it's container.
Nice donut!