methnen / m-chart-highcharts-library

Adds Highcharts library to M Chart.
Other
7 stars 1 forks source link

stacked area charts? #27

Open nkryptic opened 2 years ago

nkryptic commented 2 years ago

Hi,

Thanks for merging the PRs. I'm wondering if you'd be interested in adding stacked area charts? It's something that would also be applicable and could be mirrored with the charts.js charts in the main m-charts plugin.

I'd be happy to add a pull request for both repositories to add the chart type. If you're keen on it, I'd also inquire if adding another chart type of 'stacked-area' makes sense, or whether it would be preferable to move the "stacked" flag out to a separate checkbox that would only be shown for column, bar, and area charts. Going this route would of course require an upgrade step like you've done previously with the upgrade_to_1_7_4 function.

Cheers

methnen commented 2 years ago

I'd be fine with that in the next version.

I'd still add it as a separate chart type instead of a toggle though given the current number of options in that settings panel is already kind of crowded at times.

I'd like to get this next version of the library and the core plugin out today or tomorrow though so I'm gonna lock down any bigger changes for the time being.

And adding a new chart type would be 1.x level change anyway instead of a 1.x.x level change. :)

And yes, I'd want to make sure it's implemented in chart.js as well. I'm trying to keep the core plugin with basic feature parity going forward, it's taken a few years to get there but it's basically there at this point.

nkryptic commented 2 years ago

Sounds good. Then, I'd ask some follow-up questions:

methnen commented 2 years ago

Sorry for the delay in responding. Paying work has been kind of crazy recently.

I just pushed those two branches live.

I think it's fine to keep the theme hook specific. If you didn't you'd need to pass an additional $library argument to handle things properly anyway. Since we've already launched with one way lets just follow through with the core library.

And yeah, I think it makes sense to keep some feature parity between them.

I've gone ahead and created 1.3 and 1.10 branches and PRs respectively:

That way I'm no longer blocking you from working on this if you have the time:

https://github.com/methnen/m-chart/pull/167

https://github.com/methnen/m-chart-highcharts-library/pull/28