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.

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

Headless SVG generation #11

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

Conversation

jamesscottbrown
Copy link

Description of proposed changes

This PR does several things:

Related issue(s)

This addresses Issues #1 and #8.

Testing

Pull the code, install the dependencies, and run node src/createCladesSchema.js.

This will generate an SVG like:

clades

Opening the file in a web browser, and using the developer tools to set the SVG's width to a fixed value (e.g. 500px), and border to solid shows that it looks centered:

clades

I haven't tested this in Docker

@vercel
Copy link

vercel bot commented Feb 1, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nextstrain/ncov-clades-schema/D627C4bhzRUZUYZNPfmoZoffZYiD
✅ Preview: Failed

@jamesscottbrown
Copy link
Author

jamesscottbrown commented Feb 1, 2022

This breaks the Vercel deployment, as it replaces the Next app with a CLI script. Instead, a GitHub actions script could run node src/createCladesSchema.js and upload the generated SVG file as a build artifact for review.

@ivan-aksamentov
Copy link
Member

ivan-aksamentov commented Feb 2, 2022

Hi James @jamesscottbrown,

Very cool!

I find the string interpolation a bit hacky but glad that it works. It makes automating the svg generation possible, which is great.

As you noted yourself in hodcroftlab/covariants#272, in some cases, it might make sense to extend the React component with some interactivity. So I am not sure whether removing the web app is a win here.

In your opinion, how hard would it be to make both the app and the headless generation working at the same time? How much code we could share between React and Node?

Do you see a way of making the tree nodes and edges into React components, potentially highly interactive, while also preserving the headless generation? Again, how much code could be shared between the 2 paths in this scenario?

I wonder if the string interpolation can be turned into React and then React Server-Side Rendering capabilities can be used in Node to generate a string. If that would work, that would be the cleaner implementation and we could share almost all the code with the web app. Although, this solution is also more complex.

In the end, decision of web app or no web app depends on how we want to use the schema. I think there were some plans, but I am not sure what they are. I asked the team internally, let's see what they say.

@tsibley
Copy link
Member

tsibley commented Feb 2, 2022

@ivan-aksamentov My 2¢: React is really unnecessary here. Even for future interactivity, you can go quite a long way with just plain JS (either directly in the SVG or in an HTML page embedding the SVG).

As I noted internally, templated/generated SVGs have worked really well for me in the past, and it's what I personally would have started with here… I was honestly surprised to see this was a React app initially, but then assumed there was some future plan for it beyond the diagram itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

3 participants