Skip to content

Add editor shortcuts to toggle bottom panel visibility#88081

Merged
akien-mga merged 1 commit into
godotengine:masterfrom
Calinou:editor-add-bottom-panel-shortcuts
Mar 5, 2024
Merged

Add editor shortcuts to toggle bottom panel visibility#88081
akien-mga merged 1 commit into
godotengine:masterfrom
Calinou:editor-add-bottom-panel-shortcuts

Conversation

@Calinou

@Calinou Calinou commented Feb 7, 2024

Copy link
Copy Markdown
Member

Default shortcuts typically use the first or second letter of each word (with a few exceptions such as TileMap and TileSet to ensure consistency). Editor plugins can also specify shortcuts for their own bottom panels.

This also adds also a new shortcut to toggle the last opened bottom panel. On editor startup, this defaults to the first panel in the list (which is the Output panel).

All shortcuts are nearly organized in their own section of the Editor Settings Shortcuts dialog for easy access, and are also available in the command palette:

image

One issue is that the individual toggle shortcuts don't work if you're currently focusing on the Tree node in either the scene tree dock or FileSystem dock. Shortcuts will work if you're focused on their filter LineEdits though. I've tried to use tb->set_shortcut_context(nullptr) to no avail.

Preview

editor_bottom_panel_shortcuts.mp4

The shortcut appears in the tooltip when the button is hovered:

image

@Mickeon

Mickeon commented Feb 8, 2024

Copy link
Copy Markdown
Member

I feel like a default shortcut set for every single one of those panels is overkill. Outside of the always-present 5 ones, it is also extremely unlikely that all these panels will be open at the same time

If it is later planned to use any of those Alt combinations for anything else it becomes much harder to argue with (Script Editor's CTRL + D is already a big example right now).

@kitbdev

kitbdev commented Feb 8, 2024

Copy link
Copy Markdown
Contributor

Should we add them to the command palette as well? Since there are commands to switch main screens.

@Calinou Calinou force-pushed the editor-add-bottom-panel-shortcuts branch 2 times, most recently from 1cb950c to 792d36b Compare February 8, 2024 10:51
@passivestar

Copy link
Copy Markdown
Contributor

Yeah I agree with @Mickeon, it's probably best to leave all of the non-persistent buttons unbound and let the users bind what they need. For example I'd rarely need a hotkey for TileMap just because I don't make 2D games, Replication is only needed if you make multiplayer games, etc etc

@Calinou

Calinou commented Feb 8, 2024

Copy link
Copy Markdown
Member Author

If it is later planned to use any of those Alt combinations for anything else it becomes much harder to argue with (Script Editor's CTRL + D is already a big example right now).

I can change the permanent bottom panels' shortcuts to take priority over the temporary ones (in the sense that permanent bottom panels would always use the first letter as their shortcut). By the way, perhaps permanent bottom panels should be displayed differently from temporary bottom panels.

Regarding shortcuts that only use Alt as a modifier, we don't have a lot of those right now. We'd probably have much more if we had proper support for accelerators / access keys, but there are no known plans to implement those in the short term.

@ajreckof

Copy link
Copy Markdown
Member

So first I'm not sure if this a problem per se but they are two bottom panel associated with Alt+T
The second is a real problem. On mac Option + Letter will result in special symbols.
Option + N -> ~
Option + T -> †
Option + A -> æ
Option + D -> ∂
Option + O -> œ
Option + L -> ¬
Option + E -> ê
Option + R -> ®
Option + H -> Ì
Option + I -> î
Option + P -> π
Option + T -> †
Option + M -> µ
Option + S -> Ò
I think out of all of them only the R will not trigger a writing this means whenever the script main panel is opened using those shortcuts will add some random characters in the middle of your scripts. On the other end preventing them could be catastrophic for example Option + N and Option + O are two that I use a lot when writing strings. I feel the best solution should to have different shortcuts for Mac Not 100% sure how it was handled for other Alt + Letter shortcuts that might have been added previously.

@Mickeon

Mickeon commented Feb 11, 2024

Copy link
Copy Markdown
Member

There are some default shortcuts on Mac that are different for similar reasons, so they would need to either be something else on that platform or stripped out entirely.

@Cammymoop

Copy link
Copy Markdown
Contributor

Can we include a shortcut to toggle? Hide current panel if visible, otherwise show last open panel. Doesn't need a default mapping but I think it's worth one. Similar thing in VSCode is ctl-j not sure why J but it does seem free to map.

@Calinou

Calinou commented Feb 16, 2024

Copy link
Copy Markdown
Member Author

Should we add them to the command palette as well? Since there are commands to switch main screens.

Done. I've also renamed shortcuts to make them easier to search.

Can we include a shortcut to toggle? Hide current panel if visible, otherwise show last open panel. Doesn't need a default mapping but I think it's worth one. Similar thing in VSCode is ctl-j not sure why J but it does seem free to map.

Good idea; I've added Ctrl + J to toggle the last opened bottom panel.

@Calinou Calinou force-pushed the editor-add-bottom-panel-shortcuts branch from 792d36b to 03c4384 Compare February 16, 2024 17:48
@KoBeWi

KoBeWi commented Feb 27, 2024

Copy link
Copy Markdown
Member

Needs rebase.

@Calinou Calinou force-pushed the editor-add-bottom-panel-shortcuts branch from 03c4384 to 19caad0 Compare February 28, 2024 19:00
@Calinou Calinou requested review from a team as code owners February 28, 2024 19:00
@Calinou

Calinou commented Feb 28, 2024

Copy link
Copy Markdown
Member Author

Needs rebase.

Rebased and tested again, it works as expected.

@akien-mga

Copy link
Copy Markdown
Member

Needs a compat method binding:

Validate extension JSON: Error: Field 'classes/EditorPlugin/methods/add_control_to_bottom_panel/arguments': size changed value in new API, from 2 to 3.
Validate extension JSON: Error: Hash changed for 'classes/EditorPlugin/methods/add_control_to_bottom_panel', from D22B1750 to 069E37CD. This means that the function has changed and no compatibility function was provided.

@Calinou Calinou force-pushed the editor-add-bottom-panel-shortcuts branch from 19caad0 to f5994a6 Compare February 28, 2024 23:13
@Calinou Calinou requested review from a team as code owners February 28, 2024 23:13
Comment thread editor/plugins/tiles/tiles_editor_plugin.cpp Outdated
@Calinou Calinou force-pushed the editor-add-bottom-panel-shortcuts branch 2 times, most recently from 8411a45 to 71f7511 Compare February 29, 2024 19:38
@Calinou Calinou force-pushed the editor-add-bottom-panel-shortcuts branch from 71f7511 to e7ee515 Compare March 4, 2024 15:48
@viksl

viksl commented Mar 4, 2024

Copy link
Copy Markdown
Contributor

I'm getting some merge conflicts when trying this PR with master, is this meant for 4.3 rebase or some later 4.X version perhaps?

Comment thread editor/editor_plugin.compat.inc Outdated
Comment thread misc/extension_api_validation/4.2-stable.expected Outdated
Comment thread editor/editor_dock_manager.cpp Outdated

@KoBeWi KoBeWi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Still not a fan of adding that many shortcuts, but it has some merits it seems. Though if shifting button positions are the problem, we could rework it to make them more stable (e.g. always add new buttons at the end and allow rearranging them).

Code-wise looks fine.

@Calinou Calinou force-pushed the editor-add-bottom-panel-shortcuts branch from e7ee515 to 1cdab2d Compare March 4, 2024 23:19
@Calinou

Calinou commented Mar 4, 2024

Copy link
Copy Markdown
Member Author

Rebased and squashed with the review suggestions incorporated.

@Calinou Calinou force-pushed the editor-add-bottom-panel-shortcuts branch from 1cdab2d to 9e1a7ad Compare March 4, 2024 23:19
@akien-mga

akien-mga commented Mar 5, 2024

Copy link
Copy Markdown
Member

I feel like a default shortcut set for every single one of those panels is overkill. Outside of the always-present 5 ones, it is also extremely unlikely that all these panels will be open at the same time

If it is later planned to use any of those Alt combinations for anything else it becomes much harder to argue with (Script Editor's CTRL + D is already a big example right now).

I want to echo that, I feel adding a default unique hotkey to every single panel, even some which are very contextual and only appear when a specific node is selected (like Replication, etc.) is overkill.

I would only set default hotkeys for the panels which are always present: Output, Debugger, Audio, Animation, Shader Editor, and now FileSystem since it can also be moved to the bottom panel.

The rest can have shortcuts without a default binding, so users who need them can set their own preferred bindings (e.g. for TileMap / TileSet for users who do tiles-based 2D games, or Replication for users making multiplayer games with the high level multiplayer API).

@Calinou

Calinou commented Mar 5, 2024

Copy link
Copy Markdown
Member Author

I would only set default hotkeys for the panels which are always present: Output, Debugger, Audio, Animation, Shader Editor, and now FileSystem since it can also be moved to the bottom panel.

The rest can have shortcuts without a default binding, so users who need them can set their own preferred bindings (e.g. for TileMap / TileSet for users who do tiles-based 2D games, or Replication for users making multiplayer games with the high level multiplayer API).

Done.

@KoBeWi

KoBeWi commented Mar 5, 2024

Copy link
Copy Markdown
Member

I see the shortucts are still there 🤔

@akien-mga

Copy link
Copy Markdown
Member

Hm yes you forgot to push :P

@Calinou Calinou force-pushed the editor-add-bottom-panel-shortcuts branch from 9e1a7ad to 49c79b8 Compare March 5, 2024 14:10
Comment thread editor/editor_plugin.h Outdated
Comment thread editor/editor_plugin.compat.inc Outdated
Default shortcuts use the first or second letter of each word.

This also adds a new shortcut to toggle the last opened bottom panel.
On editor startup, this defaults to the first panel in the list
(which is the Output panel).
@Calinou Calinou force-pushed the editor-add-bottom-panel-shortcuts branch from 49c79b8 to 8221e75 Compare March 5, 2024 14:53
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Mar 5, 2024
@akien-mga akien-mga merged commit 4bb2193 into godotengine:master Mar 5, 2024
@akien-mga

Copy link
Copy Markdown
Member

Thanks!

@Calinou Calinou deleted the editor-add-bottom-panel-shortcuts branch March 6, 2024 22:01
@kitbdev kitbdev mentioned this pull request Apr 5, 2024
@njt1982

njt1982 commented Sep 27, 2024

Copy link
Copy Markdown
Contributor

@Calinou

Calinou commented Oct 4, 2024

Copy link
Copy Markdown
Member Author

Does docs.godotengine.org/en/4.3/tutorials/editor/default_key_mapping.html need updating?

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.

Add hotkeys for the editor's bottom panels [output, debugger, search results, audio, animation]