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 special repo state if avatar is set #7387

Closed
wants to merge 2 commits into from

Conversation

Cherrg
Copy link
Contributor

@Cherrg Cherrg commented Jul 8, 2019

add icons like on repo list

suggested in, fix #7384

#Images

grafik

add lock icon lige in repo list

suggested in, fix go-gitea#7384

Signed-off-by: Michael Gnehr <michael@gnehr.de>
@@ -11,6 +11,7 @@
<a href="{{AppSubUrl}}/{{.Owner.Name}}">{{.Owner.Name}}</a>
<div class="divider"> / </div>
<a href="{{$.RepoLink}}">{{.Name}}</a>
{{if and .RelAvatarLink .IsPrivate}}<i class="text gold octicon octicon-lock"></i>{{end}}
Copy link
Member

Choose a reason for hiding this comment

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

Two things I'm wondering...

  1. Should it always be on the right side, rather than only if there's an avatar? Then it would be consistent.
    a. Alternatively, should the icons just always show up on the left, next to the avatar if it exists?

  2. An icon should show regardless of whether it's private or not, similar to how it would look with no avatar. At least in my opinion.

Suggested change
{{if and .RelAvatarLink .IsPrivate}}<i class="text gold octicon octicon-lock"></i>{{end}}
{{if not .RelAvatarLink}}<i class="text gold octicon octicon-{{if .IsPrivate}}lock{{else if .IsMirror}}repo-clone{{else if .IsFork}}repo-forked{{else}}repo{{end}}"></i>{{end}}

Copy link
Contributor Author

@Cherrg Cherrg Jul 9, 2019

Choose a reason for hiding this comment

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

My opinion

  • icon position:
    • I would place the small ones always on right side. So text will not jump around if there is something special on repository (special: private,mirror,fork,archived)
  • I would only show the small icons, when it is hidden by avatar.
    When no avatar is set then there would be a second icon with the same information
  • furthermore i would't show a symbol on right side if it is a normal repository
    • so you see with a short glance if it is a normal repo or not (i know, with no avatar the normal repo icon is shown, but the size is bigger.)
  • you're right i missed the other special icons, thanks for the hint. But the first condition in your proposal isn't correct. Your proposal would only be rendered if avatar is not set.
    -> I will add the other icons there

Copy link
Member

Choose a reason for hiding this comment

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

Ah, good catch on my suggestion.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 8, 2019
@Cherrg Cherrg changed the title show repo lock icon on private repo header with avatar show special repo state if avatar is set Jul 9, 2019
add missing icons

Signed-off-by: Michael Gnehr <michael@gnehr.de>
@codecov-io
Copy link

Codecov Report

Merging #7387 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7387      +/-   ##
==========================================
- Coverage   41.19%   41.18%   -0.01%     
==========================================
  Files         469      469              
  Lines       63536    63536              
==========================================
- Hits        26172    26167       -5     
- Misses      33944    33948       +4     
- Partials     3420     3421       +1
Impacted Files Coverage Δ
modules/process/manager.go 76.81% <0%> (-4.35%) ⬇️
models/repo_list.go 72.08% <0%> (-1.02%) ⬇️

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 0622e83...9d390e5. Read the comment docs.

@lunny lunny added the topic/ui Change the appearance of the Gitea UI label Jul 9, 2019
@lunny lunny added this to the 1.10.0 milestone Jul 9, 2019
@lunny
Copy link
Member

lunny commented Jul 9, 2019

When REPOSITORY_AVATAR_FALLBACK=none then it's not consist between repository list and repository view.

image

image

@Cherrg
Copy link
Contributor Author

Cherrg commented Jul 9, 2019

@lunny

  • are configuration/setting flags accessible on templates, or do I need to set some key in ctx.Data manually?
  • suggestion: - if REPOSITORY_AVATAR_FALLBACK=none
    • hide left icon
    • show right one (if special case)

@mrsdizzie
Copy link
Member

  • are configuration/setting flags accessible on templates, or do I need to set some key in ctx.Data manually?

You could add a helper to this file if needed:

https://github.com/go-gitea/gitea/blob/master/modules/templates/helper.go

@lunny
Copy link
Member

lunny commented Jul 9, 2019

How about always put lock icon right of the repositories' name on repository view page?

@lafriks
Copy link
Member

lafriks commented Jul 9, 2019

I agree with @lunny about placing it always right as it is easier and more consistent

@silverwind
Copy link
Member

Would also suggest increasing the size of right-side icons a bit and try to vertically center them with the text.

@stale
Copy link

stale bot commented Sep 10, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Sep 10, 2019
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Sep 12, 2019
@stale stale bot removed the issue/stale label Sep 12, 2019
@lunny lunny closed this Sep 14, 2019
@lunny lunny removed this from the 1.10.0 milestone Sep 14, 2019
@Cherrg Cherrg deleted the issue_7384 branch November 15, 2019 23:15
@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
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't see if repository is private
8 participants