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 svg use for svg icons #1428

Merged
merged 3 commits into from
Jul 18, 2024
Merged

Use svg use for svg icons #1428

merged 3 commits into from
Jul 18, 2024

Conversation

Earlopain
Copy link
Collaborator

You can imagine this as a sort of spritemap if you will. Icons can be referenced by id and inserted into an svg element with use. On the jobs index page, size is reduced from 305kb to 237kb. Quite nice. The browser also only has to parse the content once.

Closes #1364


Firefox has differences in how the content security policy needs to be set up. On chrome image-src takes, firefox requires default-src.

Additionally, href in use interprets query parameters differently. I believe they would be accessible as parameters in the svg itself and both firefox and chrome render nothing if a query parameter is used. So I made the version part of the path instead in a separate commit.

`<use href>` can't make use of query parameters through the normal means
You can imagine this as a sort of spritemap if you will.
Icons can be referenced by id and inserted into an `svg` element with `use`.

On the jobs index page, size is reduced from 305kb to 237kb. Quite nice
@bensheldon
Copy link
Owner

@Earlopain thank you! There's never been really a good reason for doing individual icons vs a sprite or an icon font other than the file size and my reluctance to unnecessarily bloat up the gem package. And balance that against the effort of rebuilding a custom sprite sheet when I need to add more icons (which is not my favorite)

The other option would be to add the entire bootstrap-icons.woff2 (~130kb) and its css (~60kb). What do you think of that?

@Earlopain
Copy link
Collaborator Author

I don't see much difference to how it is currently except that they now reside in a single file.

For my own project which uses fontawesome I construct the svg file programmatically through the contents of the npm package and assert in a test that it is up-to-date. That's about 50 lines including the test but also rather overkill since there aren't that many different icons here.

I would consider myself a minimalist so my answer about bundling the whole font + css should be pretty clear, it's ~190kb vs ~8kb. With fontawesome at least that also increases over time when they add new icons which doesn't feel that great when I'm not using them. Still, it would be an improvement over current situation, except that it would also bloat the gems size.


I'm not particularly attached to this PR. I saw the issue and thought this was the direction you were looking at, and since I did something like this myself already thought why not. I'm good if you had something different in mind.

@bensheldon
Copy link
Owner

I don't see much difference to how it is currently except that they now reside in a single file.

Oh! 🤦🏻 I just looked in the file and saw that the icons are clearly enumerated in the file. That's not difficult to modify at all 😁

@Earlopain
Copy link
Collaborator Author

Oops, yeah. Perhaps I should have elaborated a bit more here. GitHub web just renders it (which is fair, most of the time that works out) but not really useful here.

@bensheldon
Copy link
Owner

Just fixed one misnamed SVG and I'm gonna let CI run through and then merge this. Thank you!

@bensheldon bensheldon added the refactor Code changes that do not introduce new features label Jul 18, 2024
@bensheldon bensheldon merged commit 17d3369 into bensheldon:main Jul 18, 2024
@Earlopain
Copy link
Collaborator Author

🤦nice catch

@Earlopain Earlopain deleted the svg branch July 19, 2024 05:30
bensheldon added a commit that referenced this pull request Jul 31, 2024
* Scope assets by version through a path element

`<use href>` can't make use of query parameters through the normal means

* Use html `use` elements for svg icons

You can imagine this as a sort of spritemap if you will.
Icons can be referenced by id and inserted into an `svg` element with `use`.

On the jobs index page, size is reduced from 305kb to 237kb. Quite nice

* Fix dash_circle naming

---------

Co-authored-by: Ben Sheldon [he/him] <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code changes that do not introduce new features
Projects
Development

Successfully merging this pull request may close these issues.

Change how svg images are inserted into partials
2 participants