DistributedProofreaders / dproofreaders

Distributed Proofreaders is a web application intended to ease the process of converting public domain books into e-texts.
https://www.pgdp.net
GNU General Public License v2.0
46 stars 28 forks source link

Replace jpgraph with a SVG-based library #559

Closed cpeel closed 2 years ago

cpeel commented 3 years ago

jpgraph is an external, manual dependency required to render stats graphs. Because jpgraph renders PNGs they are difficult to theme -- impossible with CSS even. Using an SVG-based graphing library should enable us to theme the graphs using CSS.

Something like svggraph might work and is available via composer (https://packagist.org/packages/goat1000/svggraph) which would remove the need for a manual dependency.

cpeel commented 3 years ago

Some investigation with svggraph show that in order to make the graphs theme-able they need to be pulled inline into the HTML (see Embedding the SVG in a page option 5). It would be far better if we could theme "stand-alone" SVG files and embed them instead (option 1).

It's worth investigating if stand-alone SVG files inserted into the HTML using <embed> tags can have and use stylesheets -- which is implied in the SVT working draft. If so, svggraph would need to be updated to include a stylesheet reference and attach class attributes to objects instead of an explicit color.

chrismiceli commented 3 years ago

Our charts are pretty simple and there are many javascript charting libraries that may be easily themeable. My experience is with the paid highcarts solution, but there are others such as:

  1. https://d3js.org/
  2. https://developers.google.com/chart/ (google scares me)
  3. https://www.chartjs.org/
cpeel commented 3 years ago

Note that we explicitly want them theme-able via CSS - otherwise we have to support theming in two places (CSS and in the code).

chrismiceli commented 3 years ago

I implemented a proof of concept with https://d3js.org here: https://www.pgdp.org/~chrismiceli/c.branch/graphs/stats/projects_Xed_graphs.php?which=PPd

I only converted the first bar chart as an example to get feedback. d3 supports theming, but the api is a little low level. Luckily it is super popular throughout the web for data visualization and supports everything we need. Here's a zoomed out screenshot of the implementation on charcoal theme showing the chart 1st in d3, and then the current jpgraph (note, data doesn't match because my local env didn't have good data so I made some up for the d3 chart). image

Is a client side charting library ok? It will involve more data being sent to the client and the client browser processing the data to create the svg. On the other hand, it should reduce some server CPU demand when image cache misses I would imagine. Either solution is more accessible, easier to copy from, and themeable via svg classes and currentColor value.

cpeel commented 3 years ago

A client-side library is fine by me, but we need to figure out how we can support all of our current chart use-cases with those from a practical perspective (i.e. I don't want to support 2 charting libraries). The nice thing about image-based graphs is that they're stupidly easy to insert in places with a simple tag -- which means we've done so in lots and lots of places. For instance, the round activity graphs in the news item on the P1 page.

Some of the graphs have a lot of data on them currently and we would be sending a lot more data over the wire to the client. I'll see if I can dump a JSON of one of the larger graphs on PROD for you to play with.

chrismiceli commented 3 years ago

@cpeel @srjfoo , should I move this discussion to slack? I was curious if this was the best place for a discussion.

srjfoo commented 3 years ago

@cpeel @srjfoo , should I move this discussion to slack? I was curious if this was the best place for a discussion.

I don't have a strong opinion either way, but do tend to gravitate to Slack if I know there's likely to be a lot of discussion. I'll defer to @cpeel's preference.

cpeel commented 2 years ago

This was officially merged into master in https://github.com/DistributedProofreaders/dproofreaders/pull/697 and rolled out to pgdp.net on 2021-10-12.

Excellent job @chrismiceli - thank you!