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

Barchart rework #83

Closed SeppPenner closed 5 years ago

SeppPenner commented 5 years ago

I'm not yet sure how this mixable dataset works and thus had some problems there. (The chart example doesn't work either).

Can you check this and give me some hints, please?

SeppPenner commented 5 years ago

Ok, wait. Why are the already merged commits in there?

Joelius300 commented 5 years ago

added 17 commits 11 days ago huh? I don't think we can use this :/

You should reset https://github.com/Joelius300/ChartJSBlazor/pull/83/commits/45124bed0855d85d5e250de41bacb092e213c447 using git reset 45124bed (should work, I'm no expert sadly). Then you can stash that using git stash. Now hard-reset on my master. After that you should be able to apply the stash again using git stash apply. If everything goes well you should now have all the changes of the rework-commit as unstaged changes which you can then stage and commit. You should only be one commit further than my master after doing so.

In general the rework looks good though (I didn't look into it in detail yet). A few things though:

Also what doesn't work with the sample? Are there errors or does it just not work?

SeppPenner commented 5 years ago

I cloned your project into another location and manually moved the changed files towards that folder because your suggestions didn't work...

You don't really need to add summaries for the private constructors in your object enums, they have almost no value.

I know. I just don't like it if there is no commentary at all.

Also what doesn't work with the sample? Are there errors or does it just not work?

The configuration is somehow strange (Because of the MixableDataset stuff). --> I didn't get it to work properly with the labels and data items.

We should discuss this in the new pull request: https://github.com/Joelius300/ChartJSBlazor/pull/86

SeppPenner commented 5 years ago

This is done in https://github.com/Joelius300/ChartJSBlazor/pull/86 now because of the git noise...