mariusmuntean / ChartJs.Blazor

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

Fix for AspectRatio configuration #88

Closed esso23 closed 4 years ago

esso23 commented 4 years ago

Making it possible to not set width/height on chart canvas. This is required for AspectRatio to work. I removed the default settings for width/height of 400/400 since it makes more sense to not have the canvas width/height set by default.

Joelius300 commented 4 years ago

Thank you for the fix! For future reference, you should first open an issue explaining the problem but you've done a great job at the PR description so you don't need to create one now :)

I will have to do some tests with it before merging but it looks like a straight forward fix.
One thing though. Could you remove the = null; assignments? Since the default of int? is always null, I dislike the explicit default value. This is just a small style thing but if we always do these things the same way, we might be able to some day get rid of all the style inconsistencies within the library.

Otherwise great work, I'm looking forward to incorporate this in the next release!

esso23 commented 4 years ago

Damn, sorry for that. Did that before my morning coffee :X