mariusmuntean / ChartJs.Blazor

Brings Chart.js charts to Blazor
https://www.iheartblazor.com/
MIT License
677 stars 151 forks source link

Removed try-catchs from MomentJsInterop.cs and ChartJsInterop.cs wher… #93

Closed SeppPenner closed 4 years ago

SeppPenner commented 4 years ago

…e the async-call inside the try is not awaited.

Fixes https://github.com/mariusmuntean/ChartJs.Blazor/projects/3#card-28660703.

Joelius300 commented 4 years ago

Thanks for the PR.

This is exactly what I had in mind with that card but it's going to be a breaking change (since it also catches the expando stuff) so I can't merge this just now. Also this class is heavily affected by #70 so I would wait with adjusting things there until that's merged. Changes are high that I'll fix this in #70 anyway. Either way, I can't merge this PR but thank you, we can use it as a reference for later.

SeppPenner commented 4 years ago

Ah, okay. I didn't know that this is affected by your rework there. I was just trying to start with the low hanging easy stuff before trying to break something here :)

Joelius300 commented 4 years ago

70 is quite big, I know 😅 When there's not such a big change in the room, small things like this are greatly appreciated but right now might not be the best time for things like that.

If you want to help right now, I would greatly appreciate some opinions in #90 because I'm not sure how to handle that and once the discussion starts going it's easier to talk about it. Just saying though, if you don't have an opinion, don't force yourself to comment.
Also something that always helps is documenting what's wrong with current implementations. The scatter chart for example has many problems and searching for solutions to that, trying to find connections and common base classes, writing out what properties we need, collecting docs links etc is something that will have to be done at some point but requires some time and motivation. It's really not about code in that case, just writing out whats wrong and possible solutions is a great way to help us! And you don't even need code :)

SeppPenner commented 4 years ago

I will check what I can do with the scatter chart :)