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

Conversation

Eekle
Copy link
Contributor

@Eekle Eekle commented May 28, 2022

#10375

This PR adds the ability for repository administrators to pin up to 3 repositories to the home page of the repository owner.

This applies to both users and orgs.

Pinned repositories are stored in the user setting pinned_repos.

image

image
image
image

Not included in this PR:

  • Any API access to pinning/unpinning
  • Configurable maximum pinned repos per org/user

@Eekle Eekle marked this pull request as draft May 28, 2022 19:29
"code.gitea.io/gitea/modules/log"
)

//Pin Repo or unpin repo
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//Pin Repo or unpin repo
// PinRepo or unpin repo

err = json.Unmarshal([]byte(pinstring), &pinitems)

if err != nil {
log.Warn("Couldn't deserialise pinned repos: %v", pinstring)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
log.Warn("Couldn't deserialise pinned repos: %v", pinstring)
log.Warn("Cannot deserialize pinned repos: %v", err)

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 28, 2022
@Eekle
Copy link
Contributor Author

Eekle commented May 29, 2022

@delvh All the bits of logging here are debugging hangovers, they won't be present in the finished PR :)

Thank you for the notes! I'll go through it all today.

@lunny
Copy link
Member

lunny commented May 29, 2022

Could you give some screenshots about the UI design? I think we need some discuss there.

@Eekle
Copy link
Contributor Author

Eekle commented May 29, 2022

Could you give some screenshots about the UI design? I think we need some discuss there.

image

I tried cards above the search bar, but couldn't get it to look coherent.

Part of it is that the repository names can be very long, right? So I found it hard to think of a better presentation than in a list with the rest.

However I take @delvh 's point that these are a user thing and should appear separately to the searched repositories. So I will try again with some cards.

@delvh
Copy link
Member

delvh commented May 29, 2022

I'd say if the name is too long for a card, we can simply overflow,
i.e. really-long-repository-name could be shortened to really-lo..., depending on the width of the card.
That should be easy to achieve with CSS.


EDIT: I just found out that GitHub handles long repository names as follows:
image
I don't know, do we want to keep that approach, or do we want to use overflow instead?

@Eekle
Copy link
Contributor Author

Eekle commented May 29, 2022

image

Moved to a card approach. I know it's not elegant yet but just as a centre for discussion.

@Eekle
Copy link
Contributor Author

Eekle commented May 29, 2022

image

Latest UI grab

Eekle and others added 2 commits May 30, 2022 10:26
Co-authored-by: delvh <[email protected]>
Co-authored-by: delvh <[email protected]>
@ell1e
Copy link

ell1e commented Oct 15, 2022

I'm curious since none of the test screenshots are anything close to a phone in portrait mode, but what happens with the pinned repos with a small page width? It would be cool if they then became vertically stacked or something else smart that doesn't involve just a giant horizontal scrollbar (and doesn't involve the pin cards becoming so tiny they can't be read anymore).

In overall this is really cool. Especially on instances with a lot of developers, this can help a lot with finding out what cool stuff the others made.

@Eekle
Copy link
Contributor Author

Eekle commented Oct 15, 2022

I'm curious since none of the test screenshots are anything close to a phone in portrait mode, but what happens with the pinned repos with a small page width? It would be cool if they then became vertically stacked or something else smart that doesn't involve just a giant horizontal scrollbar (and doesn't involve the pin cards becoming so tiny they can't be read anymore).

In overall this is really cool. Especially on instances with a lot of developers, this can help a lot with finding out what cool stuff the others made.

I believe stacking vertically is exactly what they do. They use the Fomantic stackable class.

@@ -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.

@lunny
Copy link
Member

lunny commented Oct 24, 2022

Moved to v1.19 because one should pin his organization repositories

@lunny lunny modified the milestones: 1.18.0, 1.19.0 Oct 24, 2022
@Eekle
Copy link
Contributor Author

Eekle commented Oct 24, 2022

I may not have the time to implement this new version any time soon. If someone else is interested I'm happy to hand it over.

So to be clear the spec here is:

  • Users and orgs should be able to have up to 3 repositories pinned on their profiles
  • For organisations, this must be repositories within that organisation
  • For users, it can be any public repository (no restrictions?)

It is not super clear to me what the UI for this will be. For example if I'm on a repository owned by an org for which I'm the administrator, I need the option to pin it to my profile or the org's profile. I don't know how best to do that.

@ell1e
Copy link

ell1e commented Oct 24, 2022

For users, it can be any public repository (no restrictions?)

For what it's worth, I think GitHub restricts it to having either some sort of repo ownership or past contributions.

I'm really just a random user (who happens to like pins), but I personally think no restriction may actually be cooler. It changes pins from only "look at my work" to "hey, look at this cool thing", and that feels more social and dynamic to me. But I wonder what others think?

@delvh
Copy link
Member

delvh commented Oct 25, 2022

Both have their advantages and disadvantages.
The owner-based approach prevents errors, the freely-pinnable approach is less restrictive.

I'm also fine with both approaches, as long as we can agree on one and keep it at that.

@Eekle
Copy link
Contributor Author

Eekle commented Oct 26, 2022

Both have their advantages and disadvantages. The owner-based approach prevents errors, the freely-pinnable approach is less restrictive.

I'm also fine with both approaches, as long as we can agree on one and keep it at that.

I think free pinning gives users the opportunity to use the feature how they wish, which maybe feels more expressive?

Plus it's simpler to implement!

@eeyrjmr
Copy link
Contributor

eeyrjmr commented Oct 26, 2022

Both have their advantages and disadvantages. The owner-based approach prevents errors, the freely-pinnable approach is less restrictive.
I'm also fine with both approaches, as long as we can agree on one and keep it at that.

I think free pinning gives users the opportunity to use the feature how they wish, which maybe feels more expressive?

Plus it's simpler to implement!

While that is true think about an organisation that wants to bring focus to a key repository. Users pinning for their convenience is important but an organisation pinning for visibility is equally important

eg https://github.com/go-gitea

@Eekle
Copy link
Contributor Author

Eekle commented Oct 27, 2022

Both have their advantages and disadvantages. The owner-based approach prevents errors, the freely-pinnable approach is less restrictive.
I'm also fine with both approaches, as long as we can agree on one and keep it at that.

I think free pinning gives users the opportunity to use the feature how they wish, which maybe feels more expressive?
Plus it's simpler to implement!

While that is true think about an organisation that wants to bring focus to a key repository. Users pinning for their convenience is important but an organisation pinning for visibility is equally important

eg https://github.com/go-gitea

I don't understand why users being able to free pin would impact orgs?

Orgs will be able to pin their own repos to their profiles for sure.

@delvh
Copy link
Member

delvh commented Oct 27, 2022

I am equally confused.
The only thing I can somewhat guess would be that you need a dropdown when pinning to decide where to pin it to, with the options being you, or the orgs you have admin rights in. If you don't have admin rights in any org, we should probably shortcut to directly pinning to your own page.

@Eekle
Copy link
Contributor Author

Eekle commented Oct 27, 2022

I am equally confused. The only thing I can somewhat guess would be that you need a dropdown when pinning to decide where to pin it to, with the options being you, or the orgs you have admin rights in. If you don't have admin rights in any org, we should probably shortcut to directly pinning to your own page.

Hold on a mo.

It seems like you're suggesting that orgs should also be able to pin any repository to their profiles - not just ones tha they own?

I thought we were suggesting users could pin anything, but orgs could only pin their own repos.

@ell1e
Copy link

ell1e commented Oct 27, 2022

but orgs could only pin their own repos.

Would that be important to limit though? Maybe it should also be unrestricted.

@Eekle
Copy link
Contributor Author

Eekle commented Oct 27, 2022

but orgs could only pin their own repos.

Would that be important to limit though? Maybe it should also be unrestricted.

Yeah that's a fair question. I suppose I'm worried about the UI explosion.

If the dropdown is just:

  • Pin to my profile
  • Pin to [orgname]

Then that's pretty digestable. If it's...

  • Pin to my profile
  • Pin to [org1]
  • Pin to [org2]
  • Pin to [org3]
  • ...

It's a lot more irritating, especially given that 99% of the time you're going to want to pin the repo just to its owning org.

@delvh
Copy link
Member

delvh commented Oct 27, 2022

Please move the discussion to #10375, I think that's more appropriate as we have a lot to discuss as it seems.

@jolheiser jolheiser modified the milestones: 1.19.0, 1.20.0 Jan 31, 2023
@silverwind silverwind changed the title Pin Repositories (Fixes #10375) Pin Repositories on user page (Fixes #10375) May 30, 2023
@silverwind
Copy link
Member

Now that #24406 is in, I assume at least the HTML templates and CSS from it could be re-used here.

@delvh delvh removed this from the 1.20.0 milestone Jun 4, 2023
@hiifong
Copy link
Member

hiifong commented Jul 21, 2023

Are you still doing this PR?

@denyskon
Copy link
Member

Closing as stale and the author having said that this feature needs a completely new implementation (#10375 (comment))

@denyskon denyskon closed this Sep 10, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Dec 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.