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.

Sign up for GitHub

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

Jump to bottom

Cannot record previously recorded shortcut #15

Closed
p0deje opened this issue Jun 19, 2020 · 4 comments
Closed

Cannot record previously recorded shortcut #15

p0deje opened this issue Jun 19, 2020 · 4 comments

Comments

@p0deje
Copy link

p0deje commented Jun 19, 2020

I've found a small issue when integrating the package. It can also be reproduced in Lungo app:

  1. Open "Preferences" -> "Keyboard Shortcuts..."
  2. Register CMD+SHIFT+K shortcut.
  3. Register CMD+SHIFT+L shortcut.
  4. Try to register CMD+SHIFT+K shortcut again.

For some reason, it's no longer possible to register CMD+SHIFT+K shortcut until the application is restarted.
p0deje added a commit to p0deje/Maccy that referenced this issue Jun 20, 2020
@p0deje
Migrate hotkey to sindresorhus/KeyboardShortcuts
1001035 
Replaces the global hotkey library from combination of KeyHolder,
Magnet and Sauce to KeyboardShortcuts.

The package has few benefits:

1. More native-like UI.
2. Better non-US keyboard support (fixes #108).
3. Built-in storage via UserDefaults.

The package has few limitations:

1. Storage key and type is hardcoded. This requires us to migrate from
   old "hotKey" string preference to "KeyboardShortcuts_popup" JSON
   preference.
2. There is no way to initialize KeyboardShortcuts.Key with character
   string. This is needed for migration. To workaround the limitation,
   Sauce package is left and is used to construct the key from string,
   retrieve key code from it and construct KeyboardShortcuts.Key with
   it.
3. There is no support for default shortcuts. This is addressed in
   sindresorhus/KeyboardShortcuts#13, which is why forked branch is used.
4. Recorder view cannot be previewed in Interface Builder:
   sindresorhus/KeyboardShortcuts#14.
5. It's impossible to register the same shortcut twice:
   sindresorhus/KeyboardShortcuts#15.

The package is great even with all these limitations. The most important
piece is of course better non-US keyboard support. DVORAK is supported
as well.
@sindresorhus
Copy link
Owner

This should have been fixed by 56778e1. I just confirmed that the problem existed before that commit and I cannot reproduce it after. Lungo is still on an older version of KeyboardShortcuts.

@sindresorhus
Copy link
Owner

sindresorhus commented Jun 22, 2020 •
edited
Loading

I noticed p0deje/Maccy@1001035. That's awesome! Feel free to open issue for anything else that might have simplified switching over.

I think #1 would be helpful for you. If you have any thoughts on that, feel free to comment. I think you also might find https://gist.github.com/sindresorhus/2bb90276ad608a22ee5e8fb291b35b88 useful. I saw you're using those Carbon APIs, but that gist wraps them up into a nicer API. If I publish it as a Swift package, would you be interested in using it?

@sindresorhus
Copy link
Owner

New release is out: https://github.com/sindresorhus/KeyboardShortcuts/releases/tag/v0.3.0

p0deje added a commit to p0deje/Maccy that referenced this issue Jun 22, 2020
@p0deje
Upgrade KeyboardShortcuts to 0.3.0
f34ee48 
It adds support for default shortcuts and fixes a bug when the same
shortcut could not be recorded 2+ times:

* sindresorhus/KeyboardShortcuts#13
* sindresorhus/KeyboardShortcuts#15
@p0deje
Copy link
Author

p0deje commented Jun 22, 2020

Thank you!

@p0deje p0deje closed this as completed Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@sindresorhus @p0deje