Skip to content

Conversation

@Kontrabant
Copy link
Contributor

By default, popups are automatically constrained to be completely within display bounds, so as not to cut off information and result in an unusable menu, or unreadable tool tip. In some cases, however, this is not wanted, so a property to toggle this behavior is added.

There are also cases where the client may not want a popup menu to implicitly grab the keyboard focus when shown, as is the default behavior, so popup menus now respect the focusable flag/property, as well as being able to toggle focus grabbing via SDL_SetWindowFocusable().

Much of the general popup focus code was identical between backends, and the changes here would have introduced even more verbatim code, so the popup focus code was factored out. There can probably be further simplification here, but care has to be taken.

Fixes #12533


// Transfer keyboard focus back to the parent from a grabbing popup.
if ((window->flags & SDL_WINDOW_POPUP_MENU) && !(window->flags & SDL_WINDOW_NOT_FOCUSABLE)) {
SDL_Window *new_focus;
Copy link
Collaborator

Choose a reason for hiding this comment

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

whitespace

Copy link
Collaborator

@slouken slouken left a comment

Choose a reason for hiding this comment

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

These seem good to me. The changes are a little involved so I'd leave this on main for now.

By default, popups are automatically constrained to be completely within display bounds, so as not to cut off information and result in an unusable menu, or unreadable tooltip. In some cases, however, this is not wanted, so a property to toggle this behavior is added.

There are also cases where the client may not want a popup menu to implicitly grab the keyboard focus, as is the default behavior, so popup menus now respect the focusable flag/property, as well as being able to toggle focus grabbing via SDL_SetWindowFocusable().
@Kontrabant
Copy link
Contributor Author

Did another round of testing and this seems good to go.

The changes are a little involved so I'd leave this on main for now.

I agree, and this is more of a feature addition than a bug fix anyways.

@Kontrabant Kontrabant merged commit b871ac0 into libsdl-org:main Apr 29, 2025
39 checks passed
@Kontrabant Kontrabant deleted the popup_work branch April 29, 2025 16:02
@GoaLitiuM
Copy link

GoaLitiuM commented Jun 17, 2025

Would it be possible to include this fix in next 3.2.X release? I tried briefly cherry-pick this on top of 3.2.X changes and haven't had no issues with it on Wayland so far.

@Kontrabant
Copy link
Contributor Author

I'm not opposed to it if someone has a use for it. Would you be willing to give things a quick check on the other platforms as well? Your use cases do a good job at shaking out bugs.

@GoaLitiuM
Copy link

The changes seems OK after testing these on X11, Wayland and Windows. I can't verify the changes on macOS for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Why add display area monitoring for setting the position of a child window?

3 participants