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.

Sign up for GitHub

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

Jump to bottom

Octicons v2 #11889

Closed
CirnoT opened this issue Jun 15, 2020 · 11 comments · Fixed by #12240
Closed

Octicons v2 #11889

CirnoT opened this issue Jun 15, 2020 · 11 comments · Fixed by #12240
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@CirnoT
Copy link
Contributor

CirnoT commented Jun 15, 2020 •
edited
Loading

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:

  • Private repo has no template icon now, and this is confirmed to be exact on GitHub however they also decided to present the type of repository in this way:
    chrome_2020-06-15_09-03-15
    Interestingly they decided to keep one for public templates :\
  • Internal repos now use same icon as normal, but with organization avatar absolutely positioned near actual octicon (presumably, based on public discussion on octicons-v2 repo; the alternative is that there is unpublished v2 variant or organization icon is used along with avatar; would need to wait for GHE update)

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.
@CirnoT CirnoT mentioned this issue Jun 15, 2020
Merged
@silverwind
Copy link
Member

silverwind commented Jun 15, 2020 •
edited
Loading

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
Copy link
Member

silverwind commented Jun 15, 2020 •
edited
Loading

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
Copy link
Member

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
Copy link
Contributor Author

CirnoT commented Jun 15, 2020

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 CirnoT mentioned this issue Jun 15, 2020
Merged
@CirnoT
Copy link
Contributor Author

CirnoT commented Jun 15, 2020

We should be good to go for it after #11891 and #11895

@CirnoT
Copy link
Contributor Author

CirnoT commented Jun 15, 2020

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

@silverwind
Copy link
Member

silverwind commented Jun 15, 2020 •
edited
Loading

@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.

@lunny lunny added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Jun 21, 2020
@6543
Copy link
Member

6543 commented Jun 25, 2020

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

@CirnoT
Copy link
Contributor Author

CirnoT commented Jun 25, 2020 •
edited
Loading

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
Copy link
Member

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 added a commit to silverwind/gitea that referenced this issue Jul 13, 2020
@silverwind
Update Octicons to v10
a55e456 
Besides a few renames, these icons are no longer present in v10 that we've
used, so had to change:

file-symlink-directory -> file-submodule
internal-repo -> repo
repo-force-push -> repo-push
repo-template-private -> repo-template

Fixes: go-gitea#11889
Ref: https://github.com/primer/octicons/releases/tag/v10.0.0
@silverwind silverwind mentioned this issue Jul 13, 2020
Merged
@silverwind
Copy link
Member

#12240

lafriks pushed a commit that referenced this issue Jul 17, 2020
@silverwind @CirnoT @techknowlogick
Update Octicons to v10 (#12240)
0e24af6 
* Update Octicons to v10

Besides a few renames, these icons are no longer present in v10 that we've
used, so had to change:

file-symlink-directory -> file-submodule
internal-repo -> repo
repo-force-push -> repo-push
repo-template-private -> repo-template

Fixes: #11889
Ref: https://github.com/primer/octicons/releases/tag/v10.0.0

* add custom sliders svg for removed octicon-settings

* apply suggestion

* fix triangles and use play on admin dashboard

* add custom mirror svg

* add missing build files

* unify custom svgs

* move to octicon-repo-clone to gitea-mirror

* use octicon-x on conflicts

* tweak timeline icons

* tweak comment buttons

* update settings icon to octicons v1

* switch to octicon-mirror and octicon-tools

* replace two wiki buttons with octicons

* remove whitespace in svg sources

* Fix filepath basename on Windows for SVG bindata (#12241)

* move octicons to devDependencies

* move back to dependencies

* move svgo to devDependencies again

Co-authored-by: Cirno the Strongest <1447794+CirnoT@users.noreply.github.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
@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.
Assignees
No one assigned
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update Octicons to v10 silverwind/gitea
4 participants
@lunny @silverwind @CirnoT @6543