Skip to content

Pin Repositories on user page (Fixes #10375) #19831

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

Closed
wants to merge 43 commits into from
Closed
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
e3fe53f
Add checks for pinned repositories
May 28, 2022
fa46bfd
Working pin and unpin in repository.
May 28, 2022
11faab0
Merge branch 'go-gitea:main' into feature/Pinning
Eekle May 28, 2022
9f87fd6
Fixes unpin typo
Eekle May 29, 2022
5c22473
Simplify canPin check
Eekle May 29, 2022
8464a09
Remove disabled pin button for anons
Eekle May 29, 2022
0140a49
Refactor based on advice in PR and implement cards
May 29, 2022
30a2076
Working and styled carded pinned repos
May 29, 2022
cf5ef19
Revert weird permissions changes
May 29, 2022
33c5db9
Set correct initial pinnedRepo capacity
Eekle May 29, 2022
a0f3148
Move CanPin to services
May 29, 2022
bfc0a3b
Merge branch 'feature/Pinning' of https://github.com/Eekle/gitea into…
May 29, 2022
b6d4403
Pin/Unpin takes batch args. Max limit imposed.
May 29, 2022
0cfd468
Adds better error reporting when too many repos are pinned.
May 29, 2022
ef80b11
Add signin check to pin button
Eekle May 30, 2022
ca30e0c
Tidier empty array code
Eekle May 30, 2022
4e2e304
Merge branch 'go-gitea:main' into feature/Pinning
Eekle May 30, 2022
d7cc870
Linter pleasing
Eekle May 30, 2022
2e94d4e
Linter pleasing
Eekle May 30, 2022
65c1c26
Copyright typo
Eekle May 30, 2022
3bcfaa2
Better logs for pinning repos without permission
Eekle May 30, 2022
f0e8a25
Topics and metas at bottom of pin card
Eekle May 30, 2022
8be963a
Move pinned_repos.tmpl to shared. Reverts whitespace issues in home.tmpl
Eekle May 30, 2022
f489c68
Remove Pinned option in repo search and associated IsPinned check
Eekle May 30, 2022
6cbdb44
Pinned repos on user profiles
Eekle May 30, 2022
0d11561
Add docs to pin funcs
Eekle May 31, 2022
94396a6
make fmt
Eekle May 31, 2022
3ee1ade
Backend lint fixes
Eekle May 31, 2022
22f2956
Document GetPinnedRepositoryIDs
Eekle May 31, 2022
332c8fa
Merge branch 'main' into feature/Pinning
Eekle Jun 1, 2022
9eeb53b
Add tests for pinning
Jun 1, 2022
16a9906
Make fmt
Eekle Jun 1, 2022
5655553
Pin button correctly rounded and unpin svg
Eekle Jun 2, 2022
d8d7a40
Fix svg formatting
Eekle Jun 2, 2022
70274d5
Fix svg issues with frontend check
Eekle Jun 2, 2022
791122a
Merge branch 'main' into feature/Pinning
Eekle Jun 2, 2022
3e566e0
Change pin access to org owners, not repo admins
Eekle Jun 3, 2022
6e65d34
Merge branch 'feature/Pinning' of github.com:Eekle/gitea into feature…
Eekle Jun 3, 2022
0e50a7c
Fix backend linting
Eekle Jun 3, 2022
58f5754
Fix admin pin permission
Eekle Jun 3, 2022
cb10529
Typo in canPin
Eekle Jun 3, 2022
826d921
Merge branch 'main' into feature/Pinning
Eekle Jun 14, 2022
b2a7270
Apply suggestions from code review - Delvh
Eekle Oct 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions models/repo/repo.go
Original file line number Diff line number Diff line change
@@ -217,6 +217,20 @@ func (repo *Repository) IsBroken() bool {
return repo.Status == RepositoryBroken
}

// IsPinned indicates that repository is pinned
func (repo *Repository) IsPinned() bool {
Copy link
Member

Choose a reason for hiding this comment

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

I think the function should be IsPinned(userID int64). One could pin another public repository?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A repo is either pinned to its owner's profile, or not at all.

This is maybe the same confusion I had with @delvh ? This is not an implementation for pinning any repo you have access to to your profile - it's an implementation for pinning a repo to its owner's profile.

Copy link

@ell1e ell1e Oct 19, 2022

Choose a reason for hiding this comment

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

Hm, the most interesting thing I happen to work on is in an org I own, not in my user namespace. So I couldn't pin that then? I'd assume that's a common use case, but maybe that's just me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It sounds like that's everyone's else's view too. This may need reimplementing with that in mind.

pinned, err := user_model.GetPinnedRepositoryIDs(repo.OwnerID)
if err != nil {
return false
}
for _, r := range pinned {
if r == repo.ID {
return true
}
}
return false
}

// AfterLoad is invoked from XORM after setting the values of all fields of this object.
func (repo *Repository) AfterLoad() {
// FIXME: use models migration to solve all at once.
105 changes: 105 additions & 0 deletions models/user/pin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// Copyright 2022 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package user

import (
"fmt"

"code.gitea.io/gitea/modules/json"
)

const maxPinnedRepos = 3

// GetPinnedRepositoryIDs returns all the repository IDs pinned by the given user or org.
// If they've never pinned a repository, an empty array is returned.
func GetPinnedRepositoryIDs(ownerID int64) ([]int64, error) {
pinnedstring, err := GetUserSetting(ownerID, PinnedRepositories)
if err != nil {
return nil, err
}

var parsedValues []int64
if pinnedstring == "" {
return parsedValues, nil
}

err = json.Unmarshal([]byte(pinnedstring), &parsedValues)

if err != nil {
return nil, err
}

return parsedValues, nil
}

func setPinnedRepositories(ownerID int64, repos []int64) error {
stringed, err := json.Marshal(repos)
if err != nil {
return err
}

return SetUserSetting(ownerID, PinnedRepositories, string(stringed))
}

type TooManyPinnedReposError struct {
count int
}

func (e *TooManyPinnedReposError) Error() string {
return fmt.Sprintf("can pin at most %d repositories, %d pinned repositories is too much", maxPinnedRepos, e.count)
}

// PinRepos pin the specified repos for the given user or organization.
// The caller must ensure all repos belong to the owner.
func PinRepos(ownerID int64, repoIDs ...int64) error {
repos, err := GetPinnedRepositoryIDs(ownerID)
if err != nil {
return err
}
newrepos := make([]int64, 0, len(repoIDs)+len(repos))

repos = append(repos, repoIDs...)

for _, toadd := range repos {
alreadypresent := false
for _, present := range newrepos {
if toadd == present {
alreadypresent = true
break
}
}
if !alreadypresent {
newrepos = append(newrepos, toadd)
}
}
if len(newrepos) > maxPinnedRepos {
return &TooManyPinnedReposError{count: len(newrepos)}
}
return setPinnedRepositories(ownerID, newrepos)
}

// UnpinRepos unpin the given repositories for the given user or organization
func UnpinRepos(ownerID int64, repoIDs ...int64) error {
prevRepos, err := GetPinnedRepositoryIDs(ownerID)
if err != nil {
return err
}
var nextRepos []int64

for _, r := range prevRepos {
keep := true
for _, unp := range repoIDs {
if r == unp {
keep = false
break
}
}
if keep {
nextRepos = append(nextRepos, r)
}
}

return setPinnedRepositories(ownerID, nextRepos)
}
37 changes: 37 additions & 0 deletions models/user/pin_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2022 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package user

import (
"testing"

"code.gitea.io/gitea/models/unittest"

"github.com/stretchr/testify/assert"
)

func TestPinAndUnpinRepos(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
// User:2 pins repositories 1 and 2
userID, repoID1, repoID2 := 2, 1, 2
{
assert.NoError(t, PinRepos(userID, repoID1, repoID2))
pinned, err := GetPinnedRepositoryIDs(userID)

assert.NoError(t, err)
expected := []int64{repoID1, repoID2}
assert.Equal(t, pinned, expected)
}
// User 2 unpins repository 2, leaving just 1
{
assert.NoError(t, UnpinRepos(userID, repoID2))

pinned, err := GetPinnedRepositoryIDs(userID)

assert.NoError(t, err)
expected := []int64{repoID1}
assert.Equal(t, pinned, expected)
}
}
1 change: 1 addition & 0 deletions models/user/setting_keys.go
Original file line number Diff line number Diff line change
@@ -9,4 +9,5 @@ const (
SettingsKeyHiddenCommentTypes = "issue.hidden_comment_types"
// SettingsKeyDiffWhitespaceBehavior is the setting key for whitespace behavior of diff
SettingsKeyDiffWhitespaceBehavior = "diff.whitespace_behaviour"
PinnedRepositories = "pinned_repos"
)
1 change: 1 addition & 0 deletions public/img/svg/octicon-custom-pin-off.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
36 changes: 36 additions & 0 deletions routers/web/org/home.go
Original file line number Diff line number Diff line change
@@ -10,9 +10,12 @@ import (

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
access "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/markup"
"code.gitea.io/gitea/modules/markup/markdown"
"code.gitea.io/gitea/modules/setting"
@@ -149,8 +152,41 @@ func Home(ctx *context.Context) {
return
}

pinnedRepoIDs, err := user_model.GetPinnedRepositoryIDs(org.ID)
if err != nil {
ctx.ServerError("GetPinnedRepositoryIDs", err)
return
}
pinnedRepos := make([]*repo_model.Repository, 0, len(pinnedRepoIDs))
for _, id := range pinnedRepoIDs {
repo, err := repo_model.GetRepositoryByID(id)
if err != nil {
ctx.ServerError("GetRepositoryByID", err)
return
}
if err = repo.LoadAttributes(ctx); err != nil {
ctx.ServerError("LoadAttributes", err)
return
}
if repo.OwnerID != org.ID {
log.Warn("Ignoring pinned repo %v because it's not owned by %v", repo.FullName(), org.Name)
} else {
perm, err := access.GetUserRepoPermission(ctx, repo, ctx.Doer)
if err != nil {
ctx.ServerError("GetUserRepoPermission", err)
return
}
if !perm.HasAccess() {
log.Info("Ignoring pinned repo %v because user %v has no access to it.", repo.FullName(), ctx.Doer)
} else {
pinnedRepos = append(pinnedRepos, repo)
}
}
}

ctx.Data["Owner"] = org
ctx.Data["Repos"] = repos
ctx.Data["PinnedRepos"] = pinnedRepos
ctx.Data["Total"] = count
ctx.Data["MembersTotal"] = membersCount
ctx.Data["Members"] = members
121 changes: 121 additions & 0 deletions routers/web/repo/pin_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// Copyright 2022 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package repo

import (
"testing"

"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/test"

"github.com/stretchr/testify/assert"
)

const (
pin = true
unpin = false
)

func TestUserPinUnpin(t *testing.T) {
unittest.PrepareTestEnv(t)
// These test cases run sequentially since they modify state
testcases := []struct {
uid int64
rid int64
action bool
endstate bool
failmesssage string
}{
{
uid: 2,
rid: 2,
action: pin,
endstate: pin,
failmesssage: "user cannot pin repos they own",
},
{
uid: 2,
rid: 2,
action: unpin,
endstate: unpin,
failmesssage: "user cannot unpin repos they own",
},

{
uid: 2,
rid: 5,
action: pin,
endstate: pin,
failmesssage: "user cannot pin repos they have admin access to",
},
{
uid: 2,
rid: 5,
action: unpin,
endstate: unpin,
failmesssage: "user cannot unpin repos they have admin access to",
},

{
uid: 2,
rid: 4,
action: pin,
endstate: unpin,
failmesssage: "user can pin repos they don't have access to",
},

{
uid: 5,
rid: 4,
action: pin,
endstate: pin,
failmesssage: "user cannot pin repos they own (this should never fail)",
},
{
uid: 2,
rid: 4,
action: unpin,
endstate: pin,
failmesssage: "user can unpin repos they don't have access to",
},
{
uid: 1,
rid: 4,
action: unpin,
endstate: unpin,
failmesssage: "admin can't unpin repos",
},
{
uid: 1,
rid: 4,
action: pin,
endstate: pin,
failmesssage: "admin can't pin repos",
},
}

for _, c := range testcases {
ctx := test.MockContext(t, "")
test.LoadUser(t, ctx, c.uid)
test.LoadRepo(t, ctx, c.rid)

switch c.action {
case pin:
ctx.SetParams(":action", "pin")
case unpin:
ctx.SetParams(":action", "unpin")
}

Action(ctx)
ispinned := getRepository(ctx, c.rid).IsPinned()

assert.Equal(t, ispinned, c.endstate, c.failmesssage)

if c.endstate != ispinned {
// We have to stop at first failure, state won't be coherent afterwards.
return
}
}
}
17 changes: 17 additions & 0 deletions routers/web/repo/repo.go
Original file line number Diff line number Diff line change
@@ -32,6 +32,7 @@ import (
"code.gitea.io/gitea/services/forms"
repo_service "code.gitea.io/gitea/services/repository"
archiver_service "code.gitea.io/gitea/services/repository/archiver"
user_service "code.gitea.io/gitea/services/user"
)

const (
@@ -292,6 +293,22 @@ func Action(ctx *context.Context) {
err = repo_model.StarRepo(ctx.Doer.ID, ctx.Repo.Repository.ID, true)
case "unstar":
err = repo_model.StarRepo(ctx.Doer.ID, ctx.Repo.Repository.ID, false)
case "pin":
if user_service.CanPin(ctx, ctx.Doer, ctx.Repo.Repository) {
err = user_model.PinRepos(ctx.Repo.Owner.ID, ctx.Repo.Repository.ID)
if _, ok := err.(*user_model.TooManyPinnedReposError); ok {
ctx.Error(http.StatusBadRequest, err.Error())
return
}
} else {
err = errors.New("user does not have permission to pin")
}
case "unpin":
if user_service.CanPin(ctx, ctx.Doer, ctx.Repo.Repository) {
err = user_model.UnpinRepos(ctx.Repo.Owner.ID, ctx.Repo.Repository.ID)
} else {
err = errors.New("user does not have permission to unpin")
}
case "accept_transfer":
err = acceptOrRejectRepoTransfer(ctx, true)
case "reject_transfer":
2 changes: 2 additions & 0 deletions routers/web/repo/view.go
Original file line number Diff line number Diff line change
@@ -41,6 +41,7 @@ import (
"code.gitea.io/gitea/modules/typesniffer"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/routers/web/feed"
user_service "code.gitea.io/gitea/services/user"
)

const (
@@ -733,6 +734,7 @@ func Home(ctx *context.Context) {
}

ctx.Data["FeedURL"] = ctx.Repo.Repository.HTMLURL()
ctx.Data["CanPinRepos"] = ctx.IsSigned && user_service.CanPin(ctx, ctx.Doer, ctx.Repo.Repository)

checkHomeCodeViewable(ctx)
if ctx.Written() {
37 changes: 36 additions & 1 deletion routers/web/user/profile.go
Original file line number Diff line number Diff line change
@@ -13,10 +13,12 @@ import (
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
access "code.gitea.io/gitea/models/perm/access"
project_model "code.gitea.io/gitea/models/project"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/markup"
"code.gitea.io/gitea/modules/markup/markdown"
"code.gitea.io/gitea/modules/setting"
@@ -271,9 +273,42 @@ func Profile(ctx *context.Context) {

total = int(count)
}

pinnedRepoIDs, err := user_model.GetPinnedRepositoryIDs(ctx.ContextUser.ID)
if err != nil {
ctx.ServerError("GetPinnedRepositoryIDs", err)
return
}
pinnedRepos := make([]*repo_model.Repository, 0, len(pinnedRepoIDs))
for _, id := range pinnedRepoIDs {
repo, err := repo_model.GetRepositoryByID(id)
if err != nil {
ctx.ServerError("GetRepositoryByID", err)
return
}
if err = repo.LoadAttributes(ctx); err != nil {
ctx.ServerError("LoadAttributes", err)
return
}
if repo.OwnerID != ctx.ContextUser.ID {
log.Warn("Ignoring pinned repo %v because it's not owned by %v", repo.FullName(), ctx.ContextUser.Name)
} else {
perm, err := access.GetUserRepoPermission(ctx, repo, ctx.Doer)
if err != nil {
ctx.ServerError("GetUserRepoPermission", err)
return
}
if !perm.HasAccess() {
log.Info("Ignoring pinned repo %v because user %v has no access to it.", repo.FullName(), ctx.Doer)
} else {
pinnedRepos = append(pinnedRepos, repo)
}
}
}
Comment on lines +277 to +307
Copy link
Member

Choose a reason for hiding this comment

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

Instead of copying the code between user profiles and org home pages, it might make sense to add a common method.


ctx.Data["Repos"] = repos
ctx.Data["Total"] = total

ctx.Data["PinnedRepos"] = pinnedRepos
pager := context.NewPagination(total, setting.UI.User.RepoPagingNum, page, 5)
pager.SetDefaultParams(ctx)
if tab != "followers" && tab != "following" && tab != "activity" && tab != "projects" {
24 changes: 24 additions & 0 deletions services/user/user.go
Original file line number Diff line number Diff line change
@@ -18,6 +18,8 @@ import (
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
packages_model "code.gitea.io/gitea/models/packages"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/avatar"
@@ -181,3 +183,25 @@ func DeleteAvatar(u *user_model.User) error {
}
return nil
}

// Verify that a user has permission to pin/unpin a repository
func CanPin(ctx context.Context, u *user_model.User, r *repo_model.Repository) bool {
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused: Is r the repo to pin?
Because if that's the case, then the logic looks wrong to me:
Why would you require admin rights for the repo to pin it to your page?
That doesn't make sense, read rights should suffice.
But you need admin rights on your own page/ the org you want to pin to.
So, to me, the preconditions for pinning should instead be:

  1. If you're not signed in, then it's always disallowed
  2. If you're an admin, then it's always allowed
  3. You have read rights on the repo to pin
  4. (Optional, I'm unsure if we should implement it: The repo must belong to the org/ user you're trying to pin to. However, I can also see it making sense to exclude that precondition, i.e. when you want to link to a foreign repo explicitly)
  5. if you are trying to pin a repo to your own pinned repos, you have no further preconditions as you already have admin rights for your own repos
  6. if you are trying to pin a repo to the pinned repos of an organization you're a member of, you need to be in a team with admin rights in that organization

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your optional point 4 was what I intended - when a repository is pinned, it is pinned to its owner. So if a user U attempts to pin a repo R owned by org O, then R is added to O's list of pinned repositories - not U's.

So the intended logic is:

  1. If you're not signed in, then it's always disallowed
  2. If you're an admin, then it's always allowed
  3. If the repo belongs to an organisation, you must be a member of a team with admin rights in that organisation to pin it
  4. Otherwise, you must have administrative access to the repository (presumably, you own it)

Copy link
Member

Choose a reason for hiding this comment

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

Well, the problem I see with your approach:
Assume I add user B as admin for repo A.
B can now pin A (to his own page, which might even make sense as he most likely works a lot with A if he has admin rights) and pins A.
I take away Bs admin rights.
B still has A pinned - how should that be handled?
Do we remove A from Bs pinned repos, do we keep A, or do we disallow removing B as admin?
The best solution is then to remove it from his pins as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohhhhh we're talking about user pages, for which you've given someone else admin access? The intention was that pages could only be pinned to their owner's profile, whether that's a user or org. So B pinning a repo owned by A to B's profile would not be possible.

if u.IsAdmin {
return true
}
if r.Owner.IsOrganization() {
org := organization.OrgFromUser(r.Owner)
teams, err := org.GetUserTeams(u.ID)
if err != nil {
return false
}
for _, team := range teams {
if team.AccessMode >= perm.AccessModeAdmin {
return true
}
}
return false
}
perm, err := access_model.GetUserRepoPermission(ctx, r, u)
return err == nil && perm.IsAdmin()
}
9 changes: 8 additions & 1 deletion templates/org/home.tmpl
Original file line number Diff line number Diff line change
@@ -18,7 +18,14 @@
</div>
</div>
</div>

{{if .PinnedRepos}}
<div class="ui container">
<div class="ui divider"></div>
<div id="pinned_repos" class="item">
{{template "shared/pinned_repos_threewide" .}}
</div>
</div>
{{end}}
{{template "org/menu" .}}

<div class="ui container">
10 changes: 10 additions & 0 deletions templates/repo/header.tmpl
Original file line number Diff line number Diff line change
@@ -64,6 +64,16 @@
</div>
</form>
{{end}}
{{if $.CanPin}}
<form method="post" action="{{$.RepoLink}}/action/{{if .IsPinned}}un{{end}}pin?redirect_to={{$.Link}}">
{{$.CsrfTokenHtml}}
<div tabindex="0">
<button type="submit" class="ui compact small basic button">
{{if .IsPinned}}{{svg "octicon-custom-pin-off" 16}}Unpin{{else}}{{svg "octicon-pin"}}Pin{{end}}
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't these button texts be translated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep good point

</button>
</div>
</form>
{{end}}
<form method="post" action="{{$.RepoLink}}/action/{{if $.IsWatchingRepo}}un{{end}}watch?redirect_to={{$.Link}}">
{{$.CsrfTokenHtml}}
<div class="ui labeled button{{if not $.IsSigned}} tooltip{{end}}" tabindex="0"{{if not $.IsSigned}} data-content="{{$.i18n.Tr "repo.watch_guest_user" }}" data-position="top center"{{end}}>
70 changes: 70 additions & 0 deletions templates/shared/pinned_repos_cards.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
{{range .PinnedRepos}}
<div class="ui card">
<div class="content">
<div class="header">
<div class="repo-title">
{{$avatar := (repoAvatar . 32 "mr-3")}}
{{if $avatar}}
{{$avatar}}
{{end}}
<a class="name" href="{{.Link}}">
{{if or $.PageIsExplore $.PageIsProfileStarList }}{{if .Owner}}{{.Owner.Name}} / {{end}}{{end}}{{.Name}}
</a>
<div class="labels df ac fw">
{{if .IsArchived}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.archived"}}</span>
{{end}}
{{if .IsTemplate}}
{{if .IsPrivate}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.private_template"}}</span>
{{else}}
{{if .Owner.Visibility.IsPrivate}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.internal_template"}}</span>
{{end}}
{{end}}
{{else}}
{{if .IsPrivate}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.private"}}</span>
{{else}}
{{if .Owner.Visibility.IsPrivate}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.internal"}}</span>
{{end}}
{{end}}
{{end}}
{{if .IsFork}}
{{svg "octicon-repo-forked"}}
{{else if .IsMirror}}
{{svg "octicon-mirror"}}
{{end}}
</div>
</div>
</div>
<div class="description">
{{ $description := .DescriptionHTML $.Context}}
{{if $description}}<p>{{$description}}</p>{{end}}
{{if .Topics }}
<div class="ui tags">
{{range .Topics}}
{{if ne . "" }}
<a href="{{AppSubUrl}}/explore/repos?q={{.}}&topic=1">
<div class="ui small label topic">{{.}}</div>
</a>
{{end}}
{{end}}
</div>
{{end}}
</div>
</div>
<div class="extra content">
<div class="metas df ac">
{{if .PrimaryLanguage }}
<span class="text grey df ac mr-3"><i class="color-icon mr-3" style="background-color: {{.PrimaryLanguage.Color}}"></i>{{ .PrimaryLanguage.Language }}</span>
{{end}}
{{if not $.DisableStars}}
<span class="text grey df ac mr-3">{{svg "octicon-star" 16 "mr-3"}}{{.NumStars}}</span>
{{end}}
<span class="text grey df ac mr-3">{{svg "octicon-git-branch" 16 "mr-3"}}{{.NumForks}}</span>
</div>
</div>
</div>
{{end}}
3 changes: 3 additions & 0 deletions templates/shared/pinned_repos_onewide.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div class="ui one cards">
{{template "shared/pinned_repos_cards" .}}
</div>
3 changes: 3 additions & 0 deletions templates/shared/pinned_repos_threewide.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div class="ui three stackable cards">
{{template "shared/pinned_repos_cards" .}}
</div>
7 changes: 6 additions & 1 deletion templates/user/profile.tmpl
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@
<div class="ui container">
<div class="ui stackable grid">
<div class="ui five wide column">
<div class="ui card">
<div id="profile-card" class="ui card">
<div id="profile-avatar" class="content df"/>
{{if eq .SignedUserName .Owner.Name}}
<a class="image tooltip" href="{{AppSubUrl}}/user/settings" data-content="{{.i18n.Tr "user.change_avatar"}}" data-position="bottom center">
@@ -89,6 +89,11 @@
</div>
</div>
<div class="ui eleven wide column">
{{if .PinnedRepos}}
<div id="pinned_repos" class="item">
Copy link
Member

Choose a reason for hiding this comment

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

  1. I don't quite understand why you use IDs so freely here… Won't that simply make problems? Why not a class instead?
  2. please be consistent. Why should we suddenly have IDs with underscores?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Inexperience in this sort of work. I was just following the example of the profile-avatar div that is also in this file. Happy to change it if required - would appreciate if you could help me understand why :)
  2. Fair

Copy link
Member

Choose a reason for hiding this comment

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

See for example #21230, which was caused exactly by having duplicate IDs without noticing it.
See also https://ultimatecourses.com/blog/html-class-id:

Avoid styling id elements, prefer a class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha. Easy enough fix. Does that mean that the instance of profile-avatar is also an issue that needs fixing?

Copy link
Member

Choose a reason for hiding this comment

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

Yes.
However, I'm also fine with postponing that for another PR.
Unless you want to fix it right now…

{{template "shared/pinned_repos_onewide" .}}
</div>
{{end}}
<div class="ui secondary stackable pointing tight menu">
<a class='{{if and (ne .TabName "activity") (ne .TabName "following") (ne .TabName "followers") (ne .TabName "stars") (ne .TabName "watching") (ne .TabName "projects")}}active{{end}} item' href="{{.Owner.HomeLink}}">
{{svg "octicon-repo"}} {{.i18n.Tr "user.repositories"}}
2 changes: 1 addition & 1 deletion web_src/less/_user.less
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@

.user {
&.profile {
.ui.card {
#profile-card.ui.card {
Copy link
Member

Choose a reason for hiding this comment

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

I don't exactly understand why this selector was restricted... Why couldn't it stay as it was before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's been a while so I'm not certain I'm remembering right - but once pinned repos are added to the profile, there is more than one ui card element in the page.

So I imagine this selector is here to specifically style the user profile card, and make sure it never interferes with the pin cards?

.header {
display: block;
font-weight: 600;
1 change: 1 addition & 0 deletions web_src/svg/octicon-custom-pin-off.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.