add legendLabelAccessor to piechart #916
Conversation
|
Hi @agrass, this will be a useful contribution, and the implementation looks straightforward. A few questions:
In the bigger picture, I also wonder if the functionality belongs higher up, like in baseMixin. It seems like this must be an issue for other kinds of charts. I'll comment on the issue. Thanks! |
|
Hi @gordonwoodhull, thank you for your fast reply! Yes the method Thanks |
|
@gordonwoodhull I added some test and documentation. I checked on piechart specs, and there are already a test who checks if the text of the legend is the same text of the key group, so I added another test to check if the text change when I use the |
|
This would be great to have! What's the status of getting it in? |
|
LGTM; I think all we need to do is also use the accessor in the other "legendable" charts. |
|
+1 |
|
@gordonwoodhull does it make sense for another "legendables" charts? Because when you set the legend label for a bar or line chart, the name apply for the whole group, so is a little different compared with piechart that each key of the group has a different label. You can set the legend label without an acessor: .group(frequencies, "Apples").valueAccessor(function (d) {
return d.value.apples;
})
.stack(frequencies, "Kiwis", function (d) {
return d.value.kiwis;
}) |
I added a method legendLabelAccessor to change the text on legends in piecharts. It's the same of issue #785