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

Fix dropdown content overflow #31610

Merged
merged 11 commits into from
Oct 13, 2024
Merged

Fix dropdown content overflow #31610

merged 11 commits into from
Oct 13, 2024

Conversation

charles7668
Copy link
Contributor

close #31602

圖片

@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 11, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 11, 2024
@techknowlogick techknowlogick added type/bug topic/ui Change the appearance of the Gitea UI labels Jul 11, 2024
@6543 6543 added the backport/v1.22 This PR should be backported to Gitea 1.22 label Jul 11, 2024
@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 Jul 11, 2024
silverwind
silverwind previously requested changes Jul 12, 2024
View reviewed changes
Copy link
Member

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

This is definitely the wrong place to put this and I guarantee this will cause regressions. I would put it on selector .ui.multiple.search.selection.dropdown or maybe just .ui.ui.dropdown (but more dangerous) and test if any other similar dropdowns work fine.

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 12, 2024
@charles7668
Copy link
Contributor Author

This is definitely the wrong place to put this and I guarantee this will cause regressions. I would put it on selector .ui.multiple.search.selection.dropdown or maybe just .ui.ui.dropdown (but more dangerous) and test if any other similar dropdowns work fine.

Would it be better to put the tw-flex-wrap class in the div that has this issue?

Here is a test of adding tw-flex-wrap in the place where I know this issue occurs.

圖片
圖片
圖片

@6543
Copy link
Member

6543 commented Jul 12, 2024

I personally prevere tailwind classes over own css ...

@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Jul 12, 2024
6543
6543 previously approved these changes Jul 12, 2024
View reviewed changes
@silverwind
Copy link
Member

silverwind commented Jul 12, 2024 •
edited
Loading

I personally prevere tailwind classes over own css ...

The problem is unfortunately because we set display: flex on .ui.dropdown which is something Fomantic's CSS does not anticipate. Fomantic's CSS is only designed to have display: block on this element.

@6543: Tailwind is not ideal in this case because it needs to be replicated to every such dropdown in the UI and we need to fix all of them, so it would be a hacky fix if we do it with Tailwind because Tailwind only targets single elements, not "classes".

Ideally I would like to see whether we can remove this display: flex entirely, and if that is deemed too problematic, I can accept a override on .ui.multiple.search.selection.dropdown as well.

@6543
Copy link
Member

6543 commented Jul 12, 2024

Uh in this cas its hands of for me ... feel free to ping me if it's just about testing the result 😅

@silverwind can you describe your solution so @charles7668 can implement it ☺️ ?

@pull-request-size pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 12, 2024
@pull-request-size pull-request-size bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 12, 2024
@github-actions github-actions bot removed the modifies/templates This PR modifies the template files label Jul 12, 2024
@charles7668
Copy link
Contributor Author

I changed the .ui.ui.dropdown display style to inline-block and checked the dropdown divs that I know of, and it seems to work fine.

In single selection: seems like no difference.
In multiple selection: the display is the same as flex-wrap.

The following are the areas where I checked multiple selection:

  1. Create repository page: .gitignore section
  2. Commit graph page
  3. Repository settings page -> Tags

@silverwind
Copy link
Member

I changed the .ui.ui.dropdown display style to inline-block and checked the dropdown divs that I know of, and it seems to work fine.

How about just removing the display: flex and let the original Fomantic CSS take effect? If there is a specific reason for inline-block in some places, I recommend overwriting with tailwind classes there.

@silverwind
Copy link
Member

silverwind commented Jul 13, 2024 •
edited
Loading

#25163 had added this display: flex, so that is the PR where this was regressed.

@wxiaoguang do you recall the reason for display: flex?

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jul 13, 2024 •
edited
Loading

#25163 had added this display: flex, so that is the PR where this was regressed.

@wxiaoguang do you recall the reason for display: flex?

To make the UI align. All known cases (at that time) were written into the devtest page in #25163

@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 13, 2024
@6543 6543 dismissed their stale review July 13, 2024 14:47

outdated

@wxiaoguang
Copy link
Contributor

@6543 why "outdated" ? only one line change.

@lunny
Copy link
Member

lunny commented Aug 14, 2024

This is definitely the wrong place to put this and I guarantee this will cause regressions. I would put it on selector .ui.multiple.search.selection.dropdown or maybe just .ui.ui.dropdown (but more dangerous) and test if any other similar dropdowns work fine.

Please review again.

@wxiaoguang
Copy link
Contributor

@6543 why "outdated" ? only one line change.

@6543

@lunny lunny dismissed silverwind’s stale review October 13, 2024 02:10

I think the dismiss has been resolved and outdated.

@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 Oct 13, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 13, 2024
@lunny lunny enabled auto-merge (squash) October 13, 2024 02:14
@lunny lunny merged commit 6fced33 into go-gitea:main Oct 13, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.24.0 milestone Oct 13, 2024
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 13, 2024
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Oct 13, 2024
@charles7668 @wxiaoguang @GiteaBot
Fix dropdown content overflow (go-gitea#31610)
9408c71 
close go-gitea#31602 

![圖片](https://github.com/go-gitea/gitea/assets/30816317/43722917-bc5e-4e06-b0de-db52552a20df)

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@GiteaBot GiteaBot mentioned this pull request Oct 13, 2024
Merged
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Oct 13, 2024
wxiaoguang added a commit that referenced this pull request Oct 13, 2024
@GiteaBot @charles7668 @wxiaoguang
Fix dropdown content overflow (#31610) (#32250)
bcfe1f9 
Backport #31610 by charles7668

close #31602 

Co-authored-by: charles <30816317+charles7668@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@lunny lunny modified the milestones: 1.24.0, 1.23.0 Oct 13, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Oct 16, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
88e59ef 
* giteaofficial/main:
  Make `owner/repo/pulls` handlers use "PR reader" permission (go-gitea#32254)
  make `show stats` work when only one file changed (go-gitea#32244)
  Update scheduled tasks even if changes are pushed by "ActionsUser" (go-gitea#32246)
  Support migrating GitHub/GitLab PR draft status (go-gitea#32242)
  Only rename a user when they should receive a different name (go-gitea#32247)
  Fix dropdown content overflow (go-gitea#31610)
  Make git push options accept short name (go-gitea#32245)
  Allow code search by filename (go-gitea#32210)
  Allow maintainers to view and edit files of private repos when "Allow maintainers to edit" is enabled (go-gitea#32215)
  Use per package global lock for container uploads instead of memory lock (go-gitea#31860)
TKaxv-7S added a commit to TKaxv-7S/gitea that referenced this pull request Dec 1, 2024
@TKaxv-7S
Merge tag 'v1.22.4' into release/v1.22
c787b16 
* SECURITY
  * Fix basic auth with webauthn (go-gitea#32531) (go-gitea#32536)
  * Refactor internal routers (partial backport, auth token const time comparing) (go-gitea#32473) (go-gitea#32479)
* PERFORMANCE
  * Remove transaction for archive download (go-gitea#32186) (go-gitea#32520)
* BUGFIXES
  * Fix `missing signature key` error when pulling Docker images with `SERVE_DIRECT` enabled (go-gitea#32365) (go-gitea#32397)
  * Fix get reviewers fails when selecting user without pull request permissions unit (go-gitea#32415) (go-gitea#32616)
  * Fix adding index files to tmp directory (go-gitea#32360) (go-gitea#32593)
  * Fix PR creation on forked repositories via API (go-gitea#31863) (go-gitea#32591)
  * Fix missing menu tabs in organization project view page (go-gitea#32313) (go-gitea#32592)
  * Support HTTP POST requests to `/userinfo`, aligning to OpenID Core specification (go-gitea#32578) (go-gitea#32594)
  * Fix debian package clean up cron job (go-gitea#32351) (go-gitea#32590)
  * Fix GetInactiveUsers (go-gitea#32540) (go-gitea#32588)
  * Allow the actions user to login via the jwt token (go-gitea#32527) (go-gitea#32580)
  * Fix submodule parsing (go-gitea#32571) (go-gitea#32577)
  * Refactor find forks and fix possible bugs that weaken permissions check (go-gitea#32528) (go-gitea#32547)
  * Fix some places that don't respect org full name setting (go-gitea#32243) (go-gitea#32550)
  * Refactor push mirror find and add check for updating push mirror (go-gitea#32539) (go-gitea#32549)
  * Fix basic auth with webauthn (go-gitea#32531) (go-gitea#32536)
  * Fix artifact v4 upload above 8MB (go-gitea#31664) (go-gitea#32523)
  * Fix oauth2 error handle not return immediately (go-gitea#32514) (go-gitea#32516)
  * Fix action not triggered when commit message is too long (go-gitea#32498) (go-gitea#32507)
  * Fix `GetRepoLink` nil pointer dereference on dashboard feed page when repo is deleted with actions enabled (go-gitea#32501) (go-gitea#32502)
  * Fix `missing signature key` error when pulling Docker images with `SERVE_DIRECT` enabled (go-gitea#32397) (go-gitea#32397)
  * Fix the permission check for user search API and limit the number of returned users for `/user/search` (go-gitea#32310)
  * Fix SearchIssues swagger docs (go-gitea#32208) (go-gitea#32298)
  * Fix dropdown content overflow (go-gitea#31610) (go-gitea#32250)
  * Disable Oauth check if oauth disabled (go-gitea#32368) (go-gitea#32480)
  * Respect renamed dependencies of Cargo registry (go-gitea#32430) (go-gitea#32478)
  * Fix mermaid diagram height when initially hidden (go-gitea#32457) (go-gitea#32464)
  * Fix broken releases when re-pushing tags (go-gitea#32435) (go-gitea#32449)
  * Only provide the commit summary for Discord webhook push events (go-gitea#32432) (go-gitea#32447)
  * Only query team tables if repository is under org when getting assignees (go-gitea#32414) (go-gitea#32426)
  * Fix created_unix for mirroring (go-gitea#32342) (go-gitea#32406)
  * Respect UI.ExploreDefaultSort setting again (go-gitea#32357) (go-gitea#32385)
  * Fix broken image when editing comment with non-image attachments (go-gitea#32319) (go-gitea#32345)
  * Fix disable 2fa bug (go-gitea#32320) (go-gitea#32330)
  * Always update expiration time when creating an artifact (go-gitea#32281) (go-gitea#32285)
  * Fix null errors on conversation holder (go-gitea#32258) (go-gitea#32266) (go-gitea#32282)
  * Only rename a user when they should receive a different name (go-gitea#32247) (go-gitea#32249)
  * Fix checkbox bug on private/archive filter (go-gitea#32236) (go-gitea#32240)
  * Add a doctor check to disable the "Actions" unit for mirrors (go-gitea#32424) (go-gitea#32497)
  * Quick fix milestone deadline 9999 (go-gitea#32423)
  * Make `show stats` work when only one file changed (go-gitea#32244) (go-gitea#32268)
  * Make `owner/repo/pulls` handlers use "PR reader" permission (go-gitea#32254) (go-gitea#32265)
  * Update scheduled tasks even if changes are pushed by "ActionsUser" (go-gitea#32246) (go-gitea#32252)
* MISC
  * Remove unnecessary code: `GetPushMirrorsByRepoID` called on all repo pages (go-gitea#32560) (go-gitea#32567)
  * Improve some sanitizer rules (go-gitea#32534)
  * Update nix development environment vor v1.22.x (go-gitea#32495)
  * Add warn log when deleting inactive users (go-gitea#32318) (go-gitea#32321)
  * Update github.com/go-enry/go-enry to v2.9.1 (go-gitea#32295) (go-gitea#32296)
  * Warn users when they try to use a non-root-url to sign in/up (go-gitea#32272) (go-gitea#32273)

# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCAAdFiEEumb2f9c/cFjXEtMIw7fJG2Mvc4oFAmdEyeoACgkQw7fJG2Mv
# c4pythAAn57Z9Csfd8UrHbCd87SBlEGydhlng5Oc99pQIAvExR0hc9VFWjt5pFr4
# aXTajtzb/sDQkAPZEiL45CL471z+Ga81ixaKRfrBeMiSECB0wBaL4+XH94qQ3lw3
# /dNfQsc9bUnomGWQyEIbQ6mT85fJdvBD1nibUSH3b5P4WqOBHbY9YlehPmE96KY2
# 9k1IYvBvcfCjK6njVQ7m+sFOr7/Y2ZHe9FeN8hEf/1Bfnc75wtkeNyeXnlNe67Eo
# ViFzcA35WyTXw4NRY+TG/8xZEXHl8DuOuUdPoBqkpFw9TzxR2svO0QLzRIHgJP+t
# /Cdd16zZd6fQ+ET+DV8IaF2wlXdEgVDWs2aT04VDLGpSw9czxsUEUQ0ETWFFomXN
# //goTLu1B3fVQYrE9MK2vfUQGe2Su3ChGwNtNEK9bMQpO6sLFGRE0nPgBJMPJ0yA
# bfPhRlsVxnyEToqeKoC77wv0kPiOkzPfDm6sFLAt+tATcij5UlTU4nVXyXsELk14
# p5mtsTfaEqiH3U+JW0Drz8wV7nk8F599lZbYO92M3Z59bqC5TsOVYgqb1ODTpqQO
# 7gLdgdKmQbKWTPHLA9Hz+0/3bT1MirMRdtXW7TmgW83TuN37wOuElCmXmJTN2feY
# LG4k417kVrBwF+fdGPXo+T7H0MqxX1fTkVftG3C63sdaRQrUM1M=
# =jyQM
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue, Nov 26, 2024  3:03:06 AM
# gpg:                using RSA key BA66F67FD73F7058D712D308C3B7C91B632F738A
# gpg: Note: database_open 134217901 waiting for lock (held by 1152) ...
# gpg: Note: database_open 134217901 waiting for lock (held by 1152) ...
# gpg: Note: database_open 134217901 waiting for lock (held by 1152) ...
# gpg: Note: database_open 134217901 waiting for lock (held by 1152) ...
# gpg: Note: database_open 134217901 waiting for lock (held by 1152) ...
# gpg: keydb_search failed: Connection timed out
# gpg: Note: database_open 134217901 waiting for lock (held by 1152) ...
# gpg: Note: database_open 134217901 waiting for lock (held by 1152) ...
# gpg: Note: database_open 134217901 waiting for lock (held by 1152) ...
# gpg: Note: database_open 134217901 waiting for lock (held by 1152) ...
# gpg: Note: database_open 134217901 waiting for lock (held by 1152) ...
# gpg: keydb_search failed: Connection timed out
# gpg: Can't check signature: No public key
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jan 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@delvh delvh delvh approved these changes

@silverwind silverwind silverwind left review comments

@6543 6543 6543 left review comments

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created backport/v1.22 This PR should be backported to Gitea 1.22 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/templates This PR modifies the template files size/M Denotes a PR that changes 30-99 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI type/bug
Projects
None yet
Milestone
1.23.0
Development

Successfully merging this pull request may close these issues.

Adding .gitignore templates overflow
8 participants
@charles7668 @6543 @silverwind @wxiaoguang @lunny @delvh @techknowlogick @GiteaBot