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

Show dropdown with all statuses for commit #13977

Merged
merged 19 commits into from
Dec 20, 2020
Merged

Conversation

CirnoT
Copy link
Contributor

@CirnoT CirnoT commented Dec 14, 2020

  • Fetch all statuses for commit
  • Create element with list of statuses for commit
  • Style the dropdown element
  • Abstract dropdown element to template
  • Use dropdown element in all places where commit status icon is shown

chrome_2020-12-17_23-59-08

chrome_2020-12-17_23-59-24

@codecov-io
Copy link

codecov-io commented Dec 14, 2020

Codecov Report

Merging #13977 (cc4868d) into master (36bd5d7) will decrease coverage by 0.00%.
The diff coverage is 68.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13977      +/-   ##
==========================================
- Coverage   42.32%   42.32%   -0.01%     
==========================================
  Files         726      726              
  Lines       77679    77689      +10     
==========================================
+ Hits        32876    32879       +3     
- Misses      39405    39410       +5     
- Partials     5398     5400       +2     
Impacted Files Coverage Δ
routers/repo/blame.go 0.00% <0.00%> (ø)
routers/repo/commit.go 29.38% <0.00%> (-0.12%) ⬇️
modules/setting/service.go 81.81% <50.00%> (-2.50%) ⬇️
routers/user/auth.go 12.04% <50.00%> (ø)
models/commit_status.go 63.08% <100.00%> (+0.24%) ⬆️
routers/repo/issue.go 38.14% <100.00%> (+0.03%) ⬆️
routers/repo/view.go 38.24% <100.00%> (+0.13%) ⬆️
routers/user/home.go 60.67% <100.00%> (+0.07%) ⬆️
services/pull/pull.go 41.37% <100.00%> (ø)
modules/queue/unique_queue_disk_channel.go 53.84% <0.00%> (-1.54%) ⬇️
... and 1 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 029836c...cc4868d. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 14, 2020
@a1012112796 a1012112796 added the topic/ui Change the appearance of the Gitea UI label Dec 14, 2020
@zeripath
Copy link
Contributor

this looks good to me

@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 Dec 14, 2020
@silverwind
Copy link
Member

silverwind commented Dec 14, 2020

Not strictly required but I think this would be a good opportunity to switch the font-awesome circle for https://primer.style/octicons/dot-fill-16.

@CirnoT CirnoT marked this pull request as ready for review December 17, 2020 22:17
@silverwind
Copy link
Member

Can you remove this strange inner border on the popup?

image

@silverwind
Copy link
Member

silverwind commented Dec 17, 2020

Maybe don't use a segment but a (divided) list:

https://fomantic-ui.com/elements/list.html#list

I guess some manual styling will be inevitable thought, as always.

@CirnoT
Copy link
Contributor Author

CirnoT commented Dec 17, 2020

@silverwind Check now

@6543 6543 added the type/enhancement An improvement of existing functionality label Dec 17, 2020
@silverwind
Copy link
Member

Please add this for the divider border:

diff --git a/web_src/less/_base.less b/web_src/less/_base.less
index 8847a5e3f..369a9543c 100644
--- a/web_src/less/_base.less
+++ b/web_src/less/_base.less
@@ -528,8 +528,12 @@ a.ui.card:hover,
 .ui.card .avatar img {
   border-radius: var(--border-radius);
 }

+.ui.divided.list > .item {
+  border-color: var(--color-secondary);
+}

 .dont-break-out {
   overflow-wrap: break-word;
   word-wrap: break-word;
   word-break: break-all;

@silverwind
Copy link
Member

silverwind commented Dec 17, 2020

Looks good but we also need to add dropdowns to the other places where those icons appear like on the PR list. This one is non-interactive currently:

image

@CirnoT
Copy link
Contributor Author

CirnoT commented Dec 18, 2020

we also need to add dropdowns to the other places where those icons appear like on the PR list

The purpose here is to show statuses for specific commit, in case of PR you should always open PR page anyway and check the latest statuses from there. I don't see much point in presenting that information outside of PR page really.

@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 Dec 18, 2020
@silverwind
Copy link
Member

The purpose here is to show statuses for specific commit, in case of PR you should always open PR page anyway and check the latest statuses from there. I don't see much point in presenting that information outside of PR page really.

Maybe just make that icon a link to the PR, possibly with a tooltip like GH has?

image

@CirnoT
Copy link
Contributor Author

CirnoT commented Dec 18, 2020

Made icon be part of a link and fixed hardcoded translation string that I've missed.

@silverwind
Copy link
Member

silverwind commented Dec 18, 2020

Unrelated but I think we really need to make .i18n.tr a proper template helper, it's horrendous having to pass it to every sub-template like that.

@techknowlogick techknowlogick added this to the 1.14.0 milestone Dec 19, 2020
@lunny lunny merged commit f3c4baa into go-gitea:master Dec 20, 2020
@CirnoT CirnoT deleted the commit-statuses branch December 20, 2020 05:08
@jpraet jpraet mentioned this pull request Jan 13, 2021
7 tasks
@go-gitea go-gitea locked and limited conversation to collaborators Feb 11, 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 type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants