-
-
Notifications
You must be signed in to change notification settings - Fork 397
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
full support for auto tallscreen/widescreen panel layout #458
full support for auto tallscreen/widescreen panel layout #458
Conversation
You are welcome! I was afraid that you maybe wouldn't appreciate tallscreen support. I have made the changes, except for the color picker, which I overlooked. I am a little bit hesitant to move too many elements around, as later changes in the UI would also require the script to change. Moving the color picker is a bit ... hmm, I really don't know! Check out how it is so far: I later noticed that it is most important to minimize the mouse distance to the tools. With the tools tool panel at the bottom, it is a tad bit longer, but you will make the same downward movement every time, which I believe is more intuitive. If you still really want the color picker above the CanvasPreview, I can do it as well. |
I'm a fan of the layers being moved on the bottom. It looks good so far! The tools may need a second column in the future and that would greatly decrease the canvas area visible in vertical screens. While the colour picker looks good when the preview is removed, when the preview is there it has empty space under it and it seems unbalanced (visually). I would suggest having it at its current position without the preview and when the preview is enabled have it over it? As for the tools being moved down there, it may even require less mouse movements if you need to choose a tool and change its tool options so I think it's ideal now! |
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.
Tested it more and it seems to be working well. Left a couple more comments and after they get resolved, it should be ready to get merged!
src/Classes/Project.gd
Outdated
for j in Global.PanelLayout.values(): | ||
Global.panel_layout_submenu.set_item_checked(j, j == panel_layout) |
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.
Is the panel layout meant to be a per-project setting? Because if I, for example, select Tallscreen and then switch tabs, the submenu item that is checked returns to Auto, but the UI does not change.
Per-project setting means that the UI would change depending on the project, each project would have its own panel layout. If this is something we want, then this code also needs to call change_ui_layout()
from Main.gd. If this is something we don't want, then these 3 new lines of code should be removed completely. Personally, I don't think it should be a per-project setting as it's a UI change that may confuse the users.
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.
It will now save and restore the layout if you close and reopen the window.
On that matter: shouldn't most other global view settings also save and restore? If the users sets a different window transparency or hides the animation timeline for example. Maybe not tile mode though and especially not mirror view.
Honestly, when I tied panel_layout to current_project I wasn't sure whether or not saving the view settings was broken. So I didn't think it through any further. Sorry about that.
src/UI/TopMenuContainer.gd
Outdated
@@ -314,6 +323,13 @@ func window_transparency_submenu_id_pressed(id : float) -> void: | |||
window_transparency(id/10) | |||
|
|||
|
|||
func panel_layout_submenu_id_pressed(id : int) -> void: | |||
Global.current_project.panel_layout = id | |||
for i in Global.TileMode.values(): |
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.
I think this line should be for i in Global.PanelLayout.values():
.
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.
fixed
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.
Check out new commit.
src/Classes/Project.gd
Outdated
for j in Global.PanelLayout.values(): | ||
Global.panel_layout_submenu.set_item_checked(j, j == panel_layout) |
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.
It will now save and restore the layout if you close and reopen the window.
On that matter: shouldn't most other global view settings also save and restore? If the users sets a different window transparency or hides the animation timeline for example. Maybe not tile mode though and especially not mirror view.
Honestly, when I tied panel_layout to current_project I wasn't sure whether or not saving the view settings was broken. So I didn't think it through any further. Sorry about that.
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. Thank you!
If the window aspect ratio is lowered below 3:4, then the panel layout will switch to "tallscreen", which entails that the right toolbar will be moved to the bottom instead. Of course the reverse is also true. The user can manually override the panel layout behavior in View -> Panel Layout.
Tested with Godot 3.2.1 on Linux and HTML5 with Chrome. Should be pretty error-proof though.
This illustrates the maximum usable drawing area with the different modes: