Skip to content
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

migrate plugin to Typescript and generate modular packages #24

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

migrate plugin to Typescript and generate modular packages #24

wants to merge 17 commits into from

Conversation

stklcode
Copy link

@stklcode stklcode commented Jan 27, 2023 •
edited
Loading

This is quite a major update.
The Plugin and the project structure are updated to get closer to the Chartist v1 ecosystem.

I was wondering if the changes should be part of a new, shared repo, but I couldn't manage to start a discussion upstream. My goal is not to create and maintain yet another fork, so maybe we want to move this project in the proposed direction...

Changes:

  • Migrate sources from JavaScript to Typescript
  • Use Chartist v1 API instead of the hacky workaround in v1
  • Remove "donutSolid" Pie Chart support
    They are no longer available in Chartist.
  • Remove "currency" and "currencyFormat" options
    Kind of tricky to use and they can easily be replaced with a one-line callback.

Build Process:

  • Replace Grunt with Yarn tasks
  • Build ESM, CJS and UMD versions using Rollup.js
  • Extended unit tests
  • (Re-)Introduce ESLint and Prettier

New usage example:

import { LineChart } from 'chartist';
import { ChartistPluginTooltip } from 'chartist-plugin-tooltips';

new LineChart(
  '.ct-chart',
  {
    labels: [1, 2, 3, 4, 5, 6, 7],
    series: [
      [1, 5, 3, 4, 6, 2, 3],
      [2, 4, 2, 5, 4, 3, 6]
    ]
  },
  {
    plugins: [
      ChartistPluginTooltip
    ]
  }
);

or as UMD:

<html>
  <head>
  <script src="node_modules/chartist/dist/index.umd.js"></script>
  <script src="node_modules/chartist-plugin-tooltips-updated/dist/chartist-plugin-tooltip.umd.min.js"></script>

  <script>
    new Chartist.LineChart(
      '#chart',
      {
        labels: [1, 2, 3, 4, 5, 6, 7],
        series: [
          [1, 5, 3, 4, 6, 2, 3],
          [2, 4, 2, 5, 4, 3, 6]
        ]
      },
      {
        plugins: [ ChartistPluginTooltip ]
      }
    );
</script>
  </head>
  <body>
  <div id="chart"></div>
  </body>
</html>

@stklcode
ci: use Yarn instead of NPM
724b3a3
@stklcode
introduce eslint code style checks
874ef1d 
Ruleset copied from the one used in the Chartist project.
@stklcode
update datatypes for compatibility with Chartist v1 UMD version
56daef7 
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
refactor build process
4a50cd3 
* migrate Grunt to Yarn tasks
* use Jest in favor of Jasmine for testing
* remove generated CSS sources
* use single build pipeline instead of Node matrix
@stklcode
migrate sources to Typescript
fa79bbe
@stklcode
build UMD version with Rollup
7e06927
@stklcode
remove Gruntfile
d4eee0b
@stklcode
extend inline documentation and tests
b410727
@stklcode
introduce stylelint
a2554f6
@stklcode
update README, CHANGELOG and LICENSE
edc0d7c
@stklcode
re-enable prettier
8649941
@stklcode
build ESM, CJS and UMD version
bccf90b
@stklcode
CHANGELOG++
ef58fbd
@stklcode
replace deprecated page{X,Y}Offset with scroll{X,Y}
123d251 
Chartist itself is not IE11 compatible anymore, so there is no
reason to stick with the legacy aliases here.
@stklcode
update and lint TSDoc comments
306b849
@stklcode
remove support for "donutSolid" pie charts
ec05e45 
This option no longer exists, so the special case is no longer
required in the plugin either.
@stklcode
drop support for "currency" and "currencyFormatCallback"
dea4c14 
There is another callback option "tooltipFnc" that allows transformation
of the value string. The currency options are hardcoded as prefix and
the transformation is applied in non-intuitive order. Finally, adding a
currency symbol can be achieved with a single callback:
  (value) => '$ ' + value
@stklcode stklcode force-pushed the modular branch 2 times, most recently from 8cb0de2 to 569090a Compare January 27, 2023 09:53
@stklcode
Copy link
Author

I tried to start a discussion about moving common plugins to the @chartist-js org: chartist-js/chartist#1391

@lukbukkit
Copy link
Owner

Sorry, for the long radio silence. The last months were very busy for me. I'll review your contributions in the following days or weeks. Did your code works well in production or do you've noticed any possible drawbacks?

@stklcode stklcode force-pushed the modular branch 2 times, most recently from 2fbeedc to dea4c14 Compare June 17, 2023 09:43
@stklcode
Copy link
Author

Just rebased the PR on top of the latest changes, so it can be applied cleanly again.

I'm using the updated lib in one project with various line charts and it served my need since I introduced the changes. (it's in an admin UI, so I currently don't have a public site to link here). I have not tested every single feature.

Don't know if there is any more complete example page that could be simply migrated to see if everything works as expected. We could probably build one.

@lukbukkit
Copy link
Owner

I'm sorry that I haven't found the time so far to review the pull request. My time available for open-source projects has decreased significantly in the last year. Are you using the plugin in production, and if so, would you be interested in maintaining it in the future?

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
None yet
Projects
None yet
Milestone
No milestone
Development

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