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

Jump to bottom

Improve user search display name #29002

Merged
merged 11 commits into from
Feb 1, 2024
Merged

Improve user search display name #29002

merged 11 commits into from
Feb 1, 2024

Conversation

KN4CK3R
Copy link
Member

@KN4CK3R KN4CK3R commented Jan 31, 2024

I tripped over this strange method and I don't think we need that workaround to fix the value.

old:
grafik

new:
grafik

@KN4CK3R KN4CK3R added the type/enhancement An improvement of existing functionality label Jan 31, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 31, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 31, 2024
delvh
delvh approved these changes Jan 31, 2024
View reviewed changes
web_src/js/features/repo-settings.js Outdated Show resolved Hide resolved
@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 Jan 31, 2024
@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 1, 2024
wxiaoguang
wxiaoguang approved these changes Feb 1, 2024
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment •
edited
Loading

Choose a reason for hiding this comment

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

XSS (approved by mistake, add a new change request)

Update: resolved.

image

image

wxiaoguang
wxiaoguang previously requested changes Feb 1, 2024
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment •
edited
Loading

Choose a reason for hiding this comment

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

XSS (feel free to dismiss this change request if the concern has been addressed)

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Feb 1, 2024
@KN4CK3R
Copy link
Member Author

KN4CK3R commented Feb 1, 2024

That's strange because Fomantic escapes the text...
https://github.com/fomantic/Fomantic-UI/blob/13b76ee714bba8efdeef8a6e402cda6774abce40/src/definitions/modules/search.js#L1493-L1498

@wxiaoguang
Copy link
Contributor

That's strange because Fomantic escapes the text... https://github.com/fomantic/Fomantic-UI/blob/13b76ee714bba8efdeef8a6e402cda6774abce40/src/definitions/modules/search.js#L1493-L1498

No. See preserveHTML

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Feb 1, 2024
@wxiaoguang
Copy link
Contributor

Maybe there are some nits. eg: the misalignment (when the item has "description") (since it has got lgtm/done, so this comment is just FYI, no blocker)

@silverwind silverwind self-requested a review February 1, 2024 13:17
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Feb 1, 2024
@silverwind
Copy link
Member

silverwind commented Feb 1, 2024 •
edited
Loading

Right, the avatar is misaligned on the item with description. Retracting review for now. Likely needs a CSS fix.

@KN4CK3R
Copy link
Member Author

KN4CK3R commented Feb 1, 2024

That's just the default style of Fomantic. Feel free to change whatever CSS is needed.

@silverwind
Copy link
Member

silverwind commented Feb 1, 2024 •
edited
Loading

I will take a look later. Likely our avatars are unsupported usage of fomantic that's causing the misalignment, so we need a CSS fix.

@silverwind
Copy link
Member

CSS fixes done:

image

@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 1, 2024
wxiaoguang
wxiaoguang reviewed Feb 1, 2024
View reviewed changes
web_src/css/repo.css Outdated Show resolved Hide resolved
silverwind
silverwind reviewed Feb 1, 2024
View reviewed changes
web_src/css/repo.css Outdated Show resolved Hide resolved
wxiaoguang
wxiaoguang reviewed Feb 1, 2024
View reviewed changes
web_src/css/repo.css Outdated Show resolved Hide resolved
silverwind
silverwind reviewed Feb 1, 2024
View reviewed changes
web_src/css/repo.css Outdated Show resolved Hide resolved
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 1, 2024
@silverwind silverwind enabled auto-merge (squash) February 1, 2024 16:46
@silverwind silverwind merged commit c3e4629 into go-gitea:main Feb 1, 2024
25 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Feb 1, 2024
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 1, 2024
@KN4CK3R
Copy link
Member Author

KN4CK3R commented Feb 1, 2024

Thank you for adjusting the styles. 👍

@KN4CK3R KN4CK3R deleted the improve-search-displayname branch February 1, 2024 17:16
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 2, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
098f110 
* giteaofficial/main:
  Wrap contained tags and branches again (go-gitea#29021)
  Avoid sending update/delete release notice when it is draft (go-gitea#29008)
  Fix incorrect button CSS usages (go-gitea#29015)
  Strip trailing newline in markdown code copy (go-gitea#29019)
  Improve user search display name (go-gitea#29002)
silverwind added a commit to silverwind/gitea that referenced this pull request Feb 20, 2024
@KN4CK3R @silverwind @wxiaoguang
Improve user search display name (go-gitea#29002)
1657bfd 
I tripped over this strange method and I don't think we need that
workaround to fix the value.

old:

![grafik](https://github.com/go-gitea/gitea/assets/1666336/c8b6797b-eb45-4dec-99db-1b0649a34ec5)

new:

![grafik](https://github.com/go-gitea/gitea/assets/1666336/ab1a65ae-de5b-4ce4-9813-3b8b39c7922e)

---------

Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@delvh delvh delvh approved these changes

@silverwind silverwind silverwind approved these changes

@wxiaoguang wxiaoguang wxiaoguang approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.
5 participants
@KN4CK3R @wxiaoguang @silverwind @delvh @GiteaBot