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

[UI] change review status icons on pr view style to github style #10737

Merged
merged 4 commits into from
Apr 3, 2020

Conversation

a1012112796
Copy link
Member

@a1012112796 a1012112796 commented Mar 16, 2020

I think the github style is more meaningfull, so make this chang

  • change the icon of ApproveReview pr from "eye" to "check" like github
  • change the icon of RejectReview pr from "x" to "request-change" like github

view:

jta

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 16, 2020
@silverwind
Copy link
Member

Got some before/after screenshots?

@lunny lunny added the topic/ui Change the appearance of the Gitea UI label Mar 17, 2020
@lunny
Copy link
Member

lunny commented Mar 17, 2020

Screenshots are welcome.

@a1012112796
Copy link
Member Author

a1012112796 commented Mar 17, 2020

Sorry , I forgot it.
This change have two main part

  • first , change icon of approve from eye to check and change the icon for Reject from x to request-change. because I think the github style is more meaningfull.
    the compare view is bellow
    now used:
    jtd
    changed to:
    jtf

after this change , I found that these icons include "eye" , "check" , "lock" and "key" can't bound at the timeline like the "primitive-dot", but the github can, but I don't know how to do it, so just add a additional "primitive-dot" to make these item onto the time-line

Remove it , because It's not a perfect solution.

@a1012112796 a1012112796 changed the title [UI] change icons on pr view style to github style [UI] change review status icons on pr view style to github style Mar 17, 2020
@codecov-io
Copy link

codecov-io commented Mar 17, 2020

Codecov Report

Merging #10737 into master will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10737      +/-   ##
==========================================
- Coverage   43.50%   43.46%   -0.04%     
==========================================
  Files         597      597              
  Lines       83916    83916              
==========================================
- Hits        36504    36472      -32     
- Misses      42903    42932      +29     
- Partials     4509     4512       +3     
Impacted Files Coverage Δ
models/review.go 42.75% <100.00%> (ø)
modules/indexer/stats/queue.go 62.50% <0.00%> (-18.75%) ⬇️
modules/indexer/stats/db.go 40.62% <0.00%> (-9.38%) ⬇️
modules/git/repo_language_stats.go 66.17% <0.00%> (-4.42%) ⬇️
services/pull/check.go 50.00% <0.00%> (-3.66%) ⬇️
modules/git/command.go 86.95% <0.00%> (-2.61%) ⬇️
services/pull/patch.go 61.93% <0.00%> (-2.59%) ⬇️
services/pull/temp_repo.go 29.05% <0.00%> (-2.57%) ⬇️
models/unit.go 41.97% <0.00%> (-2.47%) ⬇️
modules/queue/workerpool.go 56.93% <0.00%> (-1.07%) ⬇️
... and 4 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 3723b06...6b146f5. Read the comment docs.

@a1012112796
Copy link
Member Author

Hello, Are these changes usefull?Looking forward to replys , Thanks .

@silverwind
Copy link
Member

silverwind commented Mar 18, 2020

New icons seem reasonable. I wonder if there is a better solution for this though:

image

Is the example just a neutral review or a negative one? If neutral, the icon should be grey ideally. If you can't get the icon into the time line, maybe just indicate intent of the review in the colored dot (red,green,grey) and remove the icon.

@lunny
Copy link
Member

lunny commented Mar 19, 2020

I like the new icons. Thanks! @a1012112796

* change the icon of ApproveReview pr from "eye" to "check" like github
* change the icon of RejectReview pr from "x" to "request-change" like github
* add "-" after "{{" which need to be one line (TODO: may be not change all)

Signed-off-by: a1012112796 <[email protected]>
@a1012112796
Copy link
Member Author

a1012112796 commented Mar 20, 2020

now, I just change the icons style . for the problem about some items can't bound on the time-line, left it and wait other contributors who can solve it

@a1012112796
Copy link
Member Author

Hello, How about these changes now? Looking forward to replys , Thanks .

@lunny lunny added this to the 1.12.0 milestone Mar 24, 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 Mar 25, 2020
@silverwind
Copy link
Member

for the problem about some items can't bound on the time-line

I can't seem to figure it out either. Can't see any style that would cause it, it's weird.

@guillep2k
Copy link
Member

@a1012112796 could you please update the screenshots?

@a1012112796
Copy link
Member Author

@guillep2k hello, these screenshots are same as now, all not right screenshots which are add by me has been deleted.
#10737 (comment)
#10737 (comment)

@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 Mar 27, 2020
@a1012112796
Copy link
Member Author

a1012112796 commented Apr 2, 2020

I wander the status of this PR now ...

@guillep2k guillep2k merged commit f685edf into go-gitea:master Apr 3, 2020
a1012112796 added a commit to a1012112796/gitea that referenced this pull request Apr 23, 2020
* as title, do same changs on action view with go-gitea#10737
* chage default icon from "invalid type" to "question" , because  "invalid type" is not a meaningfull icon type

Signed-off-by: a1012112796 <[email protected]>
guillep2k pushed a commit that referenced this pull request Apr 24, 2020
* as title, do same changs on action view with #10737
* chage default icon from "invalid type" to "question" , because  "invalid type" is not a meaningfull icon type

Signed-off-by: a1012112796 <[email protected]>
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
* as title, do same changs on action view with go-gitea#10737
* chage default icon from "invalid type" to "question" , because  "invalid type" is not a meaningfull icon type

Signed-off-by: a1012112796 <[email protected]>
@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.

None yet

7 participants