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 navbar + menu flashing on page load #31281

Merged
merged 12 commits into from
Jun 12, 2024
Merged

Fix navbar + menu flashing on page load #31281

merged 12 commits into from
Jun 12, 2024

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Jun 7, 2024 •
edited
Loading

Fixes #31273 (comment). Same method as used in #30215. All left-opening dropdowns need to use it method.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 7, 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 7, 2024
@silverwind silverwind added type/bug backport/v1.22 This PR should be backported to Gitea 1.22 labels Jun 7, 2024
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Jun 7, 2024
@silverwind silverwind mentioned this pull request Jun 7, 2024
Merged
@silverwind silverwind changed the title Fix navbar + menu flashing Fix navbar + menu flashing on page load Jun 7, 2024
@silverwind
Copy link
Member Author

Would appreciate some swift reviews here, this bug is very annoying.

@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 11, 2024
@wxiaoguang
Copy link
Contributor

I think left menu is officially supported. And the correct usage should be: left menu transition hidden, then the menu could be placed correctly, and no flash.

@silverwind
Copy link
Member Author

I think left menu is officially supported. And the correct usage should be: left menu transition hidden, then the menu could be placed correctly, and no flash.

I don't think it is, 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

image

So left only is only supported for child menus to a dropdown, not the root menu itself.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jun 11, 2024 •
edited
Loading

It is. When you only have <div class="menu user-menu"> in HTML, then when it shows, Fomantic calculates the position, and the final HTML is:

image

@silverwind
Copy link
Member Author

silverwind commented Jun 11, 2024 •
edited
Loading

OTOH, the second example on the link seems to work:

<div class="ui floating labeled icon dropdown button">
  <i class="dropdown icon"></i>
  <span class="text">Menu</span>
  <div class="left menu">

It does not show when I turn JS off like it does on gitea, so I suspect that we should just remove the conflicting display: flex !important rule as it seems not to be from original fomantic CSS:

image

@silverwind silverwind marked this pull request as draft June 11, 2024 14:24
@silverwind
Copy link
Member Author

The root of the problem is we have menu class on a parent node so fomantic thinks it's a child menu:

image

@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 Jun 11, 2024
@silverwind
Copy link
Member Author

76f5e50 removes the problematic class and copies all styles from it onto the affected elements. Renders exactly as before for me, including mobile view.

@silverwind
Copy link
Member Author

silverwind commented Jun 12, 2024 •
edited
Loading

Need to check if there are any more .ui.secondary.menu .dropdown .left.menu cases.

I think it's worth a try to remove .ui.menu:not(.vertical) .left.menu CSS in any case as we are not using dropdown submenus as far as I'm aware.

@silverwind silverwind marked this pull request as draft June 12, 2024 08:24
@silverwind
Copy link
Member Author

Actually let's merge this now as-is, don't wanna extend the scope of this PR and this is a important fix to have for v1.22.

@silverwind silverwind marked this pull request as ready for review June 12, 2024 14:28
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 12, 2024
@silverwind silverwind enabled auto-merge (squash) June 12, 2024 14:28
@wxiaoguang
Copy link
Contributor

Need to check if there are any more .ui.secondary.menu .dropdown .left.menu cases.

I do not see more bad cases.

I think it's worth a try to remove .ui.menu:not(.vertical) .left.menu CSS in any case as we are not using dropdown submenus as far as I'm aware.

That's how fomantic dropdown works. It officially uses leftward (left menu). See its "direction" function.

@silverwind silverwind removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 12, 2024
@silverwind silverwind disabled auto-merge June 12, 2024 14:36
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 12, 2024
@silverwind silverwind enabled auto-merge (squash) June 12, 2024 14:51
@silverwind silverwind merged commit 21ba5ca into go-gitea:main Jun 12, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Jun 12, 2024
GiteaBot added a commit to GiteaBot/gitea that referenced this pull request Jun 12, 2024
@silverwind @wxiaoguang @GiteaBot
Fix navbar + menu flashing on page load (go-gitea#31281)
a7d13a9 
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]>
@GiteaBot GiteaBot mentioned this pull request Jun 12, 2024
Merged
@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 12, 2024
wxiaoguang added a commit that referenced this pull request Jun 12, 2024
@GiteaBot @silverwind @wxiaoguang
Fix navbar + menu flashing on page load (#31281) (#31342)
4b6ef92 
Backport #31281 by silverwind

Co-authored-by: silverwind <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
@silverwind silverwind deleted the navflash branch June 12, 2024 15:28
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 13, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
5b45fc8 
* giteaofficial/main:
  Fixed incorrect localization `explorer.go` (go-gitea#31348)
  Improve detecting empty files (go-gitea#31332)
  Fix hash render end with colon (go-gitea#31319)
  Fix line number widths (go-gitea#31341)
  Fix navbar `+` menu flashing on page load (go-gitea#31281)
  Reduce memory usage for chunked artifact uploads to MinIO (go-gitea#31325)
  Fix dates displaying in a wrong manner when we're close to the end of the month (go-gitea#31331)
  Fix adopt repository has empty object name in database (go-gitea#31333)
  Optimize profile layout to enhance visual experience (go-gitea#31278)
@sebastian-sauer sebastian-sauer mentioned this pull request Jul 24, 2024
Closed
@wxiaoguang wxiaoguang mentioned this pull request Jul 24, 2024
Merged
wxiaoguang added a commit that referenced this pull request Jul 25, 2024
@wxiaoguang
Fix "Filter by commit" Dropdown (#31695)
169031b 
Regression of #31281
Fix #31673
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@kerwin612 kerwin612 kerwin612 approved these changes

@lunny lunny lunny approved these changes

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/js modifies/templates This PR modifies the template files size/M Denotes a PR that changes 30-99 lines, ignoring generated files. type/bug
Projects
None yet
Milestone
1.23.0
Development

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