lukbukkit / chartist-plugin-tooltip

Tooltip plugin for Chartist.js
MIT License
16 stars 13 forks source link

Adjust class checks for compatibility with Chartist v1 #20

Closed stklcode closed 1 year ago

stklcode commented 1 year ago

Problem:

In Chartist v1 the chart classes have been renamed from Bar, Pie and Line to BarChart, PieChart and LineChart.

Version 0.1.4 raises an error during initialization:

if (chart instanceof Chartist.Bar) {

TypeError: Right-hand side of 'instanceof' is not an object

Proposed solution:

Using the UMD version, renaming the classes is the only change required to run with latest Chartist v1.3.0. So this PR proposes changing the class names and updates the dependencies accordingly.

Minimal working example:

<html>
<head>
  <title>Chartist test</title>
  <script src="node_modules/chartist/dist/index.umd.js"></script>
  <script src="node_modules/chartist-plugin-tooltips-updated/dist/chartist-plugin-tooltip.min.js"></script>
  <link rel="stylesheet" href="node_modules/chartist/dist/index.css">
  <link rel="stylesheet" href="node_modules/chartist-plugin-tooltips-updated/dist/chartist-plugin-tooltip.css">
</head>
<body>
<div id="chart" style="height: 50vh"></div>

<script>
  new Chartist.LineChart(
    '#chart',
    {
      labels: [1, 2, 3, 4, 5, 6, 7, 8],
      series: [[5, 9, 7, 8, 5, 3, 5, 4]]
    },
    {
      low: 0,
      showArea: true,
      plugins: [ Chartist.plugins.tooltip() ]
    }
  );
</script>
</body>
</html>
lukbukkit commented 1 year ago

Thanks for all of your work. I'm currently a bit busy, but I'll try to review of all your changes as soon as possible. So far they look great 👍

lukbukkit commented 1 year ago

I've completed review and your changes look good. I've improved the documentation a bit so it fits to the new Chartist version. We also have to increase the major version (1.0.0) to comply with semantic versioning as we're introducing a breaking change.