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

fix(AppMenu): Prevent menu entries from jumping on hover #47203

Merged
merged 2 commits into from
Aug 14, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Aug 13, 2024

@susnux susnux added bug design Design, UI, UX, etc. 3. to review Waiting for reviews regression labels Aug 13, 2024
@susnux susnux added this to the Nextcloud 30 milestone Aug 13, 2024
@susnux susnux mentioned this pull request Aug 13, 2024
@Pytal
Copy link
Member

Pytal commented Aug 13, 2024

Tested and the icons don't move anymore but the text gets offset away from the hovered app which looks a bit strange 🤔
image

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Aug 14, 2024
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Same as @Pytal noted – we could make the hovered/active app wider (like now) just without shifting the labels left and right if it, if possible? So that they keep centered below their icon.

@susnux
Copy link
Contributor Author

susnux commented Aug 14, 2024

@jancborchardt if we make the hovered wider, without shifting, then everything will move. If you hover the middle entry of 3 app, then the right one will move to the right because the hovered grows.
That why I instead just shrink the text of the neighbors to allow the hovered one to grow without any movement of the app entries.

So the problem here is, that the current version (not this PR) is shrinking & expanding items, but to prevent visual jumping the app icon is always kept on the same place (so it is not centered anymore but at least you do not see that jumping). Due to the animation the sizes are animated and when you move your mouse two items are animated in different states -> causing the problem that this PR should solve.

This PR is not growing the item anymore, but instead just expand the label -> no weird hover states anymore so no jumping. But if the app text is small this causes this issue, so I could adjust it to only shrink / grow if needed (so the hovered text is ellipsed), but this is then not CSS only anymore and needs some JS.

@jancborchardt
Copy link
Member

@susnux so by making it wider I also only mean the text label, not the whole entry (so there is no jumping).
Can the neighbor labels just shrink in width instead of moving?

(Ideal would be some overlap or fade into transparency of the neighbor items, but these don't work with non-plain backgrounds.)

@susnux
Copy link
Contributor Author

susnux commented Aug 14, 2024

@jancborchardt maybe like this:

  1. Only grow if needed
  2. Keep the label centered

The problem with 2 is, that now the label needs to be quite small (we "waste" the space on one side).

Bildschirmaufnahme_20240814_120108.webm

(commit pushed)

@susnux susnux requested a review from jancborchardt August 14, 2024 10:04
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

@susnux yeah I understand the issue – your last proposal seems like the best compromise for now. :)
(Maybe for the future it would be good to use some JS indeed to polish it more, considering it is the main nav)

@skjnldsv @Pytal what do you think?

Only grow and shrink app menu entry if needed

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the fix/app-menu-hover branch from 373695b to 44705ab Compare August 14, 2024 10:24
@skjnldsv skjnldsv modified the milestones: Nextcloud 30, Nextcloud 31 Aug 14, 2024
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Aug 14, 2024
@susnux

This comment was marked as resolved.

@susnux susnux merged commit fb90e7e into master Aug 14, 2024
112 checks passed
@susnux susnux deleted the fix/app-menu-hover branch August 14, 2024 14:56
@skjnldsv skjnldsv mentioned this pull request Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug design Design, UI, UX, etc. regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Content shift when mouse is over top menu elements
4 participants