Skip to content

Update Octicons to v10 #12240

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

Merged
merged 20 commits into from
Jul 17, 2020
Merged

Update Octicons to v10 #12240

merged 20 commits into from
Jul 17, 2020

Conversation

silverwind
Copy link
Member

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

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 pull request Jul 13, 2020
@CirnoT
Copy link
Contributor

CirnoT commented Jul 13, 2020

repo-template-private -> repo-template
internal-repo -> repo

These two should be fine thanks to previous PRs that explicitly display repo visibility/type as label.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 13, 2020
@CirnoT
Copy link
Contributor

CirnoT commented Jul 13, 2020

Index: templates/repo/header.tmpl
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- templates/repo/header.tmpl	(revision d6c55b0ab0130fca7d93b0ef52b1414e990b4e0a)
+++ templates/repo/header.tmpl	(date 1594663288676)
@@ -50,8 +50,7 @@
 						{{$.CsrfTokenHtml}}
 						<div class="ui labeled button" tabindex="0">
 							<button type="submit" class="ui compact basic button">
-								<!-- TODO use star-filled once octicons v2 are in place */ -->
-								{{if $.IsStaringRepo}}{{svg "octicon-star" 16}}{{$.i18n.Tr "repo.unstar"}}{{else}}{{svg "octicon-star" 16}}{{$.i18n.Tr "repo.star"}}{{end}}
+								{{if $.IsStaringRepo}}{{svg "octicon-star-fill" 16}}{{$.i18n.Tr "repo.unstar"}}{{else}}{{svg "octicon-star" 16}}{{$.i18n.Tr "repo.star"}}{{end}}
 							</button>
 							<a class="ui basic label" href="{{.Link}}/stars">
 								{{.NumStars}}

@silverwind
Copy link
Member Author

Removed settings icon fixed:

image

@silverwind
Copy link
Member Author

Admin dashboard now uses play:

image

@CirnoT
Copy link
Contributor

CirnoT commented Jul 13, 2020

Aside from octicon-mirror you should also update octicon-clone, as they're now same icon https://github.com/chromium/chromium

@silverwind
Copy link
Member Author

You mean repo-clone? I guess I can add a copy gitea-repo-clone.

@CirnoT
Copy link
Contributor

CirnoT commented Jul 13, 2020

I think Octicons switched to single octicon-mirror icon (that is, repo-clone is removed) but mirror icon itself is missing from v10 for now so we don't know for sure. Based on GH usage however most likely we should just rename usages of octicon-repo-clone to octicon-mirror* as 99% in next version they'll remove clone and add mirror icon instead.

*- technically gitea-mirror

@silverwind
Copy link
Member Author

I'll just move out current usage of repo-clone to gitea-mirror. If they decide to add octicon-mirror it'll be an easy switch for us.

@silverwind
Copy link
Member Author

Done. I also changed to "New Migration" icon to repo-push which seemed more appropriate than the mirror icon:

image

@silverwind
Copy link
Member Author

Conflicts were also using this repo-clone icon which when turned into a mirror icon seemed unfitting. Changed them to octicon-x:

image

@silverwind
Copy link
Member Author

One more semi-related change. Timeline icons slightly adjusted and background of commit icon fixed:

image

@silverwind
Copy link
Member Author

Comment button ... replaced with octicon and slightly tweaked:

image

@CirnoT
Copy link
Contributor

CirnoT commented Jul 14, 2020

Actually mirror octicon is in set; https://primer.style/octicons/mirror-16. Very sorry for suggesting that it isn't. We can remove gitea-mirror in that case.

@CirnoT
Copy link
Contributor

CirnoT commented Jul 14, 2020

Can't we use https://primer.style/octicons/tools-16 or https://primer.style/octicons/gear-16 instead of v1 sliders for settings?

@lafriks lafriks added the topic/ui Change the appearance of the Gitea UI label Jul 14, 2020
@lafriks lafriks added this to the 1.13.0 milestone Jul 14, 2020
@silverwind
Copy link
Member Author

@CirnoT both done.

One more semi-related change: Replaced two icons on Wiki with octicons:

image

image

I think this is looking pretty good now overall.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 15, 2020
@lafriks
Copy link
Member

lafriks commented Jul 15, 2020

devDependencies are only for build tools, so it should be moved back to dependencies

@silverwind
Copy link
Member Author

I've moved both svgo and @primer/octicons to devDependencies because they are not used during the main build process (only during the make svg task), but I guess it's probably better to have them in dependencies.

@lafriks
Copy link
Member

lafriks commented Jul 15, 2020

svgo is used for building so it is devDependency but octicons are dependency

@silverwind
Copy link
Member Author

They are both used in the same script so they shoud be together. Let's keep them in dependencies. The primary reason I've moved them to devDeps is to avoid some unnecessary installs but it's no big deal.

@lafriks
Copy link
Member

lafriks commented Jul 15, 2020

@silverwind but not the same way. Svgo is used as a lib for build script while octicons are needed get svg files from so that's different uses

@silverwind
Copy link
Member Author

Done, I guess I don't really care :)

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 17, 2020
@lafriks
Copy link
Member

lafriks commented Jul 17, 2020

🚀

@lafriks lafriks merged commit 0e24af6 into go-gitea:master Jul 17, 2020
@silverwind silverwind deleted the octicons10 branch July 17, 2020 17:52
@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/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Octicons v2
5 participants