Skip to content

Change won't sign information text to black. #9843

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
Merged
merged 5 commits into from
Jan 18, 2020
Merged

Change won't sign information text to black. #9843

merged 5 commits into from
Jan 18, 2020

Conversation

davidsvantesson
Copy link
Contributor

Small patch to #9708.

When protected branch doesn't require signed commits, there should still be an information in the pull request if the commit will be signed. However I don't think it should be a warning, as it would be normal for persons/organizations that don't use commit signing. Therefore I suggest changing this text from yellow to black.

Screenshot

Before change:
bild

After change:
bild

@codecov-io
Copy link

codecov-io commented Jan 17, 2020 •
edited
Loading

Codecov Report

Merging #9843 into master will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9843      +/-   ##
==========================================
+ Coverage   42.31%   42.31%   +<.01%     
==========================================
  Files         604      604              
  Lines       79130    79137       +7     
==========================================
+ Hits        33481    33486       +5     
+ Misses      41526    41525       -1     
- Partials     4123     4126       +3
Impacted Files Coverage Δ
models/pull.go 42.82% <0%> (-0.61%) ⬇️
modules/process/manager.go 74.69% <0%> (-3.62%) ⬇️
services/pull/check.go 54.54% <0%> (-2.1%) ⬇️
models/gpg_key.go 55.03% <0%> (-0.56%) ⬇️
models/repo.go 47.14% <0%> (+0.12%) ⬆️
routers/repo/view.go 39.47% <0%> (+0.87%) ⬆️
models/unit.go 39.5% <0%> (+2.46%) ⬆️
services/pull/patch.go 69.81% <0%> (+3.77%) ⬆️

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 0e002fb...601f18c. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 17, 2020
@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 Jan 17, 2020
@sapk sapk added the topic/ui Change the appearance of the Gitea UI label Jan 17, 2020
@sapk sapk added this to the 1.12.0 milestone Jan 17, 2020
@sapk sapk added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Jan 17, 2020
silverwind
silverwind reviewed Jan 17, 2020
View reviewed changes
templates/repo/issue/view_content/pull.tmpl Outdated
@@ -152,7 +152,7 @@
{{$.i18n.Tr "repo.signing.will_sign" .SigningKey}}
</div>
{{else if .IsSigned}}
<div class="item text yellow">
<div class="item text black">
<i class="icon unlock grey"></i>
Copy link
Member

@silverwind silverwind Jan 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<i class="icon unlock grey"></i>
<i class="icon unlock"></i>

match icon to text color for consistency.

Copy link
Member

@silverwind silverwind Jan 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I did not change the whitespace in this suggestion, seems like a GitHub bug)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Black icon also in the warning case (when there is protection)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I just remove the color it will be yellow in case of warning
bild

Copy link
Member

@silverwind silverwind Jan 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me I suppose. Icon color matching text color is visually pleasing.

@guillep2k
Copy link
Member

What about dark mode?

davidsvantesson
davidsvantesson commented Jan 17, 2020
View reviewed changes
Copy link
Contributor Author

@davidsvantesson davidsvantesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After latest commit.
Normal mode:
bild
bild

Dark mode:
bild
bild

@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 Jan 17, 2020
@sapk sapk merged commit 0641965 into go-gitea:master Jan 18, 2020
@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.
Reviewers

@silverwind silverwind silverwind approved these changes

@lafriks lafriks lafriks approved these changes

@sapk sapk sapk approved these changes

@guillep2k guillep2k guillep2k approved these changes

@6543 6543 6543 approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.
9 participants
@davidsvantesson @codecov-io @guillep2k @silverwind @lafriks @sapk @6543 @GiteaBot @lunny