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

context menu can go outside the window when too far right #1176

Closed
jakobhellermann opened this issue Jan 29, 2022 · 11 comments
Closed

context menu can go outside the window when too far right #1176

jakobhellermann opened this issue Jan 29, 2022 · 11 comments
Assignees
Labels
bug Something is broken

Comments

@jakobhellermann
Copy link

Describe the bug
When a context menu is opened too close to the edge of the window, most of it is clipped and not accessible.

To Reproduce
Add a menu button to the UI close to the right edge of the window.

ui.with_layout(Layout::right_to_left(), |ui| {
    ui.menu_button("Add", |ui| {
        if ui.button("Option 1").clicked() {
            ui.close_menu();
        }
        if ui.button("Option 2, which is longer").clicked() {
            ui.close_menu();
        }
    });
});

Expected behavior
It would be cool if the popup location was constrained so that it didn't move past the window boundary. (Once that is implemented, the submenus could also open to the left, but that should be another issue).

Screenshots
grafik

@jakobhellermann jakobhellermann added the bug Something is broken label Jan 29, 2022
@yusdacra
Copy link
Contributor

yusdacra commented Feb 6, 2022

Note this also happens for other edges as well. A popup can clip over the bottom edge of a window. This sounds like it wouldn't be easy to implement given that this is an immediate mode UI, but maybe a workaround for now could just be to let users specify a direction for popups, so that they can at least decide if their popup content is fixed size whether it will clip a screen edge or not.

@emilk
Copy link
Owner

emilk commented Feb 10, 2022

I think this could be an easy fix.

Tooltips save their sizes from one frame to another and use that to figure out how to position them so they are never outside of the screen. We could/should do the same for menus and for ComboBoxes.

@coderedart
Copy link
Contributor

#683 related issue of what happens when the menu overflows the screen (in this case) and vertically (in the linked issue).

for vertical overflows, scrolling was considered in the PR of context menus #543 (comment)

@juancampa
Copy link
Contributor

Tooltips save their sizes from one frame to another and use that to figure out how to position them so they are never outside of the screen.

I want to bring up the fact that, currently, huge tooltips (larger than the window) will flicker doing this. I think, but I'm not sure, the reason is that while adjusting to fit in the screen, the tooltip is pushed under the pointer, causing the widget to not be hovered anymore, and then hovered again, and then not hovered, etc.

@mankinskin
Copy link
Contributor

mankinskin commented Apr 16, 2022

The menu is created with its top left corner at the bottom left corner of the element it is created on (the Add button).

egui/egui/src/menu.rs

Lines 292 to 293 in 2d2022f

let pos = response.rect.left_bottom();
return MenuResponse::Create(pos, id);

It's been a while since I used this library, but last time I used it there was no easy way to know the size of a Ui before it is rendered, so we couldn't tell here if the menu goes off screen and where to place it instead. A solution would be to measure the elements before actually rendering them, there was an issue about this here #606.

There might be another way with layouts though.. you could manually set the menu to expand right to left and be positioned at the bottom right corner of the button creating it. More configuration like this should be easily possible to add, but I am not sure if it can actually work with a right to left layout..

In any case, an automatic approach will require calculating the menu's size before rendering it unless I am missing some trick.

Edit: the other option would of course be what @emilk mentioned, i.e. to save the size from the last frame. However I find this solution kind of hacky, as we would obviously like to have elements render correctly on the first frame.

@Mingun
Copy link
Contributor

Mingun commented Apr 17, 2022

However I find this solution kind of hacky, as we would obviously like to have elements render correctly on the first frame.

I'm curious... What the problem with the first frame? Even with 30Hz refresh rate hardly ever that you'll notice something wrong, besides, you may not show the first frame at all

@mankinskin
Copy link
Contributor

mankinskin commented Apr 17, 2022

For example when you click a button to show an element, the menu would render first without knowing its size, meaning it would go off the screen. Then the next frame it would change its position to stay in screen. This would cause a flicker which isn't ideal.

@mankinskin
Copy link
Contributor

mankinskin commented Apr 29, 2022

But I guess the caching of boundingboxes is still useful, so we could store and Option of a bounding box and if it is None we run a dry render which doesn't draw anything, just to get the size, then reposition/resize the element and store the bounding box to draw it next frame. There just needs to be a way to invalidate that cache when contents or contexts change, which may be tricky. For example when the contents are animated or get updated by shared data and it resizes, the position might need to change. Or when a new element is created and the elements are not supposed to overlap, the cache would need to be recalculated etc.

I mean it may not be a big issue if the layout isn't 100% correct for a single frame and there is a flicker, but I would wish things like this didn't happen in a UI. It just decreases the user experience, especially for interactive elements also with text like menus.

So I guess costly but more accurate is the approach of re measuring each element before every draw. This can be especially heavy on complex components though, because each child needs to be measured aswell. Also any code mutating state of the component shouldn't be called there because it will be called again in the actual drawing step.

To avoid stepping through the component's elements twice we could just cache the bounding box of the previous frame, which would cause the components to be rendered with wrong dimensions on the first frame, which may or may not be noticable. (maybe it isn't a problem really, I am just suspicious of it)

I guess the ideal solution would be to measure a component only when you know you need to and then reuse the cached dimensions whenever possible. That would be fast and accurate. I am not sure how to detect when a cache invalidation is needed though. This topic is also related I think: #724

I think the only times where the bounding boxes need to be recalculated is when something inside the component changes or when the component it is in changes in a way that influences the component. I we can somehow get a communication between components so they can say "yo parent, I did something that should change your size" or "yo child, I did something that should change your size/position" we could probably implement the rest very easily.

Another option is to have the downstream developer control cache invalidation, so when they know that an element will change its size they can invalidate its cache and the bounding box is recalculated. Additionally we could always cache the bounding box of the previous frame so even when the cache is not invalidated, it is still updated and will be correct at the next frame. So the cache invalidation would really just allow developers to prevent any flickers or things like that.

@isolune
Copy link

isolune commented May 9, 2022

Is there an ad hoc solution at the moment for translating the context menu area? I can compute manually whether a menu will go off the screen, but am unsure how to control where it appears.

@mankinskin
Copy link
Contributor

Not currently, but I also wanted to create a menu at a fixed position at one point and configuring how the menus are created seem generally important. So if anyone feels motivated to add this, feel free to create a pull request. I will probably not take time to work on this in the next few weeks, because I only use this for a secondary part of a hobby project.

@emilk
Copy link
Owner

emilk commented Jun 5, 2024

Fixed on master

@emilk emilk closed this as completed Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken
Projects
None yet
Development

No branches or pull requests

8 participants