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

Use CSS style for colors #33

Merged
merged 2 commits into from
Jan 10, 2024
Merged

Use CSS style for colors #33

merged 2 commits into from
Jan 10, 2024

Conversation

patrislav1
Copy link
Collaborator

This PR is trying to use CSS styles for recurring style attributes (primarily for colors) to 1) reduce SVG size and make it more readable and 2) make colors easily adjustable in the SVG code for all characters that use it. This is WIP, I thought I could get away with using a single set of color styles for both background and foreground, but turns out that setting the stroke color affect the text rendering.

@wader
Copy link
Owner

wader commented Jan 8, 2024

Nice, good idea. When messing around with these thing you really appreciate having good tests :) so please add more if you like

@wader
Copy link
Owner

wader commented Jan 9, 2024

Some possible things to look into:

  • <g> can be used to inherit attributes and transformations
  • <use> be be used to duplicate a <g>

But as you also mentioned i think we should probably stay away from too crazy tricks to keep the svg easily editable. For
example i wonder if the background could be a small embedded png that is stretched? :)

@patrislav1
Copy link
Collaborator Author

I've improved it a bit, can you check again?
(Not sure if this needs additional tests..?)

I can already think of ways to make the file more compact with more use of <g>, also probably stuff like bold and italic can be pulled out to a class style definition, making the SVG code even more compact.

should we do that in separate PRs or right here? (I'm fine with merging this already if you're satisfied)

svgscreen/svgscreen.go Outdated Show resolved Hide resolved
@wader
Copy link
Owner

wader commented Jan 9, 2024

Output looks good, can merge if you want 👍 had some minor code comments but no biggies.

For new features, regressions or some tricky edge cases i usually try to add some kind of test

@patrislav1
Copy link
Collaborator Author

🚀

@wader wader merged commit 95d184a into wader:master Jan 10, 2024
1 check passed
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