mattosaurus / ChartJSCore

Implementation of Chart.js for use with .NET Core.
GNU General Public License v3.0
116 stars 34 forks source link

Handling of data set type (implictly) #93

Closed stephanstapel closed 1 year ago

stephanstapel commented 1 year ago

Hi, I just found your project and really like it. Congratulations and thanks a lot! Just a tiny proposal for an enhancement: I took your demo code, modified the chart type to bar and found that the digram didn't change at all because the dataset was still implicitly set to 'line'. Would it make sense to derive the dataset type from the chart type if not explictly stated otherwise?

mattosaurus commented 1 year ago

Good suggestion.

Feel free to submit a PR adding this functionality, otherwise I'll take a look at doing it myself when I get a chance.

stephanstapel commented 1 year ago

Hi again, I was of course thinking about doing so. After learning more about your library (still like it :) ), I finally :) found that you specifically derived the Dataset class, e.g. LineDataset. I understand that this is necessary due to different option sets.

If you have an idea and would like to discuss, I'd be more than happy to issue a PR. How do you think about the possibility?

mattosaurus commented 1 year ago

Hmm, looking at the code again I'd have thought that just updating the Type property on the dataset would be enough to get it to change type, though any properties not in the new type would be ignored.

If you want to cause the chart to rerender after you've changed the type then you're going to need to call update on the chart object as well. I did something similar previously here.

https://github.com/mattosaurus/bitScry/blob/87259f0fdf3f6da24798dcbe7950983f86ba75d9/bitScry/Views/Projects/Chart/Index.cshtml#L132-L143

In this case, if you're changing the type based on a dropdown or similar then you probably won't want to actually generate the chart JSON again but rather just edit what you already have using jQuery or similar.