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

Add editor shortcuts to toggle bottom panel visibility #88081

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Feb 7, 2024

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
Copy link
Contributor

Mickeon commented Feb 8, 2024

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
Copy link
Contributor

kitbdev commented Feb 8, 2024

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
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
Copy link
Member Author

Calinou commented Feb 8, 2024

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
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
Copy link
Contributor

Mickeon commented Feb 11, 2024

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
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
Copy link
Member Author

Calinou commented Feb 16, 2024

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
Copy link
Member

KoBeWi commented Feb 27, 2024

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
Copy link
Member Author

Calinou commented Feb 28, 2024

Needs rebase.

Rebased and tested again, it works as expected.

@akien-mga
Copy link
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
@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
Copy link
Contributor

viksl commented Mar 4, 2024

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?

Copy link
Member

@KoBeWi KoBeWi left a comment

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
Copy link
Member Author

Calinou commented Mar 4, 2024

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
Copy link
Member

akien-mga commented Mar 5, 2024

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
Copy link
Member Author

Calinou commented Mar 5, 2024

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
Copy link
Member

KoBeWi commented Mar 5, 2024

I see the shortucts are still there 🤔

@akien-mga
Copy link
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
editor/editor_plugin.h Outdated Show resolved Hide resolved
editor/editor_plugin.compat.inc Outdated Show resolved Hide resolved
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
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Calinou Calinou deleted the editor-add-bottom-panel-shortcuts branch March 6, 2024 22:01
@njt1982
Copy link
Contributor

njt1982 commented Sep 27, 2024

@Calinou
Copy link
Member Author

Calinou commented Oct 4, 2024

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]