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

Improve Keybind/Hotkey Handling #5145

Open
Tacodiva opened this issue Sep 29, 2022 · 45 comments
Open

Improve Keybind/Hotkey Handling #5145

Tacodiva opened this issue Sep 29, 2022 · 45 comments
Labels
priority: 3 Medium priority. Includes bugs and useful features scope: addon.json About the addon.json file structure scope: webpages Related to the web pages (settings page, pop-up, etc) type: enhancement New feature for the project
Milestone

Comments

@Tacodiva
Copy link
Member

As I see it, currently, the way SA handles keybindings is not ideal.

  • There is no way to rebind keys on anything
  • There is no way to keybind a lot of the vanilla behaviors without an addon ( and addons like 'duplicate scripts with alt key' don't support changing the key either)
  • The handling of keybinds must be implemented by every addon individually

The only real progress that has been made on this was the addition of the currently unused 'key' setting type in #793.

========================

Not sure if an issue is the best place to put this wall of text, but here we go.

I've put a bit of though into it and what follows is the solution I think would work best to mitigate all of these problems. It involves some substantial changes and contains a large amount of what I consider my personal opinion on how things should be done, so I want feedback before I try and actually do any of it. I also cannot do all of this by myself (I have no clue about vue.js 😅 ) so I would need some help.

The idea of keybinds as a setting is good but the current implementation has issues. Right now, when an addons uses settings.get() on a 'key' setting, you get a string like Ctrl + Shift + F, for an addon to use this it has to be parsed then code has to be written to detect that keybind. Instead, for key settings, settings.get() should return an object that contains helpful things for implementing the keybind, such as an isPressed property, a way to get the keys in the bind without needing to parse a string and events that can be subscribed to for when the keybind is pressed and when it is released. I think this would have to be implemented through an additional script which gets injected to every page and listens for the keybinds of the addons that are enabled on that page. The 'key' settings also needs more options for constraining what types of keys can be bound to it.

As well as these core changes, a new addon (maybe enabled by default) would need to be added for the keybind manager. This addon adds a new popup and tab which allows you to search keybinds from all addons by name or by the actual keybinding and change them from there (I'm thinking something like the keyboard shortcuts menu in vscode). It would also provide keybindings for vanilla scratch features that don't have one, so addons like 'duplicate scripts with alt key' or #705 could be put into this one. Keybinds should be mostly or all empty by default so loads of keybinds could be added without worrying about users having every key on their keyboard doing something they didn't set it to.
This addon could be split in two, one for the manager and another for providing keybinds for vanilla features but I'm not sure why you'd want one without the other.

Finally, I think the key setting appearing in the addons main settings should be optional (so these keybinds would only be visible in the manager). This will allow addons like devtools or the debugger to not have to clutter their settings section with 1,000 hotkey options. Obviously, all current addons which have keybindings should be moved over to the new system so their rebindable and play nice with the other addons.

This could be done in stages, firstly making the API better, than adding the manager UI, then moving all current addons over to the new system and finally adding keybinds for vanilla features into the manager.

@Tacodiva Tacodiva added the status: needs triage Needs triaging by putting relevant labels label Sep 29, 2022
@mxmou mxmou added type: enhancement New feature for the project scope: addon.json About the addon.json file structure scope: webpages Related to the web pages (settings page, pop-up, etc) and removed status: needs triage Needs triaging by putting relevant labels labels Sep 29, 2022
@Samq64
Copy link
Member

Samq64 commented Sep 29, 2022

Wait, there's already an undocumented hotkey setting type?

@Tacodiva
Copy link
Member Author

Wait, there's already an undocumented hotkey setting type?
@Samq64

Yes there is, I'm pretty sure it's not even in the manifest schema validator, but it's there.

@BroJac5246
Copy link
Contributor

Is this related to #4946?

@CST1229
Copy link
Member

CST1229 commented Sep 29, 2022

I think keyboard shortcuts and hotkeys should be implemented as a separate thing from settings (with its own property in the addon.json and own API that does things like fire events for keybinds), and keyboard shortcuts for vanilla elements would be be in a separate addon.
Keybinds would be shown and modifiable in the addon card, below the other settings.
Alongside that, you would also be able to view and modify all keybinds in More Settings (and maybe in the popup).

@lisa-wolfgang
Copy link
Member

In order to create a truly comprehensive hotkey addon API, we should also have a way to define target elements for the click action of a hotkey. For example, "Alt + Green Flag" would be defined as "Alt + click event on the CSS selector for the green flag".

@lisa-wolfgang
Copy link
Member

Another benefit of having a universal hotkey manager is that it could automatically check for hotkey conflicts (whether it be with the OS, the browser, Scratch, or another addon) and report that to developers. If we had an accurate database, this would make it easy to select a non-problematic hotkey.

@Tacodiva
Copy link
Member Author

Tacodiva commented Oct 2, 2022

In order to create a truly comprehensive hotkey addon API, we should also have a way to define target elements for the click action of a hotkey. For example, "Alt + Green Flag" would be defined as "Alt + click event on the CSS selector for the green flag".

Would that really need to be a part of the API? If you can add listners to the hotkeys this can be done in one line with a lambda expression.
Something like

I'm sorry I misunderstood what you where saying-
I agree this is something that needs to get addressed but I'm not sure how to do the UI. How do we let the user input the 'green flag' button? If '+ green flag' is always there so the user just inputs alt, how do you change it so the keybind will trigger without clicking the flag (like pressing Ctrl + y for 60fps instead).

One possible option is have two hotkey settings, one with the + green flag and one without. This seems a bit unideal and it may be confusing why there's two hotkey options for the same thing.

@lisa-wolfgang
Copy link
Member

There could be a button alongside the regular hotkey input that has a "Click:" label to the left of it. We could do a text description on said button or even images.

@Tacodiva
Copy link
Member Author

Tacodiva commented Oct 3, 2022

I'm not sure it makes sense to make a whole system for this when I can't think of any other button we would put there other than green flag and stop (even stop I'm not sure what you'd use it for).

Another possible solution- we'll want to add support for binding multiple key combinations that map to the same setting anyways, maybe we could just have is so the first bind always has the '+ green flag' and the ones you add after that don't.

I guess I just can't see why you would ever want to change the button it's bound to and it seems like a lot of additional complication to a UI already has the potential to be confusing.

@lisa-wolfgang
Copy link
Member

We wouldn't have to provide a way to change the button that "click" is bound to -- that would be unnecessary complication for the user. All I meant was that the "click" action should be defined by the addon and shown as an option.

@Secret-chest
Copy link
Contributor

Sorry if this was mentioned before, but we should be able to set keybindings that use modifiers.

@WorldLanguages
Copy link
Member

Sorry if this was mentioned before, but we should be able to set keybindings that use modifiers.

Can you elaborate what you mean by "modifiers"?

@WorldLanguages
Copy link
Member

@Tacodude7729 Thanks for the writeup.

I also think it's important to mention that addons should be able to reference a keybind action, so users can easily change keybinds that are related to specific addons, even if the keybind is handled by a different addon.
For example, if we add a "pause project" keybind, the "pause" addon should have a way to include it as part of its settings page UI.

I also wonder what would happen to that keybind if the "pause" addon is disabled. Would users still expect it to work? Would we disable the keybind? IMO, it should still work - but if the user accidentally pressed the keybind, they might not know how to unpause the project because there's no visible UI to unpause (probably not a serious problem, but worth thinking).

@mxmou
Copy link
Member

mxmou commented Oct 6, 2022

@WorldLanguages It might be better to only allow addon hotkeys to be configured if the addon is enabled.

@lisa-wolfgang
Copy link
Member

Agreed -- if something is disabled in the settings, the user should be able to trust that it's disabled.

@WorldLanguages
Copy link
Member

It might be better to only allow addon hotkeys to be configured if the addon is enabled.

In that case, if we decide to add a dedicated "hotkeys" section to the settings page that shows all supported keybinds, then we would need to either hide hotkeys that are currently unavailable because the addon is disabled. Or, alternatively, mark them as disabled and make it clear to the user why.

Having keybinds that disappear might be a problem if we decide two separate actions can't have the same keybind. We would have to tell the user that the keybind can't be set because it's already used by another action, but we couldn't show which one.

@lisa-wolfgang
Copy link
Member

lisa-wolfgang commented Oct 6, 2022

Why wouldn't we allow the user to set a keyboard shortcut that's currently used by an addon they never plan to use?
We mentioned a fallback system at some point -- that would be handy for this scenario.

@Secret-chest
Copy link
Contributor

Sorry if this was mentioned before, but we should be able to set keybindings that use modifiers.

Can you elaborate what you mean by "modifiers"?

Control, Alt and Shift

@Tacodiva
Copy link
Member Author

Tacodiva commented Oct 7, 2022

It might be better to only allow addon hotkeys to be configured if the addon is enabled.

I agree but this solution is problematic for the pause hotkeys specifically, as they should be in both the debugger and in the pause button plugin. A user may want to use the hotkeys with only the debugger or only the pause button enabled. Additionally, it would be confusing if a user was looking at the debugger and wanted to hotkey unpausing but the setting was actually under the pause button and vice versa.
We could either have two hotkeys for the same thing, which could be confusing or have the two addons share the same hotkey setting somehow, and the hotkey would be enabled if either of the addons are enabled, which seems complex to implement.

@WorldLanguages
Copy link
Member

have the two addons share the same hotkey setting somehow

That's what I had in mind - some hotkeys are related to more than one addon at once.
I think we could create "mini-addons" that only handle actions, which Scratch Addons can let the user trigger by keybinds, or maybe even an "action menu", if we wish. This type of addon wouldn't (intentionally) make any changes until an action is triggered.

and the hotkey would be enabled if either of the addons are enabled, which seems complex to implement.

Using pause hotkey as an example, I think we have two alternatives, each one with its own dropbacks:

  1. Pause hotkey only works if either pause or debugger (or both) are enabled.
    The user might not expect this, but it's always easy to unpause the project by using the UI.
  2. Pause hotkey continues to work even if those two addons are disabled.
    However, if the user accidentally pressed the keybind, the project will look as if it "crashed", with no clear UI indication telling the user that the project has been paused.

Personally, I prefer option 2, and adding some type of UI notification in a corner that reads "project has been paused". An "undo" addon could possibly be added to the element, which unpauses the project when clicked.

@WorldLanguages
Copy link
Member

Note that what I just said would not apply for all types of keybinds.
For example, Duplicate script with Alt key would continue to be an addon, but would allow users to customize what key(s) should be held to consider the block should be duplicated.

@WorldLanguages
Copy link
Member

WorldLanguages commented Oct 7, 2022

In order to create a truly comprehensive hotkey addon API, we should also have a way to define target elements for the click action of a hotkey. For example, "Alt + Green Flag" would be defined as "Alt + click event on the CSS selector for the green flag".

We also need to consider "gotchas" like this one. Building an architecture to support many different types of keybinds, while preserving an easy to use user-facing UI, is not going to be easy.

Here's a non-comprehensive list of edge-cases and things to consider: (please tell me if there's more I haven't thought of)

  • Hotkeys related to more than one addon
  • Hotkeys which are "Alt+dragblock" or "Ctrl+greenflag"
  • Whether some types of keybinds will work, when all related addons are disabled
  • Categorizing different types of keybinds, depending on the context where they can be used (example: costume editor)
  • When to allow 2 actions to have the same keybind (example: we can't have 2 costume editor actions attached to the same key). How to handle the user UI/UX when there's one of these conflicts.
  • Whether pressing Ctrl+Shift+3 should trigger an action that is triggered with Shift+3
  • When to allow a single key press (example: letter A) to cause an action (vanilla Scratch editor already does this). Making sure users can still write these letters on inputs, without triggering the action.
  • Consider platform and browser-specific keybinds that are impossible to use, such as Ctrl+W, Alt+Click on chromebooks, etc.
  • See Add a Hotkey Manager #4946 for more

@WorldLanguages
Copy link
Member

It would be great to have a complete list of all vanilla Scratch keybinds.

@BroJac5246
Copy link
Contributor

Just gonna point out that #4946 is related.

@Tacodiva
Copy link
Member Author

Tacodiva commented Oct 7, 2022

I think we could create "mini-addons" that only handle actions, which Scratch Addons can let the user trigger by keybinds, or maybe even an "action menu", if we wish. This type of addon wouldn't (intentionally) make any changes until an action is triggered.

As written in the initial issue, I thought there could be one big addon that contains all the keybinds for vanilla scratch functionality. Are you saying that you want all of these keybinds to be in separate addons? That seems like it's going to be way too many addons.

Pause hotkey continues to work even if those two addons are disabled.

Where would the code for this go? The way I was thinking of implementing hotkeys as mentioned above would require the addons userscript to be ran to add the even listeners to the hotkey.
Another alternative- we could put pause hotkey into a separate addon (maybe even the vanilla scratch keybinds addon, even though it's not technically 'vanilla') and maybe put a note in the debugger and pause button addons that the hotkeys for pausing and unpausing are located elsewhere.

@Hans5958
Copy link
Member

Hans5958 commented Oct 8, 2022

Wait, there's already an undocumented hotkey setting type?
@Samq64

Yes there is, I'm pretty sure it's not even in the manifest schema validator, but it's there.

ScratchAddons/manifest-schema#24 lol

Also it's called the manifest schema, not the validator.

@WorldLanguages
Copy link
Member

Just gonna point out that #4946 is related.

Whoops, looks like we have 2 issues for the same thing, but discussion never started on the other issue. My bad!

@WorldLanguages
Copy link
Member

Another alternative- we could put pause hotkey into a separate addon (maybe even the vanilla scratch keybinds addon, even though it's not technically 'vanilla') and maybe put a note in the debugger and pause button addons that the hotkeys for pausing and unpausing are located elsewhere.

That's what I had in mind. But instead of using a "note", we could allow addons to reference keybinds which are technically handled by different addons, and the settings page would lie to the user and show it as part of the "wrong" addon(s).

@lisa-wolfgang
Copy link
Member

To summarize, this is what specifically is needed regarding hotkeys:

  • Helping the user remember hotkeys, both for vanilla Scratch and our addons
  • A centralized way for addons to hook into hotkeys (allowing for addons that share hotkeys) including support for click events on specified elements
  • An easy way for addons to have fallback hotkeys for OS/browser compatibility
  • An easy way for users to change hotkey settings, at minimum in the settings of the addons that utilize them
  • An easy way for developers to check that hotkeys don't conflict with other hotkeys

With these in mind, I think it will be a bit easier to develop a good solution.

@WorldLanguages
Copy link
Member

An easy way for developers to check that hotkeys don't conflict with other hotkeys

By developers, I assume you don't mean addon developers. SA should take care of conflicts, and this should be abstracted away from addon developers.

@lisa-wolfgang
Copy link
Member

lisa-wolfgang commented Oct 18, 2022

I meant that when addons are initially created, the extension would be able to warn developers if the primary hotkey they chose conflicted with another addon's primary hotkey, one of vanilla's, or one of the browsers'/OSes'. Although it's a silly example, we wouldn't want everybody setting Ctrl+S as their hotkey because even though SA would abstract it away, it would then be pointless for most addons to set that as their primary hotkey.

@WorldLanguages
Copy link
Member

Right, I see what you mean. We need to ensure the default hotkeys, set by us, are usable in most platforms and coherent with every other hotkey.

@Tacodiva
Copy link
Member Author

Tacodiva commented Oct 20, 2022

An easy way for addons to have fallback hotkeys for OS/browser compatibility

What does this invovle? Is there differences between keybindings on different OS/browser combos? I can't find any documentation for this.

@lisa-wolfgang
Copy link
Member

Some keyboard shortcuts are reserved by certain browsers and OSes. A fallback system would allow for alternate shortcuts to be automatically applied in these scenarios, without the need for the user to manually select one.

@RIMOPA
Copy link

RIMOPA commented Jan 4, 2023

I think the ideal would be adding an editor-hotkeys addon, and a website-hotkeys addon for vanilla functions and adding boxes empty by default to click and press a key to select like videogames do in other addons, with a warning if it's already used in the same place (editor, website, project, my stuff, messages, etc.)

@Secret-chest
Copy link
Contributor

I think, to check the hotkeys, maybe we can press it using JS and check if it worked.
Regarding the default hotkeys, on Cinnamon you can set your own, and don't get mad at Cinnamon users, because OS hotkeys are more important than hotkeys for one application, so we should respect them. It looks like by default, though, we shouldn't use these keys (but still allow them, except Super)

  • Super/Windows key (can JS even detect it?)
  • Control+Alt+arrows
  • Alt+Tab (of course)
  • Control+Alt+T

@WorldLanguages WorldLanguages added this to the Backlog milestone Aug 5, 2023
@WorldLanguages WorldLanguages added the priority: 3 Medium priority. Includes bugs and useful features label Aug 5, 2023
@WorldLanguages
Copy link
Member

Related discussion: how Scratch handles its editor shortcuts, some weird behaviors about it #5145

Also related: we've added Alt+X as a shortcut for pausing projects at PR #5121

@WorldLanguages
Copy link
Member

In comment #7532 (comment) I just proposed a first step towards letting users customize keybinds. Please do read it if you're interested.

My suggestion targets only the following item mentioned in this issue:

There is no way to rebind keys on anything

And my suggestion builds on top of the existing addon architecture, so very few changes are needed, and there's no need to design any new UIs.
We should not wait until we build a major hotkey management overhaul to allow users to rebind keys that our addons listen to.

@Joeclinton1
Copy link
Member

In comment #7532 (comment) I just proposed a first step towards letting users customize keybinds. Please do read it if you're interested.

My suggestion targets only the following item mentioned in this issue:

There is no way to rebind keys on anything

And my suggestion builds on top of the existing addon architecture, so very few changes are needed, and there's no need to design any new UIs. We should not wait until we build a major hotkey management overhaul to allow users to rebind keys that our addons listen to.

As an open source project there are limited resources. I think this requires too much effort for the amount of help it provides to people. Each setting should just provide the option to pick the key (Which could perhaps be a standardised json option to reduce boilerplate). Anything beyond that is a waste of our time.

@Tacodiva
Copy link
Member Author

In comment #7532 (comment) I just proposed a first step towards letting users customize keybinds. Please do read it if you're interested.

I'm not too sure about the approach of creating a new addon which enables the hotkeys to be changed. While you're right it enables us to convey some information to the user, like it being beta/experimental and the details about two addons listening to the same key, I really don't think a new addon for this is an intuitive solution. To me, it makes a lot more sense for keybindings to be handled in scratch addons core. As a user, I wouldn't expect to have to enable a separate experimental "addon" to change the keybindings of a different addon. I don't really understand why you'd want the hotkeys to show up under a different addon instead of the one that the hotkey is actually effecting.

@lisa-wolfgang
Copy link
Member

Here are some additional reasons a decentralized solution (at least in the UI) is beneficial:

  1. It's the most convenient when enabling new addons. In the case of a centralized solution, a user would have to take the time to navigate there, then navigate back (!) if there are additional addons they want to enable or browse.
  2. A centralized solution would scale poorly. At some point, a search feature would need to be implemented, but that's pretty redundant when the user can already search for addons.
  3. For duplicate/conflict resolution, sending a user to a specific addon is much more elegant than sending them to a specific setting. When presented with the conflict notice, a user may just decide they'd rather disable the conflicting addon entirely instead of coming up with a different keybind. (Letting users leave the keybind unset would be a troubleshooting nightmare.)
  4. Additional UX headaches would be involved in hiding/disabling keybinds for disabled addons (or disabled settings within an enabled addon) rather than letting the addon handle everything itself.

That said, I think it would be a reasonable choice to modify the keybind setting type (which should be created regardless of the solution chosen) to mirror its value to an always-enabled keybind "addon" hidden from the user. This would allow for duplicate keybind checking and other benefits of a centralized solution.

@Secret-chest
Copy link
Contributor

I agree @lisa-wolfgang , but do we add a new addon foe changing Scratch hotkeys?

@WorldLanguages
Copy link
Member

If you think the "decentralized solution" is better, then go ahead with that.

It's likely that I unconsciously believed there were reasons against it. I mean, it's the simplest and most straight forward way to add some customization, and we never added it, so I assumed we didn't want to.

@WorldLanguages
Copy link
Member

To clarify, what I had in mind is allowing users to change the keys for addons such as Grab single block with Ctrl key, Duplicate script with Alt key, etc.
Would also include the upcoming Disable auto nesting addon (PR #7532) which defaults to Ctrl (but it does not conflict with the other Ctrl addon).

@mxmou
Copy link
Member

mxmou commented Jun 25, 2024

It's likely that I unconsciously believed there were reasons against it. I mean, it's the simplest and most straight forward way to add some customization, and we never added it, so I assumed we didn't want to.

One advantage of a "centralized" option is that shortcuts for vanilla Scratch features could be customized in the same place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: 3 Medium priority. Includes bugs and useful features scope: addon.json About the addon.json file structure scope: webpages Related to the web pages (settings page, pop-up, etc) type: enhancement New feature for the project
Projects
None yet
Development

No branches or pull requests