Skip to content

Adjust class checks for compatibility with Chartist v1 #20

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jan 2, 2023
Merged

Adjust class checks for compatibility with Chartist v1 #20

merged 4 commits into from
Jan 2, 2023

Conversation

stklcode
Copy link

@stklcode stklcode commented Dec 15, 2022 •
edited
Loading

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>

@stklcode
migrate Node Sass to Dart Sass and remove unused CoffeeScript dependency
4119f26
@stklcode
ci: update GitHub actions and add Node 18
af2710e
@stklcode
update datatypes for compatibility with Chartist v1 UMD version
361877d 
The chart classes have been renamed from Bar, Pie and Line to BarChart,
PieChart and LineChart.
The rest of the logic still works fine from here on.
@stklcode stklcode mentioned this pull request Dec 15, 2022
Merged
@lukbukkit lukbukkit added the bug Something isn't working label Dec 17, 2022
@lukbukkit
Copy link
Owner

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 👍

@stklcode stklcode mentioned this pull request Dec 18, 2022
Open
@lukbukkit
Copy link
Owner

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.

@lukbukkit lukbukkit merged commit 288986b into lukbukkit:master Jan 2, 2023
@stklcode stklcode deleted the master branch January 2, 2023 14:49
@lukbukkit lukbukkit mentioned this pull request Jan 26, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stklcode @lukbukkit