Initial keyboard shortcuts configuration implementation #496
Reference in New Issue
Block a user
Delete Branch "keyboard-shortcuts"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Creates a system for adding new IDE Editor menus and menu items, as well as keyboard shortcuts for those items.
Add a menu item to an existing menu
Open
app/web/src/components/EditorMenu/menuConfig.tsxIdentify the config for the menu you want to add to: at time of writing there are
fileMenuConfig,editMenuConfig, andviewMenuConfigAdd an object to the
itemsarray of the config, matching theEditorMenuItemConfigtype. The order of this array determines the order of the menu's items within the Editor. Thecomponentproperty must return a React component, and has access tostate,thunkDispatch, and the rest of theconfigof the item. Within the React component,config.callbackmust be set to a function. Typically theDropdownItemcomponent is used for menu items, but any React component that integratesuseHotkeys()could work.Be sure to check that any keyboard shortcuts defined on these items are not already taken by another item (TODO write tests for this?)
Add a menu to the Editor
[nameOfMenu]MenuConfigthat meets the criteria for theEditorMenuConfigtype.itemsarrayeditorMenuConfigarray. The order of this array determines the order of the menus within the Editor.Hey @Irev-Dev I messaged on Discord about this but I'm sorry I'm really not sure how to merge your branch with tweaks into this. I know you had a corrected implementation of my
EditorMenuItemConfigtype and a few other things. Is there a more elegant way to pick out those parts with a merge of some sort?This was a great workaround! I found a way to make the original
MenuItemsalways render and only visually/audio show/hide here, which works very similarly: https://headlessui.dev/react/menu#showing-hiding-the-menuHey, code looks good.
One or two changes would be good (mostly just fixing up the dependancies, I'm not worried about the modal corner).
But Im not sure what to do about the issue of the monaco editor competing for hotkeys?
oh and might as well run
yarn rw lint --fixtoo, does that work for you?Could you please
app/package-lock.json/appand runyarn workspace web add hotkeys-js react-hotkeys-hook.Should this also have a button in the menu, how will users learn how to get this modal up?
Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.
Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
@@ -0,0 +23,4 @@return useContext(ShortcutsModalContext)}const AllShortcutsModal = () => {Seems to be a weird artifact on the rounded corner, not sure if worth fixing?
Nice, that's a much better approach.
if you make this:
Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since
componentdoesn't exist onEditorMenuItemConfig.I think callback is not needed now and component is?
Ah dang I used NPM again 🤦🏻♂️ thanks @Irev-Dev I'll fix this
@@ -0,0 +23,4 @@return useContext(ShortcutsModalContext)}const AllShortcutsModal = () => {Oh yeah lemme check it out, I bet it's a difference between the Modal and the inner content, and maybe I rounded the inner stuff as well and they're basically overlapping.
Ah crap this is all good consideration @Irev-Dev.
Here are my recommendations for these 2:
command+/👉🏻ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?command+alt+R👉🏻ctrl+shift+R. It appears thatctrl+shiftis a fairly reliable key combination.Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
One thing that's weird to me is that HotKeys treats
CtrlandCmdas the Windows/Mac alternatives, but it's reallyCmdandWinkeys that are analogous, usually called the OS key within the browser, andCtrlis a key that both OSes have.Yup it yelled at me now!
@@ -0,0 +1,109 @@import React from 'react'import { useRender } from 'src/components/IdeWrapper/useRender'import { makeStlDownloadHandler, PullTitleFromFirstLine } from './helpers'import { useSaveCode } from 'src/components/IdeWrapper/useSaveCode'This won't work currently because I backed out of writing it, but @Irev-Dev should I use a React Context to make the
AllShortcutsModalable to be opened from outside of the component? I could make its own little context to provide around theIdeWrappercomponent, or the page component itself. I might try that and see if you like it.@@ -0,0 +46,4 @@.map((menu) => (<section key={'allshortcuts-' + menu.name} className="my-6"><h3 className="text-xl border-b-2 pb-2 mb-2">{menu.label}</h3>{menu.items.map((item) => (Yeah overwriting MaterialUI's default styles is not intuitive
@Irev-Dev let me know if this isn't the right approach. I needed to provide a hook to toggle that dialog from elsewhere in the app, so I used a Context scoped to the
IdeWrapper@@ -0,0 +1,109 @@import React from 'react'import { useRender } from 'src/components/IdeWrapper/useRender'import { makeStlDownloadHandler, PullTitleFromFirstLine } from './helpers'import { useSaveCode } from 'src/components/IdeWrapper/useSaveCode'Tried this approach below.
Seems good
Would still be good to delete the dependencies from this file now that they're in app/web/package.json
@@ -0,0 +46,4 @@.map((menu) => (<section key={'allshortcuts-' + menu.name} className="my-6"><h3 className="text-xl border-b-2 pb-2 mb-2">{menu.label}</h3>{menu.items.map((item) => (Mmm, we'll move towards headlessui where possible in future. Very tailwind friendly, and friendly to customising styles in general too.
Removed now, my bad just skipped a step.
Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like
KeybindingsRegistry.getAll()method or anything like that.The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.
Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Yeah sure, sounds good.
Added issue #499 to track this
Live now.