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

[Bug] Multiple editor instances only latest created instance is working properly #2947

Open
1 of 2 tasks
steinerjakob opened this issue Feb 7, 2022 · 21 comments
Open
1 of 2 tasks
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug editor-core

Comments

@steinerjakob
Copy link

Reproducible in vscode.dev or in VS Code Desktop?

  • Not reproducible in vscode.dev or VS Code Desktop

Reproducible in the monaco editor playground?

Monaco Editor Playground Code

/* HTML CONTENT
<div id="container" style="height: 50%"></div>
<div id="container2" style="height: 50%"></div>
*/

const model = monaco.editor.createModel("function hello() {\n\talert('Hello world!');\n}", 'javascript');
const model1 = monaco.editor.createModel("function test() {\n\talert('Hello test!');\n}", 'javascript');
const editor = monaco.editor.create(document.getElementById('container'), {
    model
});
const editor2 = monaco.editor.create(document.getElementById('container2'), {
    model:model1
});

editor.addCommand(monaco.KeyMod.CtrlCmd|monaco.KeyMod.Shift|monaco.KeyCode.KeyF, ()=> {
    const pos = editor.getPosition();
    console.log('pos', pos)
});
editor2.addCommand(monaco.KeyMod.CtrlCmd|monaco.KeyMod.Shift|monaco.KeyCode.KeyF, ()=> {
    const pos = editor2.getPosition();
    console.log('pos2', pos)
});

Actual Behavior

Triggering crtl + shift + f in the first instance, the custom command of the second instance will be triggered.

Expected Behavior

Triggering crtl + shift + f in any editor instance should trigger the command of the correct editor instance.

Additional Context

No response

@hediet
Copy link
Member

hediet commented Feb 7, 2022

Both editor instances get injected the same instance of StandaloneKeybindingService. Thus, this is currently by design. However, I can see that this looks like a bug.

I'll tag this as feature request to use independent keybinding services for different editors.

@hediet hediet added the feature-request Request for new features or functionality label Feb 7, 2022
@steinerjakob
Copy link
Author

Ok, thats weird.
Because currently I'm using the 0.31.1 version in my project with multiple editor instances and the behaviour is like the expected one.

@hediet
Copy link
Member

hediet commented Feb 7, 2022

So it is a regression in 0.32.0?

@steinerjakob
Copy link
Author

From my point of view, yes it is a regression in 0.32.0

@hediet hediet added bug Issue identified by VS Code Team member as probable bug and removed feature-request Request for new features or functionality labels Feb 7, 2022
@hediet hediet added this to the February 2022 milestone Feb 7, 2022
@jmbarbier
Copy link

Same problem here, i can confirm if needed this is a regression between 0.31.1 and 0.32. Reverting to 0.31 makes keybindings stick to their editor instance.

@alexdima alexdima modified the milestones: February 2022, March 2022 Mar 14, 2022
@jmbarbier
Copy link

Just keeping track of this issue : this is still present on 0.33

@dingiyan
Copy link

Hello everyone, Several months have passed. How is this problem fixed?

@ckorherr
Copy link

I have the same problem, with multiple editor instances on the page I need to disable actions.find for some editors. Once disabled, it is disabled for all instances. This worked before on 0.31.1.

@Prinzhorn
Copy link

Can someone explain how the team is using GitHub milestones? It appears pretty random from an outsiders perspective, given that releases don't even correlate with the months. This issue was added to the July 2022 milestone, which has 18 of 22 issues solved and is closed. Same with some other older milestones. You can see a some of the recent milestones are closed but were never finished and issues were not moved to the next recent milestone https://github.com/microsoft/monaco-editor/milestones?state=closed
So from my perspective this was planned for July 2022, but wasn't fixed. But the milestone was closed anyway. So I just want to make sure issues like this aren't forgotten, because I doubt you look into closed milestones.

@Nishchit14
Copy link

Nishchit14 commented Nov 4, 2022

Opps... This is breaking our whole application. we're using multiple editor as table cell. I have managed the tab navigation through custom editors key mapping but now the command of last cell is applying for all previous cell, thus no tab works.

image

any other work-around? appreciate the help cc @alexdima

@hedgelot
Copy link

This is taking too long, is there a new estimated date when this will be fixed?

@ihqtim
Copy link

ihqtim commented Nov 24, 2022

This has also been impacting us for the last few months since updating to a newer release of the editor (we are currently using 0.34.1). In our case a user may have multiple editors open (i.e. editing multiple source file). We use custom key binding CTRL + S to save the model displayed in the "active" editor. In prior versions this worked as expected, now the CTRL + S command is routed to the most-recently-opened editor instance rather than the active/in-focus editor instance, meaning that:

  • users are accidently saving files they do not intend to save, and
  • files that users think they have saved are not actually saved (i.e. they lose work)
    This is making our users quite unhappy.

@TheUnlocked
Copy link

Inspired by the workaround in grafana/grafana#60172 which mentioned this issue, here's a more distilled workaround:

function bindAddCommandContext(editor: editor.IStandaloneCodeEditor, context: string): { dispose(): void } {
    const addCommand = editor.addCommand;

    editor.addCommand = function addCommand_hijacked(keybinding, handler, _context) {
        if (_context !== undefined) {
            console.error(
                'editor.addCommand cannot use the context parameter. ' +
                'This is due to a workaround for an upstream bug: ' +
                'https://github.com/microsoft/monaco-editor/issues/2947'
            );
        }
        return addCommand.call(this, keybinding, handler, context);
    };

    return {
        dispose: () => {
            editor.addCommand = addCommand;
        }
    };
}

// Based on workaround in https://github.com/grafana/grafana/pull/60172
const editorFocusedContextKeyName = `__isEditorFocused-${/* unique ID function of your choice */}`;
const isEditorFocused = editor.createContextKey(editorFocusedContextKeyName, false);
const onBlurDisposable = editor.onDidBlurEditorWidget(() => isEditorFocused.set(false));
const onFocusDisposable = editor.onDidFocusEditorText(() => isEditorFocused.set(true));
const addCommandContextBinding = bindAddCommandContext(editor, editorFocusedContextKeyName);

// proceed with configuration

// dispose as needed

Not super pretty, but will hopefully help some of you in the meantime. Confirmed working with @monaco-editor/react (create the binding in onMount).

@yuanzhibang-tool
Copy link

@stoanarrr You can re add the command when the editor focus, it works for me:

addCommand () {
    this.editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS, () => {
    });
    this.editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyR, () => {
    });
    this.editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyMod.Shift | monaco.KeyCode.KeyR, () => {
    });
    this.editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.Period, () => {
    });
  }
this.editor.onDidFocusEditorText(() => {
        this.addCommand();
    });

@cihad
Copy link

cihad commented Jul 17, 2023

Yes, it's a problem.

See on Playground

@spahnke
Copy link
Contributor

spahnke commented Jul 18, 2023

A simpler workaround than the ones I've seen above might be to use actions instead of commands:

Playground link

@beanpot99
Copy link

Not related to key bindings but I am having a very similar issue when applying schemas using setDiagnosticsOptions. I have multiple instances of the Editor component and different schemas are applied to each but each subsequent Editor just overrides the schema of all the previous Editors. All the Editor components have unique ids.

@jkillian
Copy link

jkillian commented Feb 13, 2024

Just tested in the playground with the latest dev build and still seems to be an issue.

The workaround to switch addCommand -> addAction seems to have worked well for us. We were using the context option of addCommand as suggested here, and it appears that precondition of addAction works similarly.

@AmamiyaCx
Copy link

Hello,The latest version I am currently using still has this issue. Is there a plan to fix it, or is there currently any other solution available

@blixt
Copy link

blixt commented Sep 10, 2024

Still occurs in v0.51.0-dev-20240807. Since the editors now share the keybinding service, it might make sense to not use the standalone editor's addCommand at all in multi-editor setups, and instead do something like:

// Track focused editor ourselves:

let focusedEditor;

// Handle Cmd/Ctrl + S globally instead of per editor:

monaco.editor.addCommand({
    id: "myapp.save",
    run: (...args: unknown[]) => {
        if (!focusedEditor) return
        saveContents(focusedEditor.getModel())
    },
});

monaco.editor.addKeybindingRule({
    keybinding: monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS,
    command: "myapp.save",
});

// Make each editor report when it's focused:

const editor1 = monaco.editor.create()
editor1.onDidFocusEditorText(() => {
    focusedEditor = editor1
});

const editor2 = monaco.editor.create()
editor2.onDidFocusEditorText(() => {
    focusedEditor = editor2
});

// In theory you don't need to track on blur if you are doing this to all
// your editors because the commands only run while an editor has focus.

My understanding is that editor.addAction(…) is always per editor and its keybindings will be unique to that editor instance, so if you don't need a command at all (I believe they are required in some contexts), then using addAction instead is probably the way to go:

const editor1 = monaco.editor.create()
editor1.addAction({
    id: "myapp.save",
    label: "Save",
    keybindings: [monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS],
    async run(editor) {
        saveContents(editor.getModel())
    },
});

const editor2 = monaco.editor.create()
editor2.addAction({
    id: "myapp.save",
    label: "Save",
    keybindings: [monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS],
    async run(editor) {
        saveContents(editor.getModel())
    },
});

As a bonus, this approach will make the "Save" action show up in the command palette of the editor.

@blixt

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug editor-core
Projects
None yet
Development

No branches or pull requests