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 style argument which can convert SVG attributes to CSS Style attribute (the opposite of style_to_xml.) #267

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

Conversation

doom-goober
Copy link

Currently, scour allows the user two options when it comes to SVG Attributes versus CSS Style: They can either convert everything to SVG Attributes or leave all the attributes as they were originally encoded. Obviously, there is a third option, which is currently missing: convert everything to CSS Style.

While SVG Attributes are generally preferred for various CSS overriding reasons, some tools don't handle them well and prefer CSS Style. For example, Inkscape's "Save As HTML 5 Canvas" doesn't support SVG Attributes but does support CSS Style. For this reason, tools like Illustrator allow the user to choose between "Inline" or "Presentational Attributes" (or a third option, "Internal CSS" which is out of scope for this discussion.) The eventual goal of this change would be to add an option to Inkscape so it behaves somewhat like Illustrator with three options: "Preserve", "Presentational Attributes", "Inline".

This pull request adds the option to convert styling all to CSS Style through --style=inline. The other options are --style=none, --style=attributes, --style=preserve.

The backwards compatibility logic is slightly annoying. If you specify a --style other than "none", then that style will be used. If you specify --style=none (or specify nothing, as none is the default) then --disable-style-to-xml or --style-to-xml will decide the behavior. Perhaps "none" could renamed "default?"

@codecov-io
Copy link

codecov-io commented Dec 20, 2020

Codecov Report

Merging #267 (cebd259) into master (7a83e71) will decrease coverage by 0.01%.
The diff coverage is 92.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #267      +/-   ##
==========================================
- Coverage   94.14%   94.13%   -0.02%     
==========================================
  Files           5        5              
  Lines        2272     2284      +12     
==========================================
+ Hits         2139     2150      +11     
- Misses        133      134       +1     
Impacted Files Coverage Δ
scour/scour.py 94.61% <92.30%> (-0.02%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a83e71...cebd259. Read the comment docs.

Copy link
Contributor

@nthykier nthykier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

Looks good from my PoV. I have added some suggestions as a occasional contributor.

scour/scour.py Outdated Show resolved Hide resolved
scour/scour.py Outdated Show resolved Hide resolved
scour/scour.py Outdated Show resolved Hide resolved
@doom-goober
Copy link
Author

doom-goober commented Dec 4, 2021

@nthykier Sorry for leaving this pull request open for so long. Real life threw some turbulence at me, but I have finally incorporated your suggestions. Sorry again.

@nthykier
Copy link
Contributor

nthykier commented Dec 5, 2021

Hi @doom-goober

No worries. I am not a member of the scour project, so my words were only intended as suggestions and nothing more. :)

@joneuhauser
Copy link

joneuhauser commented Mar 14, 2022

Copying over my comment from the downstream issue in Inkscape:

It's not that simple, unfortunately, because inline CSS and presentation attributes have not the same specificity (inline CSS: specificity infinity, presentation attributes: specificity 0).

In practice that means: Try adding style="fill:blue;" or fill="blue" to the element in Gitlab op's source.svg. For the former, the element becomes blue, for the second, it doesn't. As soon as there is any stylesheet affecting the element, even a path {} selector, it will take precedence over presentation attributes.

Just changing inline css to presentation attributes or vice versa is not going to cut it (that is also true for the existing code converting CSS to presentation attributes). I wouldn't want my SVG optimizer to change the way the exported image is rendered.

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.

4 participants