anytype-ts
anytype-ts copied to clipboard
Feature/Custom Keybinds
- [x] I understand that contributing to this repository will require me to agree with the CLA
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
- Should this be stored in the config that syncs? Thought of starting with non-syncing at first.
- I know there are bindings that apply to e.g. editor but also to main I think this might cause issues
- Should I separate which keymaps can be disabled or which can be overriden, I feel like the fixed keymap is enough.
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)
- [x] 🍕 Feature
- [ ] 🐛 Bug Fix
- [ ] 📝 Documentation Update
- [ ] 🎨 Style
- [x] 🧑💻 Code Refactor
- [ ] 🔥 Performance Improvements
- [ ] ✅ Test
- [ ] 🤖 Build
- [ ] 🔁 CI
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
https://github.com/anyproto/anytype-ts/assets/2022460/7d272727-a013-4b70-9b42-4e0bbe5943ea
Added tests?
- [ ] 👍 yes
- [ ] 🙅 no, because they aren't needed
- [ ] 🙋 no, because I need help
Added to documentation?
Not yet
- [ ] 📜 README.md
- [ ] 📓 tech-docs
- [ ] 🙅 no documentation needed
[optional] Are there any post-deployment tasks we need to perform?
CLA Assistant Lite bot:
Thank you for your pull request, we really appreciate it!
Please sign our Contributor License Agreement before we can accept your contribution.
You can sign the CLA by simply commenting on this pull request with the following text.
I have read the CLA Document and I hereby sign the CLA
You can retrigger this bot by commenting recheck in this Pull Request
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 formatting
First of all check code compliance and formatting, right now it's wrong in some places and inconsistent.
constructor(props: I.PopupSettings) {
super(props);
}
render() {
import {
I,
translate,
ShortcutActionList,
} from 'Lib';
Storage
No need to use MobX for key storage and making it observable, it does not need reaction in components since keybindings are checked in handlers. Second thing is that storage should be done in a separate json file through Electron json storage so this file would be easier accessible, the same way it works with the application config.
Small issues
const cmdKey = UtilCommon.isPlatformMac() ? 'cmd' : 'ctrl';
- there is a method keyboard.cmdKey()
import { ShortcutActionList, ShortcutActionConstants, UserKeyboardShortcut, KeyboardShortcut } from './keyboardConstants';
- there are no exports like this in Lib, it should export container, and overall I think all logic related to keyboard should be in lib/keyboard
or lib/keyboard/constant
, interfaces should be moved to interface
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.
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.
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.
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!
Unfortunately I have to close this PR because there is not activity and OP was not added to the Slack channel. Custom key bindings will be implemented internally as part of upcoming releases.