go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
43.85k stars 5.38k forks source link

Octicons v2 #11889

Closed CirnoT closed 4 years ago

CirnoT commented 4 years ago

Octicons v2 (v10.0.0) have been released https://primer.style/octicons/

Will require some work during upgrade; there are inconsistencies with how icons are used now and certain are missing. Some examples:

The iconset now contains both 24px and 16px icons, all of them have identical width however not all icons exists in 24px set!

Other issues might be present but were not catched yet, feel free to add your observations here.

silverwind commented 4 years ago

The npm package @primer/octicons-v2 should be pretty much a drop-in replacement. It contains 203 icons compared to the 205 from @primer/octicons, internal-repo and north-star icons are absent in v2.

silverwind commented 4 years ago

The iconset now contains both 24px and 16px icons

It still seems to have non-square 16px-based SVGs in the build directory:


$ cat node_modules/@primer/octicons-v2/build/svg/repo.svg
<svg xmlns="http://www.w3.org/2000/svg" width="12" height="16" viewBox="0 0 12 16"><path fill-rule="evenodd" d="M4 9H3V8h1v1zm0-3H3v1h1V6zm0-2H3v1h1V4zm0-2H3v1h1V2zm8-1v12c0 .55-.45 1-1 1H6v2l-1.5-1.5L3 16v-2H1c-.55 0-1-.45-1-1V1c0-.55.45-1 1-1h10c.55 0 1 .45 1 1zm-1 10H1v2h2v-1h3v1h5v-2zm0-10H2v9h9V1z"/></svg>
silverwind commented 4 years ago

Actually, it seems @primer/octicons v10 is the "v2" update. Complete changelog here:

https://github.com/primer/octicons/releases/tag/v10.0.0

Notably, many renames.

CirnoT commented 4 years ago

Yes, along with renames there are some missing icons; sometimes icons with same name is different; for example old star is filled-in but new one is without filling, instead star-fill is filled in.


north-star icons are absent in v2.

https://primer.style/octicons/north-star-16


The changelog confirms that internal repo icon is now replaced by normal repo icon, so that means that GH just positions org avatar on top of it. They did that before too, but now there is no way for us to differentiate it without placing avatar; pretty annoying.

CirnoT commented 4 years ago

We should be good to go for it after https://github.com/go-gitea/gitea/pull/11891 and https://github.com/go-gitea/gitea/pull/11895

CirnoT commented 4 years ago

Given that we are setting size on some icons to 32, it might be good idea to use 16px set exclusively.

silverwind commented 4 years ago

@primer/octicons@10 does include the missing star icon, it was just missing on the old v2 module. A few webpack adjustments will be necessary with the update because size is now in the filename, e.g. repo-16.svg and repo-24.svg.

6543 commented 4 years ago

just a question why are we switching to Octicons v2 - are the old icons not working for a specific usecase anymore?

CirnoT commented 4 years ago

It's just a version bump on npm dependency. Specifically from 9.x to 10.x

They are called v2 because of radical change in style but that does not mean that both versions are maintained.

silverwind commented 4 years ago

While I don't agree with every new icon, I think they are an overall improvement. And yes, remaining on old icons would mean we'd have to lock down the dependency and can't benefit from new additions either.

silverwind commented 4 years ago

https://github.com/go-gitea/gitea/pull/12240