ankitects / anki

Anki's shared backend and web components, and the Qt frontend
https://apps.ankiweb.net
Other
18.58k stars 2.11k forks source link

Centralise shortcut definitions #1508

Open RumovZ opened 2 years ago

RumovZ commented 2 years ago

There are regularly reports like https://forums.ankiweb.net/t/2-1-49-hotkey-browser-mode/15254. In this particular case I remember suggesting several key sequences before coming up with a seemingly unconflicting one.

I wonder if it would be possible to define all of Anki's key sequences in one place, like aqt/shortcuts.py. That would facilitate testing for conflicts (with each other or OS shortcuts), make it easier to find available sequences and allow for documenting previous experiences (I have yet to find a useful online ressource for that). It should be easy enough for shortcuts defined in PyQt, but is there a way to do that for the ones defined in TS as well? Designer shortcuts would have to be moved into Python, but that's not necessarily a bad thing in my opinion.

TessTrella commented 2 years ago

If you do decide to centralize the shortcuts, I'd like to suggest putting them in a config file, like a YAML or XML file, instead of inside a source code file.

Here's the reason for this suggestion...

In this issue about shortcuts I asked if it's possible to make the shortcuts configurable by the user so that if there's a conflict with their OS-level or language input key bindings, the user can just re-bind the shortcut.

I know that fully supporting user-configurable key bindings would be a lot of work because, aside from making the shortcuts configurable, you'd also need to build out a UI. But putting the key bindings in a flat file would allow you to deliver that feature in phases.

Phase 1: Move all the key bindings to an XML file. You could just read the config in at startup time, and warn the users that if they edit that file, they need to restart the app. No need for hot reloading or anything fancy like that.

This would give the users the power to re-bind their shortcuts right away, even though there's no UI for it yet. Even a non-technical user can probably edit an XML file to change a shortcut. But if the bindings are in a python file or in the database, then only highly technical users will be able to change them.

Phase 2: Build a nice UI to allow the user to view and edit the shortcuts.

Thanks so much for all your work. I love Anki!

RumovZ commented 2 years ago

In this issue about shortcuts I asked if it's possible to make the shortcuts configurable by the user so that if there's a conflict with their OS-level or language input key bindings, the user can just re-bind the shortcut.

Agreed. I'm not sure if it's a good idea to have a full-fledged UI for that though, because if you make it too easy for users to fiddle with shortcuts, the number of issues with messed-up settings will also crop up—and wrong key bindings can lead to more severe problems quite fast. As for the markup file, the default shortcuts have to be hard-coded anyway in case the external file gets corrupted. Anki has a built-in console, which is already utilised to change hidden settings, so it should be much easier to use that instead of handling an external file.

TessTrella commented 2 years ago

Ah ok, that makes sense. Thanks for the explanation!

dae commented 2 years ago

I wonder how much centralising the shortcuts will help with identifying conflicts? Conflicts don't tend to be with other shortcuts defined in Anki, since we already tend to grep through the sources first to make sure the shortcut is not already being used - instead it's mainly conflicts with system shortcuts and add-ons that we seem to hit, and I'm not sure a centralized location really helps us with that, as attempting to list out all the possible system shortcuts that may conflict seems like quite a tricky task.

A centralised registry could make it easier for users to override specific shortcuts, but it brings its own complexities, like how to warn about multiple actions sharing the same shortcut, when depending on the screen they are used in, that may or may not matter. We'd also have to consider things like cases where we want a different shortcut on different platforms, multiple shortcuts referring to the same action, etc. I see the appeal of it, but also fear it may end up being a bigger project than first expected :-)

RumovZ commented 2 years ago

I'm not sure a centralized location really helps us with that, as attempting to list out all the possible system shortcuts that may conflict seems like quite a tricky task.

I don't know if there are really that many global system shortcuts. On Windows for example, all I can think of involve the Windows key, which is easily avoided. But either way, the idea was not to list them out exhaustively, but to add them whenever we ran into an issue, so we at least don't repeat previous mistakes. Alt+number might be an example that could crop up again.

how to warn about multiple actions sharing the same shortcut, when depending on the screen they are used in, that may or may not matter. We'd also have to consider things like cases where we want a different shortcut on different platforms

I imagined a simple dataclass that has a fallback sequence and arbitrary system-specific sequences defined. It can be scoped depending on its context. Then when checking for conclicts, it would only be compared to sequences within the same main window/modal dialog, and if the shortcut requires widget focus, even in the same widget. That would also allow to more easily assign shortcuts consistently across different windows, if they do the same thing.

dae commented 2 years ago

I still have the feeling this could end up being somewhat complicated, and my gut feeling is that it might be best put off for now. Shortcut handling in Svelte was only introduced by @hgiesel fairly recently, and I think we probably need to give it some time to mature/issues to be discovered and ironed out. IIRC the TS shortcuts all operate at a global scope at the moment for example.

stephenll commented 2 years ago

Just sharing some information on how another app has done this: https://support.rstudio.com/hc/en-us/articles/206382178-Customizing-Keyboard-Shortcuts-in-the-RStudio-IDE

kenwuuu commented 1 year ago

I think scope crept a little in the comments, I'd argue you can centralize without exposing the config to users. That would close this issue, provide a central source of truth, and allow you to utilize the config file in the future if you wish to.