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

Convert to octicon SVG library #9258

Closed
wants to merge 1 commit into from
Closed

Conversation

jolheiser
Copy link
Member

This is an exploratory PR looking to (incrementally) replace the webfont Octicons with SVGs using https://gitea.com/go-icon/octicon

The version of octicons we use is one of the last before they abandoned webfonts and moved to only SVGs.
The vendored library basically converts the SVGs into Go-code that can be used to inline them.
Updating it is trivial compared to a webfont (not to mention we can't update it otherwise, as noted above)

For this draft, I updated the repository header icons for a preview.
Images are with no avatar, though it also updates the icons placed after the repo name if there is an avatar.
public
public-template
private
private-template

@guillep2k
Copy link
Member

Our current bindata.go system compresses the resources into a virtual file system, so Gitea's executable stays compact. Perhaps there's a way of compressing this too?

How will this affect custom templates that currently use the octicon font?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 5, 2019
@lafriks
Copy link
Member

lafriks commented Dec 5, 2019

I think I would prefer that icons would stay purely in client side (css/webfont/js)

@silverwind
Copy link
Member

silverwind commented Dec 5, 2019

Inline SVG is certainly the way to go, it will for example allow to have multi-colored icons and allow proper targeting/sizing via CSS. Webfonts are just a ugly hack in my eyes.

@lafriks
Copy link
Member

lafriks commented Dec 5, 2019

Problem is that we can't really use them in dynamic client side scripts/vue components than

@jolheiser
Copy link
Member Author

Our current bindata.go system compresses the resources into a virtual file system, so Gitea's executable stays compact. Perhaps there's a way of compressing this too?

To be honest, I don't know how much it can compress inside the Go code it lives in.
Gitea binary size (on Windows):
TAGS="bindata sqlite sqlite_unlock_notify" make build

  • Without this library: 61.3MB
  • With this library: 61.4MB
    Sorry, I know that's not a great comparison, but it seems to be fairly small.

How will this affect custom templates that currently use the octicon font?

Currently, it won't. The two can live independently of each other.

I think I would prefer that icons would stay purely in client side (css/webfont/js)

Unfortunately we cannot ever update octicons then, as they no longer have a webfont. (Though there are some third-party libraries that seem to be attempting to re-create the webfonts)
This library still renders on client side, all it does is spit out the correct SVG XML.

@silverwind
Copy link
Member

silverwind commented Dec 5, 2019

Problem is that we can't really use them in dynamic client side scripts/vue components than

There are ways to achieve that. The backend could serve a "spritemap" in HTML and individual images could be referenced via <svg><use xlink:href="#icon"></use></svg>.

https://github.com/cascornelissen/svg-spritemap-webpack-plugin
https://css-tricks.com/svg-sprites-use-better-icon-fonts/

@lafriks
Copy link
Member

lafriks commented Dec 5, 2019

@silverwind I don't mind svg icons but I don't like that icons are being generated in html templates from go code

@silverwind
Copy link
Member

silverwind commented Dec 5, 2019

Yeah, agree on that one, that kills them for any frontend usage. Ideally I'd like to see us having a directory of SVGs to be bundled into one file via webpack or similar. Then, both backend and frontend code can load them via use tags.

@lunny lunny added the topic/ui Change the appearance of the Gitea UI label Dec 6, 2019
@lunny
Copy link
Member

lunny commented Dec 6, 2019

@guillep2k I have sent a PR #7109 to add compress to embbed resources. But that may have some conflict with macaron static middleware. That needs more work.
@lafriks we can store the generated files.

@guillep2k
Copy link
Member

The performance aspect of using SVGs is a hairy problem. There are pros and there are cons:

  • A font resolves all icons at once, and they are all cached. It will be the best solution if performance is the only concern. There may be an equivalent option with SVG (more on this later).
  • Out of line SVGs require an extra HTTP request, but once requested the resources can be cached. They can also be gzipped.
  • Inline SVGs do not require an extra HTTP request, but the information is sent over the network every time an icon is shown and also increases CPU usage due to extra data for compression (when gzip is enabled).

There may be a technique to send all possible icons in a single SVG file to the client and resource from that, a concept similar to a spritemap but in the form of a XML file with all the SVGs inside as snippets, managed by a web worker that can slice by ID. I've never tried this, so I don't know if it's taxing for the user agent, but from the network point of view it should be on par with the font option.

@jolheiser
Copy link
Member Author

I think ideal usage would indeed be a spritemap.
If someone wants to use webpack and some plugin to bundle SVGs into a spritemap, that would probably be one of the best options.

I only went this route because I am more comfortable with Go than the JS ecosystem.
But it sparked the conversation, so I'll count it as a win. 😉

@silverwind
Copy link
Member

@guillep2k spritemap is one single request, just like a webfont. It can be inlined in HTML too, thought that will not be so easy for us as HTML is not in webpack (yet).

@silverwind
Copy link
Member

silverwind commented Dec 6, 2019

Thought it could be baked right into a go template too I guess in a readable format like

<svg xmlns="http://www.w3.org/2000/svg" style="display: none">
  <symbol id="codepen" viewBox="0 0 64 64">
    <path etc.../>
  </symbol>
  <symbol id="youtube" viewBox="0 0 64 64">
    <path etc.../>
  </symbol>
  <symbol id="twitter" viewBox="0 0 64 64">
    <path etc.../>
  </symbol>
</svg>

then include that template in index.html and <use> wherever required.

@guillep2k
Copy link
Member

@guillep2k spritemap is one single request, just like a webfont. It can be inlined in HTML too, thought that will not be so easy for us as HTML is not in webpack (yet).

Oh, that's what I was talking about. I didn't know the term spritemap was used beyond bitmaps. 😝

Inlining seems inefficient, because we won't be able to cache it. Seems to me that it's better off as a separate resource, that can be cached.

@jolheiser
Copy link
Member Author

Closing this PR, it seems we've come to a proposed solution.

@jolheiser jolheiser closed this Dec 6, 2019
@jolheiser jolheiser deleted the octicon branch December 6, 2019 16:54
@jolheiser jolheiser mentioned this pull request Feb 2, 2020
3 tasks
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants