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

Allow docks to be closed and opened #89017

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

kitbdev
Copy link
Contributor

@kitbdev kitbdev commented Feb 29, 2024

Docks can be closed through their context menu, and reopened through the new Window menu.
If a dock is already open, clicking on it in the Window menu will focus on it.
Dock layouts can be saved with closed tabs. This helps with some of the use cases in godotengine/godot-proposals#1441.

Added dock slot DOCK_SLOT_NONE. Using this with add_control_to_dock allows the dock to not be opened immediately, it will be closed by default. When it is opened from the Window menu, it will appear floating.

Internally, renamed add_control_to_dock to add_dock and remove_control_from_dock to remove_dock. This is because I want to unify the language used better, the Control that is being added and removed is the dock, and it is added to a dock slot/dock container. However, I didn't change it in the editor plugin since it would break compat. I want to add it as the new names in EditorInterface in a future PR.

Closed docks aren't unloaded, they are just removed from any TabContainers and hidden.

After #88081, shortcuts can be added to open docks.

image

image

Updated (import closed, history disabled):

image

@Mickeon
Copy link
Contributor

Mickeon commented Feb 29, 2024

What I can say with confidence is that:

  • The menu could be called "View" as other programs seem to do;
  • This needs checkboxes for every item in that menu. Visual Studio Code for example:

@KoBeWi
Copy link
Member

KoBeWi commented Feb 29, 2024

The menu could be called "View" as other programs seem to do;

In GIMP the menu is called "Windows".

I think this doesn't really deserve a separate top level menu. It could be inside Editor, e.g. next to the layout button.

@kitbdev kitbdev force-pushed the add-close-docks branch 2 times, most recently from 6aa65ee to a335b09 Compare March 24, 2024 02:40
@kitbdev
Copy link
Contributor Author

kitbdev commented Mar 24, 2024

Rebased since #88003 is merged.
I moved it into the Editor menu next to the layouts.
Since its no longer top level I changed the name to 'Editor Docks', rather than 'View' or 'Window'.

Added checkboxes. This helps see what is closed and what is not.
Originally I wanted the action on clicking it to just open and focus on the dock, but the checkbox implies that clicking it will close the dock, so I'll probably change the functionality to that.
When focusing on the dock, the 2d/3d editor will probably steal the focus anyway.

Incorporated the shortcut, but the shortcut name should probably be changed.
Or maybe I should use different shortcuts, since the functionality is different.

I think I should also make the disabled docks from EditorFeatures be disabled items instead of just not present.

image

@KoBeWi
Copy link
Member

KoBeWi commented Mar 25, 2024

Yeah checkbox totally implies that you can toggle it, so it should display differently. You could make open docks show with bold text or using accent color, or show some other icon than checkbox. Maybe eye icon for open docks (to imply focus) and "open" icon for closed docks, to imply opening?

As for the shortcut, you could make the item display the dock's name instead of shortcut name. I think it's fine to reuse it, because it works correctly (i.e. reopens/focuses dock on the side and toggles dock on bottom).

@kitbdev
Copy link
Contributor Author

kitbdev commented Mar 27, 2024

I don't think I can make them bold or colored, PopupMenu items don't support BBCode and they share the same theme. The open eye icon (GuiVisibilityVisible) makes it look like clicking will make it hidden, since that is what it does in the Scene Dock. The only open icon I could find was file open (Load) but that didn't seem right for opening docks.

Here's what it looks like with Search icons for open docks (to 'find' them) and closed eye icons for closed ones.
I also made docks disabled in EditorFeatures be disabled, and set the name for the shortcut.
I tried adding tooltips, but #73926 is causing issues, so I'll remove them until that is fixed.

image

Let me know if there are better icons or if I should try anything else.
Or I can make it toggle open/closed and use checkboxes.

For reference, what other engines do: Unity has the menu items always open a new instance of the dock. Unreal has checkboxes to open and close docks, and they are in submenus for a fixed number of multiple instances.

@KoBeWi
Copy link
Member

KoBeWi commented Mar 29, 2024

Hmmm, in GIMP the docks menu simply displays the list of docks with their own icons, without providing information whether they are currently opened. Not sure how it is in other software.
Unique icon per dock is obviously overkill (at least for now), but maybe they could all use the same icon and for displaying status, icon modulate can be used (with inactive docks being gray). As for what icon to use, I think Window is fine.

@kitbdev
Copy link
Contributor Author

kitbdev commented Mar 29, 2024

Here is how it looks with the Window icon with modulate:

image

@KoBeWi
Copy link
Member

KoBeWi commented Mar 29, 2024

IMO looks fine.

Close button needs icon:
image

@kitbdev
Copy link
Contributor Author

kitbdev commented Mar 29, 2024

Unexposed dock slot none and added close icon:

image

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (rebased on top of master 29b3d9e), it works as expected.

Code looks good to me as well (outside of the suggestions).

@kitbdev
Copy link
Contributor Author

kitbdev commented Mar 30, 2024

Updated to use opacity.

Screenshot 2024-03-29 215143

Also, I realized that changing the theme wasn't updating it so I connected it to the theme_changed signal.

@KoBeWi
Copy link
Member

KoBeWi commented Mar 30, 2024

You should use NOTIFICATION_THEME_CHANGED instead of signal (also remember that it's received when node enters tree, so avoid unnecessary update).

@kitbdev
Copy link
Contributor Author

kitbdev commented Mar 30, 2024

You should use NOTIFICATION_THEME_CHANGED instead of signal (also remember that it's received when node enters tree, so avoid unnecessary update).

EditorDockManager doesn't have get that notification, and putting it into the existing one in DockContextPopup feels weird since its an unrelated child class.
Maybe I should change when the menu gets updated and only update it on about_to_popup?

@KoBeWi
Copy link
Member

KoBeWi commented Mar 30, 2024

EditorDockManager doesn't have get that notification

Then signal is fine.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Apr 4, 2024
@akien-mga akien-mga merged commit b5369ee into godotengine:master Apr 4, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@kitbdev kitbdev deleted the add-close-docks branch April 4, 2024 13:20
@ModuleCode
Copy link

ModuleCode commented May 2, 2024

I think the menu bar should stay after clicking, otherwise when I adjust multiple times, I will have to open the menu bar repeatedly, which will become very troublesome.

@kitbdev
Copy link
Contributor Author

kitbdev commented May 2, 2024

Currently, clicking on the menu for an open dock only focuses on the dock, so the menu should close in that case.
But for opening closed docks, we can make it stay open.

if (!dock.value.open) {
docks_menu->set_item_icon_modulate(id, closed_icon_color_mod);
}
docks_menu->set_item_disabled(id, !dock.value.enabled);
Copy link
Contributor

Choose a reason for hiding this comment

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

Features that are disabled via Feature Profiles usually get hidden in the UI, not disabled. Seems like a simple change to skip adding the menu item in this case, instead of disabling.

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.

Allow opening and closing any editor dock
8 participants