fxcosta / laravel-chartjs

Simple package to facilitate and automate the use of charts in Laravel 5.x using Chartjs v2 library
486 stars 112 forks source link

chart-pie-doughnut: js-legend-pie element is null #10

Closed vdhicts closed 7 years ago

vdhicts commented 8 years ago

In chart-pie-doughnut.blade.php (but also in others) there's a stement which generates a legend for a pie chart:

document.getElementById('js-legend-pie').innerHTML = PizzaChart.generateLegend();

There are a couple of problems with this code:

To prevent javascript errors, it would be a solution to check if the element exists:

var legend = document.getElementById('js-legend-pie');
if (legend) {
    document.getElementById('js-legend-pie').innerHTML = PizzaChart.generateLegend();
}

It's no problem to fix this and create a pull request, but I wanted to discuss this with you first.

fxcosta commented 8 years ago

Thanks for the feedback!

As you can see on my list of things to do, organize the javascript code is my priority. The code is old and needs a serious refactoring because I learned new things, things have changed and etc.

In fact this problem with the selectors is very serious and I would be grateful with your help if you know how we can solve this problem in this package.

Feel free to suggest improvements in the code!

vdhicts commented 8 years ago

As soon as I have some time to spare, I will give it a try.

AshishGupta001 commented 8 years ago

I have solved this 'selector' issue by using 'convention over configuration'. For example if I have a canvas for a chart with id = 'myChart' then I have a convention that the id for legend will be 'myChart_legends'. I have then modified the laravel-chartjs code accordingly.

fxcosta commented 7 years ago

With this new version, the configuration is irrelevant. Just check the documentation of the chartjs and write this in php arrays.