Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Visual glitches when updating charts #13

Closed
miroslavp opened this issue Aug 16, 2018 · 3 comments
Closed

Visual glitches when updating charts #13

miroslavp opened this issue Aug 16, 2018 · 3 comments

Comments

@miroslavp
Copy link

miroslavp commented Aug 16, 2018

I noticed visual glitches on the chart when using the update button from the example. It turns out that the cause for that is because you haven't destroyed the old chart object before setting with new one.

In BlazorComponents.js, for every update function you should add
if (myChartIndex !== -1) { myChart.chart.destroy(); }
right before
myChart.chart = {};

Another thing I noticed is that in your Initialize functions you create a new chart object even if you don't add it to the collection. So I think you can improve it by changing the code from this:

    let thisChart = initializeChartjsChart(data, 'bar');

    if (!BlazorCharts.find(currentChart => currentChart.id === data.canvasId))
        BlazorCharts.push({ id: data.canvasId, chart: thisChart });

to this

    if (!BlazorCharts.find(currentChart => currentChart.id === data.canvasId)) {
        let thisChart = initializeChartjsChart(data, 'bar');
        BlazorCharts.push({ id: data.canvasId, chart: thisChart });
    }
@miroslavp
Copy link
Author

miroslavp commented Aug 16, 2018

ChartJsBarChart.cshtml
ChartJsLineChart.cshtml
ChartJsRadarChart.cshtml
should implement IDisposable and destroy the chart on Dispose

@implements IDisposable

public void Dispose()
{
    ChartJSInterop.DestroyChart(Chart.CanvasId);
}

Here's the method in the interop class

    public static Task<bool> DestroyChart(string canvasId)
    {
        return JSRuntime.Current.InvokeAsync<bool>("BlazorComponents.ChartJSInterop.DestroyChart", new[] { canvasId });
    }

Here's the js function

DestroyChart: function (canvasId) {
    if (!BlazorCharts.find(currentChart => currentChart.id === canvasId))
        throw `Could not find a chart with the given id. ${canvasId}`;

    let myChart = BlazorCharts.find(currentChart => currentChart.id === canvasId);

    let myChartIndex = BlazorCharts.findIndex(currentChart => currentChart.id === canvasId);

    if (myChartIndex !== -1) {
        myChart.chart.destroy();
        BlazorCharts.splice(myChartIndex, 1);
    }

    return true;
}

As you can see, we remove the chart object from the global array with this line when the component is destroyed
BlazorCharts.splice(myChartIndex, 1);

@miroslavp
Copy link
Author

Also update methods of
ChartJsBarChart.cshtml
ChartJsLineChart.cshtml
ChartJsRadarChart.cshtml
should be parameterless. Right now they look like this:

public void UpdateChart(ChartJSChart<ChartJsBarDataset> updatedChart)
{
    ChartJSInterop.UpdateBarChart(updatedChart);
}

However they should pass the local property instead like this:

public void UpdateChart()
{
    ChartJSInterop.UpdateBarChart(Chart);
}

The user should always modify the local property instead of passing new objects. This is important because otherwise he/she can pass a new object with new canvasId which would be wrong.
Also I recommend you to remove the setter of the CanvasId property and pass the id in the constructor if you want to specify specific canvas id

public class ChartJSChart<T> where T: ChartJsDataset
{
    public ChartJSChart()
    {

    }

    public ChartJSChart(string canvasId)
    {
        this.CanvasId = canvasId;
    }

    public string ChartType { get; set; } = "bar";
    public ChartJsData<T> Data { get; set; }
    public ChartJsOptions Options { get; set; }
    public string CanvasId { get; } = $"BlazorChartJS_{new Random().Next(0, 1000000).ToString()}";
}

muqeet-khan added a commit that referenced this issue Sep 14, 2018
muqeet-khan added a commit that referenced this issue Oct 8, 2018
@muqeet-khan
Copy link
Owner

I am closing this now, as this should be fixed by release 0.4.0. If you still have issues let me know and I can reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants