-
Notifications
You must be signed in to change notification settings - Fork 207
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
Feature/Custom Keybinds #569
base: main
Are you sure you want to change the base?
Conversation
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request |
e.preventDefault(); | ||
// Close popups and menus | ||
this.shortcut('escape', e, () => { | ||
e.preventDefault(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved escape up to allow to escape the keybinding window dialog when all other keys are disabled.
Hello, thanks for the PR, we had different idea in mind when we were discussing custom shortcuts: the setting itself should be inside the shortcut popup, or maybe it should be combined now with the settings, idk. Better discuss with designers first when implementing functionality like this since we do a lot of polishing and design reviews. This is preliminary review cause I'm busy at the moment, you can write to the slack channel if you want to discuss some issues Regarding your code there are some issues: Code formattingFirst of all check code compliance and formatting, right now it's wrong in some places and inconsistent.
StorageNo need to use MobX for key storage and making it observable, it does not need reaction in components since keybindings are checked in handlers. Small issues
|
Ok all that sounds good, thank you for the feedback. |
I'm not sure who to contact about the slack. If you guys are really busy I can leave the conversation for later, don't want to interrupt current work. |
You can contact @tripkovsky on telegram channel, I will give him the link to this PR so he can add you. |
Hey @luisliz, could you drop me a message on Telegram (@tripkovsky) with your email and, if you have one, your community.anytype.io account name? Alternatively, you can shoot an email to [email protected] with your request. Also, I'll need the technical info from your Anytype instance, which you can find at Top menu > Help > Technical Information. Appreciate it! |
Description
This is for adding custom keybindings to the app it leverages the same shortcut mechanism that is already used in the app but, redone to use an actions mechanism instead of fixed. When no user actions are set in the
keybindingStore
it just performs the existing shortcuts.There is also a shortcut detection system in the settings popup that disables all other shortcuts when the box is focused, tied to focus to hopefully not cause issues where all keybinds get disabled.
Fixed keymaps cannot be changed or disabled.
I have a lot of placeholder text for now but wanted to get some feedback before I continue, to make sure I don't have anything fundamentally wrong.
I'll definitely replace them with the localized versions
Need guidance
P.S. I know this PR might be too big but I can break it up if needed.
P.S.S. Kudos on this repo i'm mostly a vue developer other than some react-native from work but it has been really enjoyable to work in it, i'm more of a fan of anytype after understanding the infra.
What type of PR is this? (check all applicable)
Related Tickets & Documents
https://community.anytype.io/t/customizable-keyboard-shortcuts-hotkeys/2135
My end goal is this :) https://community.anytype.io/t/vim-like-modes-keybindings/1212
Mobile & Desktop Screenshots/Recordings
2024-02-15.2020-51-46.mp4
Added tests?
Added to documentation?
Not yet
[optional] Are there any post-deployment tasks we need to perform?