Week 4: Custom Keyboard Shortcuts

GSoC Projects GSoC - Shortcut Project
1

Hi!

I spent the week mostly doing improvements and testing.

Detect duplicates in the custom keymap

If the same shortcut is used in two commands, the default keymap is used instead, and a helpful error message is logged to the console mentioning which shortcut is containing the duplicate.

This is different in having an invalid shortcut or command in one or few shortcut items. In that case, the app warns the user and skips over it.

Let me know what you think about this approach :smiley:

Refactor keymapHandler.ts to KeymapService

After a review from Laurent on my draft PR, it was decided that a class-based service would be the better way to handle the keymap operations, which makes sense. This is because the original keymapHandler module has its own state (in-memory keymap object) and provides few methods to alter it.

I've refactored keymapHandler into a service, which took me longer than I had expected.. :sweat_smile: I've learned a lot about how TypeScript differs from JavaScript during the transition.

A service in Joplin is class-based and (mostly) runs only one instance at a time, and has own logger. I've used this logger to display helpful errors and warnings to the user.

I've also had to spent a significant amount of time rewriting the existing tests and get them to work with the new service.

See my PR with the KeymapService here: https://github.com/laurent22/joplin/pull/3252

Cheers!

2 Likes
2

While looking through the conflict algorithm something else came to mind.

How do we disable a shortcut? I donā€™t see a way to do this currently.

As an example:

  • we use Cmd+N as the default for creating new note
  • thereā€™s no entry in the keymap file that changes this shortcut (by changing it directly or by using it for another action)
  • so now the user is stuck with Cmd+N and no way to remove it

This scenario is even worse for global shortcuts.

I think, we need another field in the file, something like enabled: true/false, with true as the default.
In the UI there could be a checkbox to disable the shortcut.

What do you think?

2 Likes
3

Thanks for the reply!

Iā€™ve recently pushed a commit with a solution for this. If there are any issues with it, I can try the method you described.

The currrent method is use null for the ā€œacceleratorā€ property. For example,

{ "command": "newNoteItem", "accelerator": null }

It looks a bit strange, but it works well. I modified existing functions to accept null Accelerators, and everything seems to work very well.

Please let me know what you prefer. Thanks!

1 Like
4

If null disables a shortcut, we won't need a separate field. The separate field was just an example.

Your solution is in fact a lot better. :+1:

2 Likes