mariusmuntean / ChartJs.Blazor

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

Creation of mixed charts should be possible also in version 2.0.0 #133

Closed BorisGaliev closed 3 years ago

BorisGaliev commented 4 years ago

Describe the bug

It should be possible to create mixed charts that are a combination of two or more different chart types. E.g. see a code, which works with ChartJs.Blazor version 1.1.0, below

Which Blazor project type is your bug related to?

Code example

Please provide full code examples below where possible to make it easier for the developers to check your issues.

@using System.Drawing
@using ChartJs.Blazor.ChartJS.BarChart
@using ChartJs.Blazor.ChartJS.BarChart.Axes
@using ChartJs.Blazor.ChartJS.Common.Axes
@using ChartJs.Blazor.ChartJS.Common.Axes.Ticks
@using ChartJs.Blazor.ChartJS.Common.Enums
@using ChartJs.Blazor.ChartJS.Common.Properties
@using ChartJs.Blazor.ChartJS.Common.Wrappers
@using ChartJs.Blazor.ChartJS.LineChart
@using ChartJs.Blazor.Charts
@using ChartJs.Blazor.Util

<h2>Simple Bar Chart</h2>
<div class="row">
    <button class="btn btn-primary" @onclick="AddData">
        Add Data
    </button>
    <button class="btn btn-primary" @onclick="RemoveData">
        Remove Data
    </button>
</div>
<ChartJsBarChart @ref="_barChart"
                 Config="@_barChartConfig"
                 Width="600"
                 Height="300" />

@code
{

    private BarConfig _barChartConfig;
    private ChartJsBarChart _barChart;
    private BarDataset<DoubleWrapper> _barDataSet;

    protected override void OnInitialized()
    {
        _barChartConfig = new BarConfig
        {
            Options = new BarOptions
            {
                Title = new OptionsTitle
                {
                    Display = true,
                    Text = "Simple Bar Chart"
                },
                Scales = new BarScales
                {
                    XAxes = new List<CartesianAxis>
                    {
                        new BarCategoryAxis
                        {
                            BarPercentage = 0.5,
                            BarThickness = BarThickness.Flex
                        }
                    },
                    YAxes = new List<CartesianAxis>
                    {
                        new BarLinearCartesianAxis
                        {
                            Ticks = new LinearCartesianTicks
                            {
                                BeginAtZero = true
                            }
                        }
                    }
                }
            }
        };

        _barChartConfig.Data.Labels.AddRange(new[] { "A", "B", "C", "D" });

        _barDataSet = new BarDataset<DoubleWrapper>
        {
            Label = "My double dataset",
            BackgroundColor = new[] { ColorUtil.RandomColorString(), ColorUtil.RandomColorString(), ColorUtil.RandomColorString(), ColorUtil.RandomColorString() },
            BorderWidth = 0,
            HoverBackgroundColor = ColorUtil.RandomColorString(),
            HoverBorderColor = ColorUtil.RandomColorString(),
            HoverBorderWidth = 1,
            BorderColor = "#ffffff"
        };

        _barDataSet.AddRange(new double[] { 8, 5, 3, 7 }.Wrap());
        _barChartConfig.Data.Datasets.Add(_barDataSet);

        var lineSet = new LineDataset<ChartJs.Blazor.ChartJS.Common.Point>
        {
            Label = "Line",
            Hidden = false,
            BackgroundColor = ColorUtil.FromDrawingColor(Color.DimGray),
            BorderColor = ColorUtil.FromDrawingColor(Color.DimGray),
            BorderCapStyle = BorderCapStyle.Square,
            BorderJoinStyle = BorderJoinStyle.Bevel,
            Fill = false,
            PointBackgroundColor = ColorUtil.FromDrawingColor(Color.Black),
            BorderWidth = 1,
            PointBorderWidth = 1,
            PointRadius = 3,
            SteppedLine = SteppedLine.False
        };

        lineSet.AddRange(new List<ChartJs.Blazor.ChartJS.Common.Point> ()
        {
            new ChartJs.Blazor.ChartJS.Common.Point(0, 7),
            new ChartJs.Blazor.ChartJS.Common.Point(1, 3),
            new ChartJs.Blazor.ChartJS.Common.Point(2, 2),
            new ChartJs.Blazor.ChartJS.Common.Point(3, 5),

        } );

        _barChartConfig.Data.Datasets.Add(lineSet);
    }

    private async Task AddData()
    {
        var nowSecond = DateTime.Now.Second;
        _barChartConfig.Data.Labels.Add(nowSecond.ToString());
        _barDataSet.Add(new DoubleWrapper(nowSecond));

        await _barChart.Update();
    }

    private async Task RemoveData()
    {
        if (_barChartConfig.Data.Labels.Count > 0)
        {
            _barChartConfig.Data.Labels.RemoveAt(_barChartConfig.Data.Labels.Count - 1);
        }

        if (_barDataSet.Data.Count > 0)
        {
            _barDataSet.RemoveAt(_barDataSet.Data.Count - 1);
        }

        await _barChart.Update();
    }
}
Joelius300 commented 4 years ago

There are multiple solutions to this with both pros and cons.

I'm not listing You can mix any dataset into any chart as a pro because that's a requirement if I understand the chart-mixing correctly. I didn't have enough knowledge on Chart.js mixing functionality and focused a bit much on typesafety when implementing the dataset-collections for #96. I still like the system I put in place for its typesafety but obviously it doesn't suit our needs and has to be changed. This only affects the dataset-collections, not the datasets themselfs.

The simple approach

Instead of having the dataset-collections, we could just have a List<IDataset>.

Pros

Cons

The more complex and explicit approach

This approach still uses dataset-collections and they would be implemented similarly but even more restrictive. The dataset-collection would provide methods to add only supported datasets for the same chart-type. So the BarDatasetCollection would allow adding BarDataset<TimePoint>, BarDataset<FloatingBarPoint>, BarDataset<int>, BarDataset<long> and BarDataset<double>. We would again use the trick of Add overloads in order to allow for a typesafe collection initializer.

Then in addition to the "supported" datasets, you could also add any other IDataset via a different method called something like AddMixingDataset.

Pros

Cons

What do you think? Which solution do you think is more enjoyable to use?

My vote is for the simple solution. It's less work, less complexity and the complex solution doesn't seems to provide enough value to be worth the cons and especially the work.


1 When thinking about this point you should consider that the basic pattern is to store a reference to your dataset and modify it with that reference. You shouldn't need to query the datasets of a chart and thus typesafety looses importance.

BorisGaliev commented 4 years ago

Because your library is based on chart js, it might make sense to provide a similar API: there it is implemented on a dataset level, so both your approaches correspond to their philosophy, and if the first one is the easier one, it would be just fine

Joelius300 commented 4 years ago

Agreed. One thing to consider is that the options can be mixed as well as far as I know. That shouldn't be possible for every chart though but we could create a new chart config which allows for more or less dynamic object that can be composed of several configs. Something like their helpers.merge but only for mixable chart configs.

Or we could leave it as is and let the user plug in their own implementation of a chart config which makes it so they can extend any config by deriving from it and then using that type in Chart. Easier for us, just as flexible for the user but requires a bit more code and there's less guidance. If we document that and provide a sample, I think this is the way to go.

BorisGaliev commented 4 years ago

If an example, where it is explained how to derive and use inheritance, becomes available, it will be quite okay, I think