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 PopupMenu unresponsive on right side of menu #41689

Merged

Conversation

nathanfranke
Copy link
Contributor

@nathanfranke nathanfranke commented Sep 1, 2020

2020-09-01 16-20-53 mp4

Bugsquad edit: Fixes #41762.

@akien-mga akien-mga added this to the 4.0 milestone Sep 1, 2020
@akien-mga
Copy link
Member

CC @EricEzaM

@EricEzaM
Copy link
Contributor

EricEzaM commented Sep 1, 2020

Hi @nathanfranke, @akien-mga.

Unfortunately I cannot reproduce the issue on my PR... not sure what is going on - maybe it is a platform specific issue? Anyway, this PR breaks what this piece of code tries to solve - not selecting items when trying to select the scrollbar. See below video of my test.
scrollbar_selects

So, the reason this code is required is because of the below situation. The scroll container just renders the scrollbar above the content. This is why I used the control rect initially, as it already accounts for the margin container. However, the scroll container rect would also work, as in my review below.
popup-menu-diagram

Copy link
Contributor

@EricEzaM EricEzaM left a comment

Choose a reason for hiding this comment

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

These changes do simplify the code quite a bit, thanks for that! I have suggested 2 options below - either work on my system. A good way to test the scrollbar is by going to the below button and changing the file dialog filter, since it has so many options.

image

scene/gui/popup_menu.cpp Outdated Show resolved Hide resolved
scene/gui/popup_menu.cpp Show resolved Hide resolved
@nathanfranke nathanfranke force-pushed the fix-popupmenu-unresponsive branch from ef06ef1 to 034e123 Compare September 2, 2020 06:24
@nathanfranke
Copy link
Contributor Author

I chose option "A" since it seemed more elegant 😄

@EricEzaM
Copy link
Contributor

EricEzaM commented Sep 2, 2020

All good on my end after a test, thanks!

@akien-mga
Copy link
Member

Also fixes #41762.

@akien-mga akien-mga merged commit 21da779 into godotengine:master Sep 4, 2020
@akien-mga
Copy link
Member

Thanks!

@nathanfranke nathanfranke deleted the fix-popupmenu-unresponsive branch December 9, 2021 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

X11: Can't add effects to Audio Bus, selection impossible in popup
3 participants