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.

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

Web IDE 'File -> Save'/etc menu items suggest the wrong keybindings on macOS #90

Closed
0xdevalias opened this issue Jun 8, 2024 · 3 comments
Labels
bug Something isn't working playground

Comments

@0xdevalias
Copy link

0xdevalias commented Jun 8, 2024

Describe the bug

Was just checking out the web IDE again after the following landed:

And noticed that the 'File' menu says to use Ctrl+S to save (and other similar menu items):

image

But this doesn't work on macOS, as the keybinding is actually Cmd+S:

Expected Behaviour

It would be good if the suggested key bindings in the menu were able to update themselves based on the platform's bindings.

Code

N/A

Logs

N/A

See Also

@0xdevalias 0xdevalias added the bug Something isn't working label Jun 8, 2024
@0xdevalias
Copy link
Author

0xdevalias commented Jun 8, 2024

Seems the keybindings are hardcoded in the MenuButton components' shortcut prop here:

<MenuHeader
title="File"
open={openedMenu() === 'file'}
onOpen={() => setOpenedMenu('file')}
>
<MenuButton
shortcut={['Ctrl', 'O']}
onClick={() => openFile(props.onFileOpen)}
>
Open File…
</MenuButton>
<MenuDropdown title="Open Recent">
<For each={workspaces()} fallback={<li>No recent files</li>}>
{(workspace) => (
<MenuButton class="whitespace-nowrap">
{new Date(workspace.timestamp).toLocaleString()} -
<code class="overflow-x-clip overflow-ellipsis max-w-36">
{workspace.models[0].value.slice(0, 50)}
</code>
</MenuButton>
)}
</For>
</MenuDropdown>
<MenuButton shortcut={['Ctrl', 'S']} onClick={props.onSave}>
Save
</MenuButton>

It might be cool if they were able to read from the same constant as the keybindings themselves, to ensure they remain in sync?

const openAction = editor.addAction({
id: 'editor.action.open',
label: 'File: Open',
keybindings: [monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyO],
run() {
openFile(props.onFileOpen);
},
});
const saveAction = editor.addAction({
id: 'editor.action.save',
label: 'File: Save',
keybindings: [monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS],
run() {
const model = editor.getModel();
if (model) {
downloadFile(model);
}
},

Skimming the related docs:

It looks like you may not be able to read the keybindings 'back' from the return value of editor.addAction; so you would probably need to separate them into their own const/etc, assuming there isn't another Monaco API that allows you to read them back later.

@0xdevalias 0xdevalias changed the title Web IDE 'File' menu 'Save' menu item suggests the wrong keybinding on macOS Web IDE 'File -> Save'/etc menu items suggest the wrong keybindings on macOS Jun 8, 2024
j4k0xb added a commit that referenced this issue Jun 8, 2024
@j4k0xb
Copy link
Owner

j4k0xb commented Jun 8, 2024

Doing it the same as vscode/monaco now: https://github.com/microsoft/vscode/blob/d40dff9ef9aacb8e5226bcf3938c71f4b9543120/src/vs/base/common/platform.ts#L109-L110

@j4k0xb j4k0xb closed this as completed Jun 8, 2024
@0xdevalias
Copy link
Author

Looks good, thanks! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working playground
Projects
None yet
Development

No branches or pull requests

2 participants