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.

Sign up for GitHub

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

Fix avatar misalignment #22955

Merged
merged 6 commits into from
Feb 19, 2023
Merged

Fix avatar misalignment #22955

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9f0ea4c
fix misalignments
Zettat123 Feb 17, 2023
441a768
lint code
Zettat123 Feb 17, 2023
63b69e3
use gt-ml-2 class
Zettat123 Feb 18, 2023
e1ceb01
use helpers selectors
Zettat123 Feb 18, 2023
9b529ce
use flex in user item
Zettat123 Feb 18, 2023
5ce3e13
Merge branch 'main' into bugfix/issue_22818
lunny Feb 19, 2023
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
2 changes: 1 addition & 1 deletion templates/repo/issue/view_content/pull.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
{{avatar $.Context .User}}
</a>
{{end}}
<span>
<span class="reviewer-info">
Zettat123 marked this conversation as resolved.
Show resolved Hide resolved
{{if .User}}
<a href="{{.User.HomeLink}}">{{.User.GetDisplayName}}</a>
{{else if .Team}}
Expand Down
6 changes: 6 additions & 0 deletions web_src/less/_base.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,12 @@ a.commit-statuses-trigger {
color: var(--color-text);
}

.ui.list > .item > img.ui.avatar + .content {
display: inline-block;
vertical-align: top;
padding-left: .5em;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

This selector seems too complex. If there is a certain place using the avatar, do the helper CSS classes help?

Copy link
Member

Choose a reason for hiding this comment

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

If it is a original Fomantic selector, I'm happy to keep it, even if complex. It'll ensure compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually it is not an original Fomantic selector, but there is a similar selector and I replaced .image with img.ui.avatar.

Copy link
Contributor

Choose a reason for hiding this comment

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

Which page and which elements will this style be applied to?

Copy link
Contributor

@wxiaoguang wxiaoguang Feb 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

According to my test, this style is applied to the explore org page? If so, you could just add a new .content selector beside img.ui.avatar in _explore.less, no need to introduce a new complex CSS selector.

OK, maybe not that complex, add gt-dib to the content in HTML is simple enough?

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 will be applied in the user list of /explore/users page.

Copy link
Contributor

@wxiaoguang wxiaoguang Feb 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is good enough:

diff --git a/web_src/less/_explore.less b/web_src/less/_explore.less
index 5a41f8dc5..5caf21d41 100644
--- a/web_src/less/_explore.less
+++ b/web_src/less/_explore.less
@@ -69,6 +69,7 @@
 .ui.user.list {
   .item {
     padding-bottom: 25px;
+    display: flex;
 
     &:not(:first-child) {
       border-top: 1px solid var(--color-secondary);
@@ -78,6 +79,7 @@
     img.ui.avatar {
       width: 40px;
       height: 40px;
+      margin-right: 10px;
     }
 
     .description {

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is good enough:

diff --git a/web_src/less/_explore.less b/web_src/less/_explore.less
index 5a41f8dc5..5caf21d41 100644
--- a/web_src/less/_explore.less
+++ b/web_src/less/_explore.less
@@ -69,6 +69,7 @@
 .ui.user.list {
   .item {
     padding-bottom: 25px;
+    display: flex;
 
     &:not(:first-child) {
       border-top: 1px solid var(--color-secondary);
@@ -78,6 +79,7 @@
     img.ui.avatar {
       width: 40px;
       height: 40px;
+      margin-right: 10px;
     }
 
     .description {

Good idea. Thank you very much!

.ui.list .list > .item .description,
.ui.list > .item .description {
color: var(--color-text);
Expand Down
2 changes: 2 additions & 0 deletions web_src/less/_explore.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@
img.ui.avatar {
width: 40px;
height: 40px;
display: inline-block;
vertical-align: top;
}

.description {
Expand Down
4 changes: 4 additions & 0 deletions web_src/less/_repository.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,10 @@
align-items: center;
}

.review-item-left .reviewer-info {
margin-left: .25em;
}

.text {
margin: .3em 0 .5em .5em;
}
Expand Down