Put SVG images into CSS #2623
Open
Put SVG images into CSS #2623
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
S-waiting-on-review
GuillaumeGomez
force-pushed
the
svg-into-css
branch
from
a156283
to
bb9d795
Compare
GuillaumeGomez
commented
Sep 30, 2024
|
@syphar: Any idea why there are sqlx errors? |
GuillaumeGomez
force-pushed
the
svg-into-css
branch
from
bb9d795
to
ecf053b
Compare
GuillaumeGomez
force-pushed
the
svg-into-css
branch
from
fc7a51f
to
416d7bc
Compare
syphar
reviewed
Oct 4, 2024
build.rs
Outdated
| @@ -80,11 +84,194 @@ fn main() -> Result<()> { | |||
| write_known_targets(out_dir)?; | |||
| compile_syntax(out_dir).context("could not compile syntax files")?; | |||
|
|
|||
| println!("cargo::rustc-check-cfg=cfg(icons_out_dir)"); | |||
There was a problem hiding this comment.
I'm likely missing something, but where is this cfg ever set?
Or is this just for debugging this build script?
There was a problem hiding this comment.
It's used in src/lib.rs to import the generated Rust file.
build.rs
Outdated
| let css_class = format!("f-a_{icon_name}_{icon_kind}"); | ||
| css_output.push_str(&format!( | ||
| "\ | ||
| .{css_class} {{ |
There was a problem hiding this comment.
what I wonder:
isn't a CSS distribution with the icons something that font-awesome already provides? That we can just use?
|
not finished yet with all review, but one bigger question |
GuillaumeGomez
force-pushed
the
svg-into-css
branch
from
175247b
to
607615a
Compare
GuillaumeGomez
force-pushed
the
svg-into-css
branch
from
607615a
to
c9498fb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since CSS files are often cached, we can take advantage of this to reduce network usage.
Now how it works: we inline the SVG images into a CSS file we generate at compile-time to ensure this file only contains what we need to limit its use but also to generate equivalent Rust types so we know at compile-time that the SVG exists.
I checked on caniuse for
maskhere: https://caniuse.com/css-masks. It says that more than 89% of current web browsers supports it unprefixed (without-webkit-). With the prefix, it goes up to more than 96%. Just in case, I used both (thanks to HTML variables, we don't need to duplicate the SVG for bothmask) so we should be on the safe side with more than 96% of our potential users being able to see the icons.I compared the HTML size on https://docs.rs/sysinfo/0.30.12/sysinfo/index.html. On current docs.rs, the transferred HTML is 52KB, with this PR is goes down to 40KB.