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

Add wrapper to author to avoid long name ui problem #23030

Merged
merged 15 commits into from
Feb 23, 2023

Conversation

HesterG
Copy link
Contributor

@HesterG HesterG commented Feb 21, 2023

Close #22866. Main change is to add a author-wrapper class around author name, like the wrapper added to message. The max-width is set to 200px on PC, and 100px on mobile device for now. Which will work like below:

截屏2023-02-21 11 57 53

截屏2023-02-21 11 58 43

And title is added to the wrapper like it did in message wrapper. So the full author name will show on hover.

@@ -8,14 +8,14 @@
{{if .LatestCommitUser}}
{{avatar $.Context .LatestCommitUser 24}}
{{if .LatestCommitUser.FullName}}
<a class="muted" href="{{.LatestCommitUser.HomeLink}}"><strong>{{.LatestCommitUser.FullName}}</strong></a>
<span class="author-wrapper" title="{{.LatestCommitUser.FullName}}"><a class="muted" href="{{.LatestCommitUser.HomeLink}}"><strong>{{.LatestCommitUser.FullName}}</strong></a></span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not put these styles and title on <a> directly?

Copy link
Contributor Author

@HesterG HesterG Feb 21, 2023

Choose a reason for hiding this comment

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

Why not put these styles and title on <a> directly?

Yes that is better putting them directly on <a> tag here. Changed that.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 21, 2023
@lunny lunny added type/bug outdated/backport/v1.18 This PR should be backported to Gitea 1.18 skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Feb 21, 2023
@lunny lunny added this to the 1.19.0 milestone Feb 21, 2023
@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 Feb 21, 2023
@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 Feb 21, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 21, 2023
@Maks1mS
Copy link
Contributor

Maks1mS commented Feb 21, 2023

It doesn't seem to fix the problem (сheck in other languages).
image

@wxiaoguang
Copy link
Contributor

I think the answer would be "flex". These max-width: styles don't work well for such layout.

@lunny lunny removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 21, 2023
@HesterG
Copy link
Contributor Author

HesterG commented Feb 22, 2023

I think the answer would be "flex". These max-width: styles don't work well for such layout.

Flex could fix it, but might need more code changes like overriding of padding introduced by fomantic <th> and background change, etc. Used table-layout:fixed and reduced max-width for names for now.

@HesterG
Copy link
Contributor Author

HesterG commented Feb 22, 2023

It doesn't seem to fix the problem (сheck in other languages). image

Hi, tried another fix, please check again.

@Maks1mS
Copy link
Contributor

Maks1mS commented Feb 22, 2023

It doesn't seem to fix the problem (сheck in other languages). image

Hi, tried another fix, please check again.

I think 525 px is the best option (including commit signing, CI status etc.)
image

@Maks1mS
Copy link
Contributor

Maks1mS commented Feb 22, 2023

Also need to change in other media queries. I will write the optimal values ​​in the near future.

@Maks1mS
Copy link
Contributor

Maks1mS commented Feb 22, 2023

diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less
index e5f879e9e..5604386ba 100644
--- a/web_src/less/_repository.less
+++ b/web_src/less/_repository.less
@@ -2936,7 +2936,7 @@ tbody.commit-list {
   }
 
   th .message-wrapper {
-    max-width: 280px;
+    max-width: 120px;
   }
 }
 
@@ -2946,7 +2946,7 @@ tbody.commit-list {
   }
 
   th .message-wrapper {
-    max-width: 490px;
+    max-width: 350px;
   }
 }
 
@@ -2956,7 +2956,7 @@ tbody.commit-list {
   }
 
   th .message-wrapper {
-    max-width: 650px;
+    max-width: 525px;
   }
 }

Maybe not perfect, but I think it's a good option without using flex and a lot of code changes.

@HesterG
Copy link
Contributor Author

HesterG commented Feb 22, 2023

diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less
index e5f879e9e..5604386ba 100644
--- a/web_src/less/_repository.less
+++ b/web_src/less/_repository.less
@@ -2936,7 +2936,7 @@ tbody.commit-list {
   }
 
   th .message-wrapper {
-    max-width: 280px;
+    max-width: 120px;
   }
 }
 
@@ -2946,7 +2946,7 @@ tbody.commit-list {
   }
 
   th .message-wrapper {
-    max-width: 490px;
+    max-width: 350px;
   }
 }
 
@@ -2956,7 +2956,7 @@ tbody.commit-list {
   }
 
   th .message-wrapper {
-    max-width: 650px;
+    max-width: 525px;
   }
 }

Maybe not perfect, but I think it's a good option without using flex and a lot of code changes.

added a commit to use these values @Maks1mS, please check.

@yardenshoham yardenshoham modified the milestones: 1.19.0, 1.20.0 Feb 22, 2023
@yardenshoham yardenshoham added the outdated/backport/v1.19 This PR should be backported to Gitea 1.19 label Feb 22, 2023
@HesterG
Copy link
Contributor Author

HesterG commented Feb 23, 2023

This is how the long name and long description look like on different size of screen:

截屏2023-02-23 16 01 47

截屏2023-02-23 16 01 59

截屏2023-02-23 16 02 06

截屏2023-02-23 16 02 24

Please feel free to check. And if no more change is required, i think this PR is ready to merge.

@wolfogre wolfogre added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 23, 2023
@jolheiser
Copy link
Member

🎺 🤖

@jolheiser jolheiser merged commit 8ed6096 into go-gitea:main Feb 23, 2023
@jolheiser jolheiser removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 23, 2023
yardenshoham pushed a commit to yardenshoham/gitea that referenced this pull request Feb 23, 2023
This PR is a possible solution for issue go-gitea#22866. Main change is to add a
`author-wrapper` class around author name, like the wrapper added to
message. The `max-width` is set to 200px on PC, and 100px on mobile
device for now.
lunny added a commit that referenced this pull request Feb 24, 2023
Backport #23030

This PR is a possible solution for issue #22866. Main change is to add a
`author-wrapper` class around author name, like the wrapper added to
message. The `max-width` is set to 200px on PC, and 100px on mobile
device for now. Which will work like below:

<img width="1183" alt="2023-02-21 11 57 53"
src="https://user-images.githubusercontent.com/17645053/220244146-3d47c512-33b6-4ed8-938e-de0a8bc26ffb.png">

<img width="417" alt="2023-02-21 11 58 43"
src="https://user-images.githubusercontent.com/17645053/220244154-1ea0476b-9d1c-473a-9917-d3216860f9a9.png">

And `title` is added to the wrapper like it did in message wrapper. So
the full author name will show on hover.

Co-authored-by: HesterG <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 24, 2023
* giteaofficial/main:
  Make issue meta dropdown support Enter, confirm before reloading (go-gitea#23014)
  Fix SyncOnCommit always return false in API of push_mirrors (go-gitea#23088)
  Fix commit name in Apply Patch page (go-gitea#23086)
  Add wrapper to author to avoid long name ui problem (go-gitea#23030)
  Avoid Hugo from adding quote to actions url (go-gitea#23097)
  Remove all package data after tests (go-gitea#22984)
  Change style to improve whitespaces trimming inside inline markdown code  (go-gitea#23093)
  Nest metadata in refactoring docs (go-gitea#23087)

# Conflicts:
#	templates/repo/issue/view_content/context_menu.tmpl
@yardenshoham
Copy link
Member

Please send backport to v1.18

@wolfogre wolfogre added the backport/done All backports for this PR have been created label Feb 27, 2023
lunny pushed a commit that referenced this pull request Feb 27, 2023
Backport #23030 

This PR is a possible solution for issue #22866. Main change is to add a
`author-wrapper` class around author name, like the wrapper added to
message. The `max-width` is set to 200px on PC, and 100px on mobile
device for now. Which will work like below:

<img width="1183" alt="2023-02-21 11 57 53"
src="https://user-images.githubusercontent.com/17645053/220244146-3d47c512-33b6-4ed8-938e-de0a8bc26ffb.png">

<img width="417" alt="2023-02-21 11 58 43"
src="https://user-images.githubusercontent.com/17645053/220244154-1ea0476b-9d1c-473a-9917-d3216860f9a9.png">

And `title` is added to the wrapper like it did in message wrapper. So
the full author name will show on hover.
@HesterG HesterG deleted the long-name-maxwidth-issue22866 branch March 1, 2023 06:28
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. outdated/backport/v1.18 This PR should be backported to Gitea 1.18 outdated/backport/v1.19 This PR should be backported to Gitea 1.19 skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Long full name/username causes repo view to overflow
9 participants