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

fixed the dropdown menu for the top New button to expand to the left #31273

Merged
merged 4 commits into from
Jun 6, 2024

Conversation

kerwin612
Copy link
Member

before:
1717660314025

after:
1717660674763

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 6, 2024
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 6, 2024
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Jun 6, 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 Jun 6, 2024
@lunny lunny added this to the 1.23.0 milestone Jun 6, 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 Jun 6, 2024
@silverwind
Copy link
Member

silverwind commented Jun 6, 2024

Not sure if this method is really something Fomantic supports officially, but if it works, we can go with it. I think we solved past cases via manual CSS.

@silverwind
Copy link
Member

silverwind commented Jun 6, 2024

Actually fix is correct, I checked the CSS, there are two rules which left-align .menu.left.

@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 6, 2024
@silverwind silverwind enabled auto-merge (squash) June 6, 2024 23:21
@silverwind silverwind added the backport/v1.22 This PR should be backported to Gitea 1.22 label Jun 6, 2024
@silverwind silverwind merged commit ab1948d into go-gitea:main Jun 6, 2024
26 checks passed
@GiteaBot GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Jun 6, 2024
@silverwind
Copy link
Member

silverwind commented Jun 7, 2024

Unfortunately this introduced a regression where the menu flashes on page load. Will investigate later. It needs a fix similar to #30215.

@kerwin612
Copy link
Member Author

Unfortunately this introduced a regression where the menu flashes on page load. Will investigate later. It needs a fix similar to #30215.

https://discord.com/channels/322538954119184384/322910365237248000/1248480303719252029 Can we discuss it?

@silverwind
Copy link
Member

silverwind commented Jun 7, 2024

Not right now, but to explain the issue: There is this CSS:

.ui.menu:not(.vertical) .left.item,
.ui.menu:not(.vertical) .left.menu {
  display: flex;
}

Dropdown menus are hidden on page load via

.ui.dropdown .menu {
  display: none;
}

Above selector has more specificity so dropdown shows before JS has loaded which takes a few milliseconds usually. Once JS loads, it hides the menu via hidden class.

Maybe as a more general solution we can remove the first CSS, but I'm not sure whether there would be side effects of doing that.

Fix is in #31281.

@kerwin612
Copy link
Member Author

Maybe it could be changed like this?:

.ui.dropdown .menu {
  display: none!important;
}

@silverwind
Copy link
Member

silverwind commented Jun 7, 2024

I don't think it's worth the time trying to make left menu work. Fomantic only specifies left/right classes to indicate the direction in which child menus open, not the root menu:

https://fomantic-ui.com/modules/dropdown.html#menu-direction

So a left menu is essentially not supported and that is already shady territory and any attempt trying to make it work is likely to introduce regressions in other dropdowns which have very brittle CSS.

So I suggest you leave it be and find some other areas to work on :)

@kerwin612
Copy link
Member Author

OK, I changed it because I saw this code:

leftward : 'left',

if(!module.is.leftward($menu) && !module.can.openRightward($menu)) {

leftward: function($subMenu) {

OK, that's the end of the suggest.

zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 7, 2024
* giteaofficial/main:
  Fix and clean up `ConfirmModal` (go-gitea#31283)
  Enable poetry non-package mode (go-gitea#31282)
  fixed the dropdown menu for the top New button to expand to the left (go-gitea#31273)
  Optimize repo-list layout to enhance visual experience (go-gitea#31272)
  Allow including `Reviewed-on`/`Reviewed-by` lines for custom merge messages (go-gitea#31211)
  Add `MAX_ROWS` option for CSV rendering (go-gitea#30268)
  Update `golang.org/x/net` (go-gitea#31260)
  Add replacement module for `mholt/archiver` (go-gitea#31267)
  Fix Activity Page Contributors dropdown (go-gitea#31264)
silverwind added a commit that referenced this pull request Jun 12, 2024
Fixes
#31273 (comment).
Same method as used in #30215. All
left-opening dropdowns need to use it method.

---------

Co-authored-by: wxiaoguang <[email protected]>
Co-authored-by: Giteabot <[email protected]>
GiteaBot added a commit to GiteaBot/gitea that referenced this pull request Jun 12, 2024
Fixes
go-gitea#31273 (comment).
Same method as used in go-gitea#30215. All
left-opening dropdowns need to use it method.

---------

Co-authored-by: wxiaoguang <[email protected]>
Co-authored-by: Giteabot <[email protected]>
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 5, 2024
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 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/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants