opensim-org / opensim-viewer

Front end of web based viewer of OpenSim models, data
Apache License 2.0
13 stars 3 forks source link

Integrated charts using chartjs-wrapper. #102

Closed AlbertoCasasOrtiz closed 1 year ago

AlbertoCasasOrtiz commented 1 year ago

Integrated charting library (installing wrapper from npm).

Since there are countless options to create/customize charts, instead of creating functions to modify everything, I rely on the original json-based configuration. I have added an example at /chart/ with the full code commented.

netlify[bot] commented 1 year ago

Deploy Preview for comforting-speculoos-d9e100 ready!

Name Link
Latest commit 285e2a0f56a519c50f4699e3b1891f67b796e53c
Latest deploy log https://app.netlify.com/sites/comforting-speculoos-d9e100/deploys/651364afe942ac0008f7e4ed
Deploy Preview https://deploy-preview-102--comforting-speculoos-d9e100.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

AlbertoCasasOrtiz commented 1 year ago

Looks like there is a Jest issue, I am going to fix.

AlbertoCasasOrtiz commented 1 year ago

@aymanhab Jest errors are fixed now. The latest error was related to CI having node version 12, which is deprecated and was not compatible with one of the dependencies we use (I think it was @adobe/css-tools).

I updated to version 16 in this PR, since it is the recommended version: https://github.blog/changelog/2023-06-13-github-actions-all-actions-will-run-on-node16-instead-of-node12-by-default/

Looks like there is now an issue with the wrapper, so I will look into it.

AlbertoCasasOrtiz commented 1 year ago

@aymanhab I finally figured out what is happening.

The ChartJS library uses EcmaScript Modules, and Jest is not compatible with it. This is why the charts were working locally, while tests were complaining about the syntax of EcmaScript in the wrapper and in the ChartJS library.

I just had to fix a few things in the wrapper and comment the text that imports ChartJS and everything is working now. We have to find a way of either doing the test ignoring EcmaScript modules, or make Jest support it.

aymanhab commented 1 year ago

@AlbertoCasasOrtiz Trying to build an integrated environment from latest PRs for the demo failed because of compiler issues, let's investigate when you're around.