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
Notebook Editor: scoped context key service #99318
Comments
I'm pretty sure that if you open two notebooks, the second is overwriting the context keys of the first. How this works for code editors - they need to unwrap an
@bpasero I'm confused about that code, it looks like I would need to extend it for every editor type in order to get the editor title control to render actions using the editor's contextKeyService? |
@roblourens we can use const activeControl = activeEditorPane.getControl();
if (isCompositeEditor(activeControl) && isCodeEditor(activeControl.activeCodeEditor)) {
return activeControl.activeCodeEditor;
} |
maybe we should copy logic from vscode/src/vs/workbench/services/editor/browser/codeEditorService.ts Lines 26 to 40 in 4f73c4a
to vscode/src/vs/editor/browser/editorBrowser.ts Lines 1035 to 1045 in 4f73c4a
@jrieken is it safe to do that? |
Well, I don't want the context key service from the code editor inside the notebook editor, I want it from the notebook editor itself. |
right, I misread it. I think then we can extend vscode/src/vs/editor/common/editorCommon.ts Line 537 in b987634
by adding |
@roblourens I cannot recall, I think Jo wrote that code to ensure the title menu gets the most appropriate context key service before rendering. |
Each editor group gets a dedicated context key service - a simple way to enumerate them is to paste this into the console I am not quite sure what the actual issue is but I can image that there a problem because context key hierarchies work along the dom structure and given that notebook editors might not always follow that structure, right? |
The real issue is what to do when we move a notebook from one group to another, and that partly determines whether the editor should get its own group or not. Currently we keep the notebook widget alive when the Editor is recreated in the new group, to avoid disposing the webview. So we can
|
After fixing https://github.com/microsoft/vscode/issues, this is an issue again
|
- Give NotebookEditorWidget its own ScopedContextKeyService - Give the title control a way to get the context key service inside any editor Fix #99318
The context key service we used in the notebook editor is not scoped to the DOM node, which might lead to potential context key issues
vscode/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts
Line 105 in 8fc68f7
We should create context key service which scoped to
NotebookEditorWidget
root DOM node.The text was updated successfully, but these errors were encountered: