microsoft / monaco-editor

A browser based code editor
https://microsoft.github.io/monaco-editor/
MIT License
40.26k stars 3.59k forks source link

Right click in view zone or content/overlay widget has inconsistent behavior #2395

Open JeanPerriault opened 3 years ago

JeanPerriault commented 3 years ago

monaco-editor version: 0.22.3 Browser: Safari / Chrome OS: macOS Playground code that reproduces the issue: You can use this playground to see the issue / weird behavior: https://microsoft.github.io/monaco-editor/playground.html#interacting-with-the-editor-listening-to-mouse-events

When you try to select a text in content or overlay widget, then use right click.

  1. For Chrome:
    • Content widget: test selection is not possible
    • Overlay widget: test selection is possible, and right clicking will bring contextual menu with text selection kept => OK
  2. For Safari:
    • Content widget: test selection is possible, but when right clicking, text selection is lost. Thus contextual menu will appear but without context (selected text)
    • Overlay widget: same as content widget

Playing with "contextmenu" editor option and "allowEditorOverflow" option of content widget didn't help.

From what I've understood, VSCode is using view zone and overlay combination for error markers (https://github.com/microsoft/monaco-editor/issues/83) => in this case, at least on macOS, you can select the text in widget, but right click doesn't work.

I was expecting to get text selection kept and contextual menu available in all cases, and even better to have access to monaco editor contextmenu (and not browser/system one)

JeanPerriault commented 3 years ago

Is there a timeframe for issues review in this project? I'm seeing lots of unanswered issues last month and I wonder if it is expected, if maintainers have no bandwidth currently, or if PR's are expected (in that last case some pointers to code that should be modified would be a great help). I'm asking because I've created issues or asked for info either via GitHub or dedicated stackoverflow tag without answers/followup. This lib is great but if support is "sporadic" (at this time, because I've seen tons of answers in previous months/years) and we can't easily PRs (need to understand what code will be extracted from VSCode), then it is hard to choose it for production, when having specific use cases. What would be the recommended way? Or should I check with 'microsoft support / service hub' we're subscribing to in my company?

JeanPerriault commented 3 years ago

@Prinzhorn could you elaborate? (I'm never sure of the meaning of the big eyes emoji) Would the best way be to follow https://github.com/microsoft/monaco-editor/blob/main/CONTRIBUTING.md, implement our changes and build our own version of the lib, without contributing back?

Prinzhorn commented 3 years ago

I've subscribed to this issue a few weeks ago because I'm interested in the same things. That's what that means. I keep an eye on all issues of projects that are critical to me. So you could say the eyes are looking questioningly at what you've said.

Monaco is incredible and I will use it in a production environment, but I feel exactly what you're saying. Microsoft probably has interest in maintaining VS Code but it's not clear where Monaco stands. And Monaco as a separate project does not exist, because it's extracted from VS Code. Apart from @alexdima I've never seen anyone else from Microsoft (or any other maintainer) comment on issues here. So we're in that limbo where we're at the mercy of Microsoft to continue supporting Monaco while at the same time the complexity of Monaco and the VS Code situation make it hard to contribute or maintain a fork. Because changes to Monaco have to go through the VS Code PR process and you might want changes in Monaco that are completely useless to VS Code itself.

Unfortunately I've seen this happen countless of times. That's the difference between an open-source project that grows naturally because individuals have interest in the same project and one that an enterprise develops internally and then makes public as an open-source project. There are always internal processes and issues that we have no influence in.

mofux commented 3 years ago

Because changes to Monaco have to go through the VS Code PR process and you might want changes in Monaco that are completely useless to VS Code itself

That's not really true. There's a lot of code in the vscode base that is only there to serve the standalone Monaco Editor. Of course, since Monaco is mostly generated from the vscode sources, there is always a tradeoff in functionality that should go into the core.

On the other hand the Monaco Editor has a very rich and stable API that can be easily extended and many things could be contributed via user-land extensions.

Unfortunately these extensions either have no visibility or do not exist. And I think we should start a discussion why:

IMHO the biggest pain point for providing standalone extensions to the monaco editor is of technical nature. It is very complicated for an extension to access internal API because it has no access to the loader. Unfortunately, this internal API is often required for advanced use cases.

For example, to access the readCharWidths function with webpack:

const { readCharWidths } = await import('monaco-editor-core/esm/vs/editor/browser/config/charWidthReader');

And via AMD Loader:

require.config({ paths: { vs: '../node_modules/monaco-editor-core/min/vs' } });
require(['vs/editor/browser/config/charWidthReader'], ({ readCharWidths }) => {
  // ...
});

It would be great if the monaco editor had a generic public API for this, e.g.:

const { readCharWidths } = await monaco.import('vs/editor/browser/config/charWidthReader');

This way you could simply pass the monaco instance to the extension and then the extension can work based on that. Implementing this generic import feature might be technically challenging or even impossible though.

Prinzhorn commented 3 years ago

That's not really true. There's a lot of code in the vscode base that is only there to serve the standalone Monaco Editor. Of course, since Monaco is mostly generated from the vscode sources, there is always a tradeoff in functionality that should go into the core.

Thanks for clarifying!

And I think we should start a discussion why:

Maybe let's create a different issue for that, as it's unrelated to this bug report

alexdima commented 3 years ago

@JeanPerriault I wanted to clarify some points:


Text selection in browsers can be turned on or off using a CSS property called user-select. When going to the test page at https://microsoft.github.io/monaco-editor/playground.html#interacting-with-the-editor-listening-to-mouse-events and using Inspect Element on the content widget, it is possible to see in the Styles section that this property gets inherited from one of the editor's dom nodes. The editor uses this property in its lines dom node to prevent the browser selection over the text in the editor.

image

So the solution here is to re-enable selection on the content widget dom node, like the go to error widget does here:

user-select: text;
-webkit-user-select: text;
-ms-user-select: text;

For the right click context menu, the piece of code that implements the editor's custom context menu, always ends up refocusing the editor or preventing default on the contextmenu event when right clicking on a content widget. So I think that is why the selection always gets lost. From looking at that code, I don't think there is anything you can do on your side to avoid it. The only thing I can think of is to create a custom webpack of the editor where the entire contextmenu contribution is skipped. You can find an example here.

JeanPerriault commented 3 years ago

Hi,


Regarding this specific issue.

user-select Yes I've seen this user-select CSS code, my question was more: why do we have different behavior between Chrome and Safari there => If you check on Safari, you'll see that this property "doesn't get" inherited from one of the editor's dom nodes, contrary to what happens on Chrome. Is that expected?

Moreover:

Right click context menu => For this, when you say we can't avoid it, I could at least try something in contextmenu contribution before entirely disabling it no? Is there a reason for disabling monaco editor contextual menu in widgets? (ideally I would like to use it for consistency - for now I have used the "overflowWidgetsDomNode" editor new option, so that I can use my own contextual menu, but that's not ideal). I'll need to dig in the code, thx for sharing the ref.