search

Joelius300 / ChartJSBlazor

This library is a modification of the awesome ChartJs.Blazor library by mariusmuntean. It's supposed to add more functionality to the LineChart and generally make the library more complete.
Other
40 stars 6 forks source link

Rename PieDoughnutAnimation to ArcAnimation #51

Closed SeppPenner closed 5 years ago

SeppPenner commented 5 years ago

The PieDoughnutAnimation is used in:

I would suggest to rename it to PiePolarAnimation because the doughnut chart doesn't exist in our case as we treat it as a pie chart.

SeppPenner commented 5 years ago

This should be done after https://github.com/Joelius300/ChartJSBlazor/pull/50 is checked and merged to avoid issus.

Joelius300 commented 5 years ago

Actually I would suggest RadialAnimation, ArcAnimation (this one's better). Those are a lot more generic and we don't need to associate them with either Pie nor Polar.

SeppPenner commented 5 years ago

I didn't have a good name for that. Yeah, ArcAnimation seems good. (The Chart.JS guys are using this term very often as well).

Joelius300 commented 5 years ago

Will you include this update in https://github.com/Joelius300/ChartJSBlazor/pull/50? I think it fits there even though it requires changes in other files. Just put the renaming and the associated error-fixing in one commit.

SeppPenner commented 5 years ago

Sure, I can add this as well later.

SeppPenner commented 5 years ago

This is already fixed in https://github.com/Joelius300/ChartJSBlazor/pull/50/commits/15c0662ea72e4ab3e69217f19084c416992c1027 in https://github.com/Joelius300/ChartJSBlazor/pull/50.

SeppPenner commented 5 years ago

Just for reference, this is already done: https://github.com/Joelius300/ChartJSBlazor/blob/master/ChartJs.Blazor/ChartJS/Common/Properties/ArcAnimation.cs. (In #50 as I mentioned before).