-
-
Notifications
You must be signed in to change notification settings - Fork 21.3k
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
Make visual shader editor create node window a popup #99568
base: master
Are you sure you want to change the base?
Make visual shader editor create node window a popup #99568
Conversation
It works on macos. Also looks better than a window imo btw is there a reason it's not a PopupPanel like color pickers and most small editor popups? it could have rounded corners and OS-agnostic shadow after #91333 is merged if it was. (edit: nevermind I looked at code and it is a PopupPanel) |
I tested casually on Windows 10, and this works as expected, if not better. I was really expecting some jankiness from this change. I do wish the new popup was resizeable. Prior to this PR, I would often resize the window to about 2x the vertical size you have set for the popup. Even if that's not possible, this PR is a net UX improvement, IMO. I did find one very edge case behavior, but it is exposing an existing edge case, not adding a new one. When:
when the Note that this is an existing behavior. In these videos you can see that both 4.3 and this PR have this behavior for the right-click context menu (when a node is selected), but in this PR, the behavior is also present for the In these videos, the floating window is split between the monitors. UI popups spawn at the rightmost edge of the left monitor. Note that popups spawn to the left of the mouse. 4.3: 7xo5BSaKXm.mp4This PR: rXRpov8ZGr.mp4I don't think this is something that needs to be fixed, since it is an edge case and a preexisting behavior. |
6a6fce5
to
c624e52
Compare
Yeah, this looks like a bug in the popup windows implementation and not something really related specifically to this patch, it'd require a separate bug report (is there any?). Personally i do not have a multimonitor setup currently (i have a spare monitor somewhere but not the space to install it) so i can't even test this. |
I pushed an update to the PR that makes the create shader node UI to appear as a popup only when right clicking but use a dialog when using the popup menu "Add Node" or the button at the top left corner of the editor. You still can't resize the popup (needs some GUI control for this) but it does use the last size of the dialog so there is at least that indirect method. |
This makes the Visual Shader editor's "Create Shader Node" window to be a popup instead of a dialog, thus being easily dismissed by clicking outside the window like in most other DCC applications that use a node editor. It addresses godotengine/godot-proposals#11213. It also solves the issue where the window would appear outside the screen area.
Please note that i have only tested this on Linux and X11.
The main negative with this patch is that unlike the window, the popup is not resizable and instead a 300px (scaled by
EDSCALE
) minimum width is used to ensure that the node names are visible. While it is possible to make the window resizable by setting theWindow::FLAG_RESIZE_DISABLED
tofalse
, there is no visual indicator that the window is resizable and the window closes as soon as the resize finishes (seems a race condition between the window manager and the X11 mouse handling code for popups). Ideally there should be a control that can be placed on popups (like a small triangle at the bottom right corner, as that is common in various toolkits) to handle client-side resizing, but i can't find anything like that in Godot.A potential alternative would be to use the popup-based approach in this PR for right clicking but use a dialog when "Add Node" is selected from the popup menu. This will most likely require a larger change though as from what i can tell a window can't switch "modes" between being a Popup and a Dialog.