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

Prevent flash of dropdown menu on labels list #30215

Merged
merged 5 commits into from
Mar 31, 2024

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Mar 31, 2024

On the labels list, This left class caused the dropdown content to flash on page load until JS had hidden it. Remove it as I see no purpose to it.

image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 31, 2024
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 31, 2024
@silverwind silverwind added the backport/v1.22 This PR should be backported to Gitea 1.22 label Mar 31, 2024
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Mar 31, 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 Mar 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 Mar 31, 2024
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 31, 2024
@silverwind silverwind enabled auto-merge (squash) March 31, 2024 13:27
@silverwind silverwind added the backport/v1.21 This PR should be backported to Gitea 1.21 label Mar 31, 2024
@wxiaoguang wxiaoguang removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 31, 2024
@wxiaoguang
Copy link
Contributor

IIRC, it is a must, otherwise the menu overflows on a wrong direction when the screen width is small.

Could you check the git log?

@wxiaoguang wxiaoguang added the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Mar 31, 2024
@silverwind
Copy link
Member Author

silverwind commented Mar 31, 2024

Looks fine on small screen. It's always left-aligned.

Screenshot 2024-03-31 at 15 35 37

@silverwind
Copy link
Member Author

Only issue is that if you open the menu on desktop, then downsize, it will stay right-aligned and be partially outside the screen.

@silverwind
Copy link
Member Author

Okay with these new classes, it stays hidden on page load and always opens left, regardless of screen size.

@wxiaoguang wxiaoguang removed the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Mar 31, 2024
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 31, 2024

Did a quick test, it seems that class=menu works well. Maybe because the "transition" module has been removed, so the overflow bug also disappears.

So I think it is ok to only keep class=menu now.

@silverwind
Copy link
Member Author

The problem is .ui.menu:not(.vertical) .left.menu {display: flex} has more specificity than .ui.dropdown.menu {display:none}. I don't think this usage with just left is something that fomantic supports.

@wxiaoguang
Copy link
Contributor

I don't think this usage with just left is something that fomantic supports.

IIRC it is used by Fomantic UI JS code to calculate the popup position.

@silverwind
Copy link
Member Author

silverwind commented Mar 31, 2024

The left class is meant only for child menus, not the main menu, so it was an abuse of fomantic:

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

Specifying left on a menu will make all child menus open in the same direction implicitly

Even thought I do prefer it to open to left, let's remove it for the sake of easier future migration off of it.

@silverwind
Copy link
Member Author

What's interesting is the issue filter dropdown also open to left and don't flash, so there must be a difference.

image

@silverwind
Copy link
Member Author

A I see this is a CSS override .repository .filter.menu .ui.dropdown .menu {right: 0} that makes them open to left.

@silverwind silverwind removed the backport/v1.21 This PR should be backported to Gitea 1.21 label Mar 31, 2024
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

sorry it doesn't seem right by introducing these tw- helpers.

Fomantic UI JS just calculate the position automatically. For example, when the screen width is small, the first popup menu should be left-aligned.

@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 Mar 31, 2024
@silverwind silverwind added the backport/v1.21 This PR should be backported to Gitea 1.21 label Mar 31, 2024
@silverwind
Copy link
Member Author

silverwind commented Mar 31, 2024

Added new CSS solution that will work in all these right-aligned headers. It looks better to always left-align in these imho. Fomantic is too dumb to correctly position itself.

@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 Mar 31, 2024
@silverwind silverwind removed the backport/v1.21 This PR should be backported to Gitea 1.21 label Mar 31, 2024
@silverwind silverwind merged commit 8da9130 into go-gitea:main Mar 31, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Mar 31, 2024
@silverwind silverwind deleted the labflash branch March 31, 2024 14:59
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Mar 31, 2024
On the labels list, This `left` class caused the dropdown content to
flash on page load until JS had hidden it. Remove it as I see no purpose
to it.

<img width="215" alt="image"
src="https://github.com/go-gitea/gitea/assets/115237/9e1de97f-dd89-41e0-9229-5c4a786ba762">

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Mar 31, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 31, 2024
* giteaofficial/main:
  Remove fomantic input module (go-gitea#30194)
  Remove most jQuery function calls from the repository topic box (go-gitea#30191)
  Prevent flash of dropdown menu on labels list (go-gitea#30215)
  Remove jQuery class from the `repo-issue.js` file (go-gitea#30192)
  Ignore fomantic folder in linters (go-gitea#30200)
  Remove `modifies/frontend` from labeler (go-gitea#30198)
  Make a distinction between `active` and `selected` in the issue author dropdown (go-gitea#30207)
  Move and simplify tab-size helpers (go-gitea#30196)
  Fix markdown color code detection (go-gitea#30208)
lunny pushed a commit that referenced this pull request Apr 1, 2024
Backport #30215 by @silverwind

On the labels list, This `left` class caused the dropdown content to
flash on page load until JS had hidden it. Remove it as I see no purpose
to it.

<img width="215" alt="image"
src="https://github.com/go-gitea/gitea/assets/115237/9e1de97f-dd89-41e0-9229-5c4a786ba762">

Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@wxiaoguang wxiaoguang added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Apr 27, 2024
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 <wxiaoguang@gmail.com>
Co-authored-by: Giteabot <teabot@gitea.io>
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 <wxiaoguang@gmail.com>
Co-authored-by: Giteabot <teabot@gitea.io>
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jun 29, 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. 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.

5 participants