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

Fix migration svg color #16715

Merged
merged 3 commits into from Aug 18, 2021
Merged

Fix migration svg color #16715

merged 3 commits into from Aug 18, 2021

Conversation

KN4CK3R
Copy link
Member

@KN4CK3R KN4CK3R commented Aug 17, 2021

The SVGs on the migration page inherit the current text color which created wrong icons.

Before:
grafik

After:
grafik

Possible alternative: color: unset;
@silverwind I don't know what is better in this case. unset works nice with themes but is still the wrong color.

@techknowlogick techknowlogick added this to the 1.16.0 milestone Aug 18, 2021
@techknowlogick techknowlogick added the topic/ui Change the appearance of the Gitea UI label Aug 18, 2021
@silverwind
Copy link
Member

How does one get this icon to show? I don't see it on https://try.gitea.io/repo/migrate.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 18, 2021
@KN4CK3R
Copy link
Member Author

KN4CK3R commented Aug 18, 2021

It's in #16356. The original svg contains fill="#000000" but make svg strips the black fill because it is the default color.

@silverwind
Copy link
Member

I see two options given that this icon needs to change color on dark themes:

.migrate .card {
  color: var(--color-text);
}

or specifically override the icon to a different color:

svg.gitea-onedev {
  fill: var(--color-secondary-dark-11);
}

@KN4CK3R
Copy link
Member Author

KN4CK3R commented Aug 18, 2021

Does it really "need" to change color on dark themes? If the icon is black it should be black with all themes or not? We do not adjust the git icon on orange themes.
An alternative would be to use fill="#000001" in the icon which does not get stripped.

@silverwind
Copy link
Member

Does it really "need" to change color on dark themes

I prefer to change it because it gives more contrast, even if it's not a original color anymore.

An alternative would be to use fill="#1" in the icon which does not get stripped.

I'm sure there is a svgo plugin to disable somewhere.

@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 Aug 18, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #16715 (f10fd53) into main (274aeb3) will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #16715      +/-   ##
==========================================
- Coverage   45.40%   45.36%   -0.04%     
==========================================
  Files         758      758              
  Lines       85306    85381      +75     
==========================================
+ Hits        38732    38733       +1     
- Misses      40300    40366      +66     
- Partials     6274     6282       +8     
Impacted Files Coverage Δ
modules/queue/manager.go 65.34% <0.00%> (-2.85%) ⬇️
modules/queue/workerpool.go 53.05% <0.00%> (-2.68%) ⬇️
modules/log/file.go 73.60% <0.00%> (-1.61%) ⬇️
modules/migrations/gitea_uploader.go 6.77% <0.00%> (-0.63%) ⬇️
models/models.go 52.88% <0.00%> (-0.54%) ⬇️
routers/api/v1/repo/pull.go 29.11% <0.00%> (-0.52%) ⬇️
services/pull/pull.go 41.78% <0.00%> (-0.41%) ⬇️
modules/migrations/gitlab.go 0.99% <0.00%> (+<0.01%) ⬆️
modules/migrations/gogs.go 2.36% <0.00%> (+0.01%) ⬆️
modules/setting/setting.go 49.90% <0.00%> (+0.09%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 274aeb3...f10fd53. Read the comment docs.

@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 Aug 18, 2021
@lunny lunny merged commit 9f0c8f9 into go-gitea:main Aug 18, 2021
@KN4CK3R KN4CK3R deleted the fix-svg-color branch August 18, 2021 18:26
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
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.

None yet

7 participants