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

Add SVG restyle to match vscode theme #26

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

Conversation

jotavemonte
Copy link
Contributor

@jotavemonte jotavemonte commented May 27, 2024

Summary

Intercept the SVG generated by graphviz and change the colors according to the vscode current theme.

Address the #24 "Make graph nicer." milestone

How it works

Explore the graph using a BFF algorithm, for each node, change the children's values according to their classes.

Evidences

image
image
image

@jotavemonte jotavemonte marked this pull request as ready for review May 27, 2024 19:23
@jotavemonte jotavemonte mentioned this pull request May 27, 2024
3 tasks
@jotavemonte
Copy link
Contributor Author

jotavemonte commented May 27, 2024

@beicause I used the selection background color for the nodes and it isn't ideal in every theme, but in most of them it looks good.

@beicause
Copy link
Owner

I think we'd better to theme the graphviz( .dot ) file directly. Changing the style of svg doesn't affect the .dot file we save.

@jotavemonte
Copy link
Contributor Author

@beicause Sure! Should we have this as a temporary solution and then I can open an issue to migrate the solution to the other end of the file generation? With this strategy, we would have themes for the graphs right away.

@beicause
Copy link
Owner

I have no reason to merge this PR now which adds unnecessary complexity if there is a better approach. Let's improve it firstly.

@jotavemonte
Copy link
Contributor Author

jotavemonte commented May 29, 2024

@beicause I was investigating the options and AFAIK that the color palette is only available at the web view step - we could use some placeholders and interpolate them, but this would add some complexity and processing to the rendering step too.

Do you have an idea on how to approach this? Thank you.

Edit: My previous assumptions were incorrect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants