asciidoctor / asciidoctor-chart

A set of Asciidoctor extensions that add a chart block and block macro to AsciiDoc for including charts in your AsciiDoc document.
Other
30 stars 13 forks source link

resolves #29 update c3.js defaults to latest version 0.7.20 #32

Closed uniqueck closed 2 years ago

uniqueck commented 2 years ago

fixes #29

ggrossetie commented 2 years ago

Did you make sure that everything works as expected? It's surprising that there's not a single breaking change?

uniqueck commented 2 years ago

How? Should I use this gem and render some example files and check if this chart looks ok?

ggrossetie commented 2 years ago

Yes, we can add a few examples in this repository to make it easier to test when we upgrade. We could potentially use something like Cypress to automate things but one step at a time 😉

uniqueck commented 2 years ago

How do you generate those example graphics? Do you install this gem and you this to generate these png / svg?

ggrossetie commented 2 years ago

How do you generate those example graphics?

Are you referring to the screenshots in the README? I created an AsciiDoc file, converted it to HTML with this extension enabled and then took screenshots. Technically, we could automate the process using Puppeteer but I don't know if it's worth it.

uniqueck commented 2 years ago

My question was more aimed at how to use my local packaged Ruby Gem to generate these previews?

ggrossetie commented 2 years ago

I believe the easiest way is to require lib/asciidoctor-chart.rb from your local copy:

asciidoctor -r ./lib/asciidoctor-chart.rb doc.adoc
uniqueck commented 2 years ago

Do you know a way to call npm from bundle exec? I would like to use cypress for visual regression tests and the cypress gem for ruby is for rails.

ggrossetie commented 2 years ago

I don't mind adding a package.json (since we want to transpile this extension to JavaScript). You can call npm directly and use Cypress from Node.

ggrossetie commented 2 years ago

However, please create a dedicated issue and pull request to introduce this change.

uniqueck commented 2 years ago

However, please create a dedicated issue and pull request to introduce this change.

Yes that was my plan, but i would like to try it and investigate for a solution, to check if all is fine after upgrade to latest version of c3.js.

uniqueck commented 2 years ago

I created another issue #33 for that, i think it is a good idea to wait with this change for a solution of #33.

uniqueck commented 2 years ago

Should rebased and merged after #34.

ggrossetie commented 2 years ago

The rendering slightly changed but it's barely noticeable:

After

image

Margin between 0 and 1 at the bottom is thinner and the graph is slightly wider.

Before

image

ggrossetie commented 2 years ago

Rebased on master and base image for C3.js updated