search

methnen / m-chart-highcharts-library

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

Consider adding Highcharts accessibility module #24

Closed nkryptic closed 2 years ago

nkryptic commented 2 years ago

Highcharts provides an accessibility module that only requires the corresponding javascript file to be included along with the Highcharts library. This Highcharts page explains it.

If you'd be amenable, let me know and I can put together a pull request for you.

methnen commented 2 years ago

@nkryptic I'd have no problems with that at all. :)

nkryptic commented 2 years ago

ok, will do. thanks.

methnen commented 2 years ago

Note that there's an existing PR and branch for 1.2.3, feel free to combine that change in there.

It'll go out when the latest M Chart settles down a bit feature wise and gets released.

nkryptic commented 2 years ago

ok. Hopefully, I can remember how to do that... I think that means I create my branch off of the 1.2.3 branch and then add my work and submit the PR?

Btw, are you considering updating the Highcharts js? You still have v9.1.0 from 2021-05-03 and the latest is v9.3.3 from 2022-02-01.

methnen commented 2 years ago

Yes, I almost always try to update Highcharts to whatever the latest is at the time of release. After some quick spot checks to make sure the update doesn't break anything of course.

methnen commented 2 years ago

And yes, you'd just pull the 1.2.3 branch and then submit your PR to that branch instead of master. And when I merge the change in it'll be included in the PR from 1.2.3 into master.

nkryptic commented 2 years ago

Closing this issue, as the PR has the relevant changes: #25