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

Revamp repo header #27760

Merged
merged 12 commits into from
Jan 12, 2024
Merged

Revamp repo header #27760

merged 12 commits into from
Jan 12, 2024

Conversation

denyskon
Copy link
Member

@denyskon denyskon commented Oct 23, 2023

Redesign repo header with following new aspects:

  • responsive & better-looking repo title
  • hide repo button text instead of icons in mobile view
  • use same tab style as on explore and org page
Before:

grafik
grafik
grafik
grafik

After:

grafik
grafik
grafik
grafik

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 23, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 23, 2023
@denyskon denyskon changed the title revamp repo header Revamp repo header Oct 23, 2023
@denyskon denyskon added type/enhancement An improvement of existing functionality topic/ui Change the appearance of the Gitea UI type/refactoring Existing code has been cleaned up. There should be no new functionality. modifies/templates This PR modifies the template files labels Oct 23, 2023
@denyskon denyskon added this to the 1.22.0 milestone Oct 23, 2023
@github-actions github-actions bot removed the modifies/templates This PR modifies the template files label Oct 23, 2023
@silverwind
Copy link
Member

silverwind commented Oct 23, 2023

I still prefer the user / repo rendering with bigger font than this two-line thing. It pictures what is in the URL and it's also what GitHub retained in their last header redesign. We should likely mute the links.

@denyskon
Copy link
Member Author

Well if others think the same I can bring it back, but then we'd need a way for it to no longer look horrible on mobile if the repo name is even a bit longer :)

@JakobDev
Copy link
Contributor

show owner displayname instead of username

I'm not a fan of this. The Username is used for all technical purposes (repo url, git clone etc.), s we should show the username instead of the displayname.

@puni9869
Copy link
Member

puni9869 commented Oct 24, 2023

I am biased with
image

horizontal scroller.
Could you make it like github ... for mobile approach if the row items are not accomodated in row we will shift them to ...
image

There is some view port math here, calculate the length of overall row view port and then calculate the length of every item, if sum of all view row item > view port then hide the last one and push in the ... and use the % approach for more item push and pop. It is little bit tricker in JS.

And I agree with @JakobDev point we should use the username. Not displayed name.

For Private label you can show the lock icon in mobile view. this will use less space.
image

IMO desktop view looks good in gitea, we need to focus on mobile view and Accessibility more.

@denyskon
Copy link
Member Author

Alright, so I brought back the one-line-view and showing username instead of display name. Opinions welcome!

grafik
grafik
grafik
grafik
grafik

@puni9869
Copy link
Member

This hurts my soul for mobile view. Since you are an UI expert, could you do some magic here.🪄
image

@denyskon
Copy link
Member Author

@puni9869

grafik

What do you think?

@puni9869
Copy link
Member

😃 Much better, just the template icon is should be standard, Add a tooltip if applicable in mobile view.

Overall looks good.

@denyskon
Copy link
Member Author

This is the standard template icon:

grafik

Tooltip added:

grafik

@silverwind
Copy link
Member

Could you make it like github ... for mobile approach if the row items are not accomodated in row we will shift them to ...

Yes we should do this but I think the implementation will be complex as it can not be done in CSS and it needs JS with an ResizeObserver to dynamically populate the dropdown. I would defer this to another PR.

@denyskon
Copy link
Member Author

I agree that we should do it at some point, but to be honest, my JS skills are not good enough to create something that would be maintainable 😆

@mpeter50
Copy link
Contributor

Hi!
I have noticed that the empty space around objects have also increased. I think with the exception of the 2nd "before" image, the margins were better the way it was originally.

Relatively a lot of software projects have adopted huge margins, but I don't think thats a good change. Screens are larger to fit more information nicely, not to have larger empty spaces, and at the same time, not everyone has large screens.
I agree that too small margins would make it a mess, but I don't think that was the case (with the exception of the 2nd "before" image, because of the wrapped button line).

@denyskon
Copy link
Member Author

@mpeter50

I also am a fan of small margins as long as you can still visually separate elements without thinking about it. For me, the sheer amount of different elements in that header justifies the big margins, but I can try to play with it a bit and maybe I'll find a better combination

@lafriks
Copy link
Member

lafriks commented Nov 18, 2023

I really dislike that background in repo name

@denyskon
Copy link
Member Author

@lafriks I thought it would be a good highlight on hover, but I can remove it if others think the same

@lafriks
Copy link
Member

lafriks commented Nov 18, 2023

Personally it makes it harder to read because of less contrast and the UI too crowded :)

@denyskon
Copy link
Member Author

@mpeter50 I reduced the paddings and updated the screenshots.

@lafriks I removed that hover effect

@denyskon
Copy link
Member Author

Some more reviews would be nice :) Are there any things left that should be changed?

@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 30, 2023
@delvh
Copy link
Member

delvh commented Jan 11, 2024

ping for reviewers.

@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 Jan 12, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jan 12, 2024
@lunny lunny enabled auto-merge (squash) January 12, 2024 03:08
@lunny lunny merged commit 7d62615 into go-gitea:main Jan 12, 2024
25 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jan 12, 2024
@denyskon denyskon deleted the ui/revamp-repo-header branch January 12, 2024 04:45
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 11, 2024
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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants