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

Menus do not show completely even if there is vertical space, always restricted to 50% #3920

Closed
ChristophWurst opened this issue Mar 23, 2023 · 18 comments · Fixed by #5806
Closed
Labels
1. to develop Accepted and waiting to be taken care of bug Something isn't working design Design, UX, interface and interaction design feature: actions Related to the actions components

Comments

@ChristophWurst
Copy link
Contributor

How to use GitHub

  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Steps to reproduce

  1. Join a call
  2. Click ... left to the Leave call button

Expected behaviour

Static menu

Actual behaviour

1 2
Bildschirmfoto vom 2023-03-23 15-20-01 Bildschirmfoto vom 2023-03-23 15-19-55

Talk app

Talk app version: (see apps admin page: /index.php/settings/apps)

Custom Signaling server configured: yes/no and version (see additional admin settings: /index.php/index.php/settings/admin/talk#signaling_server)

Custom TURN server configured: yes/no (see additional admin settings: /index.php/settings/admin/talk#turn_server)

Custom STUN server configured: yes/no (see additional admin settings: /index.php/settings/admin/talk#stun_server)

Browser

Microphone available: yes/no

Camera available: yes/no

Operating system: Windows/Ubuntu/...

Browser name: Firefox/Chrome/...

Browser version: 85/96/...

Browser log

``` Insert your browser log here, this could for example include: a) The javascript console log b) The network log c) ... ```

Server configuration

Operating system: Ubuntu/RedHat/...

Web server: Apache/Nginx

Database: MySQL/Maria/SQLite/PostgreSQL

PHP version: 8.0/8.1/8.2

Nextcloud Version: (see admin page)

List of activated apps:

If you have access to your command line run e.g.:
sudo -u www-data php occ app:list
from within your server installation folder

Nextcloud configuration:

If you have access to your command line run e.g.:
sudo -u www-data php occ config:list system
from within your Nextcloud installation folder

Server log (data/nextcloud.log)

Insert your server log here
@ChristophWurst ChristophWurst added the bug Something isn't working label Mar 23, 2023
@szaimen
Copy link
Contributor

szaimen commented Mar 23, 2023

I suppose this is due to this?
#3445

@szaimen
Copy link
Contributor

szaimen commented Mar 23, 2023

Scrolling is needed for small display sizes as otherwise you have no way to see the whole menu.

@ChristophWurst
Copy link
Contributor Author

Could this be reactive? My screen is big enough for the full menu.

@szaimen

This comment was marked as resolved.

@ChristophWurst

This comment was marked as resolved.

@szaimen

This comment was marked as resolved.

@szaimen
Copy link
Contributor

szaimen commented Mar 23, 2023

Shall we move this to the vue lib as it needs to be handled there?

@ChristophWurst
Copy link
Contributor Author

Makes sense. Talk is unfortunate to have a menu long enough to cause a scrollbar but too short to scroll a lot of content.

@szaimen szaimen transferred this issue from nextcloud/spreed Mar 23, 2023
@szaimen
Copy link
Contributor

szaimen commented Mar 23, 2023

Talk is unfortunate to have a menu long enough to cause a scrollbar but too short to scroll a lot of content.

Indeed. I guess we only need to add some more elements to that popover then and it will be fine XD

@raimund-schluessler
Copy link
Contributor

Makes sense. Talk is unfortunate to have a menu long enough to cause a scrollbar but too short to scroll a lot of content.

There has to be a limit for the drop-down menu height. So for a certain number of elements, the menu will just scroll a little. We could of course make the height dynamic via JavaScript and reduce the height so it at least scrolls two elements for example. That way we could also ensure it shows a fraction of the element, so it's clearer the list scrolls, see #3842.

@szaimen
Copy link
Contributor

szaimen commented Mar 23, 2023

Sounds good!

I wonder why in #3842 no scrollbar is visible?

@szaimen
Copy link
Contributor

szaimen commented Mar 23, 2023

For me it is:
image

@raimund-schluessler
Copy link
Contributor

Could this be reactive? My screen is big enough for the full menu.

I think the reason why the height is limited to 50vh minus something here
https://github.com/nextcloud/nextcloud-vue/blob/e299b0c823186e57f75fc9a2a4dc1973329ea733/src/components/NcActions/NcActions.vue#L1193
is that the menu should be completely visible no matter where the trigger resides. In the worst case, the trigger sits in the viewport center, and there is equal space above and below. Having a menu higher than half the viewport height would mean a part of it is cut off. So in order to make the menu reactively bigger, one would need to observe the trigger position and adjust the height accordingly. I don't have a good idea where or how to implement this, to be honest.

Dynamically adjusting the height so that if the menu has to scroll at least two items are hidden and that the last visible item is partially hidden seems easier as a first step, I would say.

@nickvergessen
Copy link
Contributor

In the worst case, the trigger sits in the viewport center, and there is equal space above and below.

The menu shown above is at the top of the screen (in the Talk call top bar), so always unfolds to the bottom.
So we could temporarily overwrite it inside talk with a 75vh or something?

@szaimen
Copy link
Contributor

szaimen commented Mar 27, 2023

So we could temporarily overwrite it inside talk with a 75vh or something?

yeah, that would be an idea for the meantime 👍

@jancborchardt
Copy link
Contributor

We could of course make the height dynamic via JavaScript and reduce the height so it at least scrolls two elements for example. That way we could also ensure it shows a fraction of the element, so it's clearer the list scrolls, see #3842.

@raimund-schluessler just to clarify the other issue too – it would be more important to show everything of the menu if there is enough vertical space rather than arbitrarily cutting it off to show it’s scrollable. :)

@jancborchardt jancborchardt added 1. to develop Accepted and waiting to be taken care of feature: actions Related to the actions components design Design, UX, interface and interaction design labels Nov 21, 2023
@marcoambrosini
Copy link
Contributor

So I think it's too late to touch the actions component. Modifying things there can cause all sorts of regressions everywhere and @szaimen rationale to use 50vh makes sense. I'm trying to apply a fix for talk but for some reason the deep modifier doesn't work.

In NcActions I would add overflow-y: scroll so that the scroll bar is always shown. Relying on the last element being cut is a bit flaky and prone to failure.

this would be the result after the fix
max-height: min(100dvh - 150px, 80dvh)

Screencast.from.2023-11-21.13-50-58.webm

@jancborchardt jancborchardt moved this to 🧭 Planning evaluation / ideas in 🖍 Design team Nov 21, 2023
@jancborchardt jancborchardt changed the title Call menu scrolls Call menu does not show completely Dec 12, 2023
@jancborchardt jancborchardt changed the title Call menu does not show completely Menus do not show completely even if there is vertical space, always restricted to 50% Dec 12, 2023
@jancborchardt jancborchardt moved this from 🧭 Planning evaluation / ideas to 📐 Design phase in 🖍 Design team Jan 17, 2024
@jancborchardt jancborchardt moved this from 📐 Design phase to 🏗️ At engineering in 🖍 Design team Jan 17, 2024
@AndyScherzinger AndyScherzinger moved this to 📄 To do (~10 entries) in 📁 Files team Jan 18, 2024
@juliusknorr
Copy link
Contributor

Possible related issue especially for small screens #3159

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of bug Something isn't working design Design, UX, interface and interaction design feature: actions Related to the actions components
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants