-
-
Notifications
You must be signed in to change notification settings - Fork 138
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 passing parent_window=None
to message_box
#2723
Fix passing parent_window=None
to message_box
#2723
Conversation
Well, I guess the CI really doesn't like |
ada3ef2
to
950ec13
Compare
Perhaps this should be a part of the research process for this PR? |
Well, in my understanding all this does is sort out which thread the messagebox will be opened in. That is, if you may not wish to block your process running on the main thread, you could create a new window in a child thread and then pass that window as the parent to the message box function (that you also probably need to call from that thread). Otherwise even if you call it in a thread, but don't specify a parent window, it will block the main thread.
https://wiki.libsdl.org/SDL2/SDL_ShowSimpleMessageBox There's a slight issue, unrelated to this PR. Something must be broken in the implementation because this does not work the way I have explained (and believe it should be the way it should work) with our implementation. It's beyond the scope of this PR though. |
As I understand it, setting the parent window of a modal dialog makes it so the window is greyed out until the modal dialog is closed/dealt with. It's not a thing the toolkit does (SDL, GTK, Qt), but something that must be actively supported by the desktop environment/window manager/compositor. The window manager might decide to always focus automatically on the dialog when you alt-tab to the parent window or click in the talk bar, to group the dialog together with the parent window in the dock/task bar, to grey out the parent window, and so on. If there is no parent window set, a tiling window manager might just show the dialog centered and on top of the tiling area. Otherwise it could show the window on top of the parent window. Given that wayland does not allow windows to position themselves, this information is needed for the dialog to be spawned on top of the application and on the right monitor. |
The thing you should take away from my previous comment is it depends on whether you are on Win32, Quartz, X11, Wayland, or Android. Consoles that don't have a classic "window manager" might still have limited support for modal dialogs, the same way and Android and iOS do. |
This is what parent Window does on Windows (from SDL source): /* If we have a parent window, get the Instance and HWND for them
so that our little dialog gets exclusive focus at all times. */
if (messageboxdata->window) {
ParentWindow = messageboxdata->window->driverdata->hwnd;
} And from a Microsoft blog on the attribute:
Looking at the SDL code it looks like they don't touch the 'position the window relative to the parent' flag so unless it is on by default for this window type you would just get the modal effect from setting the parent window on Windows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Currently passing
parent_window=None
tomessage_box
raises aTypeError
. This should fix that.What exactly does the
parent_window
even do? The SDL docs don't really explain it either from what I could find. The only notable thing was thatmessage_box
should be called from the thread that the parent window was created in, unless it's None, in which case, it should be called from the main thread. Should that be added to the docs as well? Can thatparent_window
param in the docs be uncommented now?Should the test be an interactive one instead?