feat[pie/donut]: custom tooltips for aggregate#305
feat[pie/donut]: custom tooltips for aggregate#305riccardogioratoatoms wants to merge 2 commits intofrappe:masterfrom
Conversation
feat[pie/donut]: custom tooltips for aggregate
|
Any news on this @scmmishra, would you like me to edit something? |
|
Hey @riccardogioratoatoms so sorry for the delay. I've been very busy lately with a few stuffs, I'll definitely review this tomorrow. On the surface it looks good to go, there won't be any changes required it seems. I'll update the documentation for it as well. |
scmmishra
left a comment
scmmishra
left a comment
There was a problem hiding this comment.
We could extend the same to percentage chart as well
| let percent = (this.state.sliceTotals[i] * 100 / this.state.grandTotal).toFixed(1); | ||
| this.tip.setValues(x, y, {name: title, value: percent + "%"}); | ||
| let valueTooltip = percent + "%"; | ||
| if(this.config.formatTooltip){ |
There was a problem hiding this comment.
Any reason to not reuse the formatTooltipX and formatTooltipY?
There was a problem hiding this comment.
I didn't want to confuse this single type of values of aggregate charts with the charts legend X and Y cause in aggregate the concept of X and Y is not present.
There was a problem hiding this comment.
formatTooltip itself is very generic, can we name it better? formatAggrTooltip is something that I can think of now, do you have any suggestions?
There was a problem hiding this comment.
Beyond this, the feature works like a breeze, we're good to merge it.
scmmishra
left a comment
scmmishra
left a comment
There was a problem hiding this comment.
I notice, this is not available for percentage chart, let's extend it to that too.
2 participants
Explanation About What Code Achieves:
This code fixes the issue I had with tooltips with pie charts and donuts charts, like I mentioned it here: #304
Considering the fact that these kind of graph have only 1 value I choose the naming of the function as tooltipOptions.formatTooltip without X or Y like it's possible to do with other kind of graphs.
Screenshots/GIFs:
TODOs:
Solves: #136