Skip to content

Commit bca395e

Browse files
authored
Fix cell editor size (#9218)
Main goal here is to correctly size cell code editors, so things like breakpoints are accessible. Also removed some parts that didn't seem to be doing anything - hopefully nothing breaks! ### Release Notes #### New Features - N/A #### Bug Fixes - N/A ### QA Notes
1 parent c3ee3a3 commit bca395e

File tree

3 files changed

+30
-65
lines changed

3 files changed

+30
-65
lines changed

src/vs/workbench/contrib/positronNotebook/browser/EnvironmentProvider.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import React from 'react';
88

99
// Other dependencies.
1010
import { ISize } from '../../../../base/browser/positronReactRenderer.js';
11-
import { ISettableObservable } from '../../../../base/common/observableInternal/base.js';
11+
import { ISettableObservable } from '../../../../base/common/observable.js';
1212
import { IScopedContextKeyService } from '../../../../platform/contextkey/common/contextkey.js';
1313

1414
/**
@@ -18,7 +18,7 @@ interface EnvironmentBundle {
1818
/**
1919
* An observable for the size of the notebook.
2020
*/
21-
sizeObservable: ISettableObservable<ISize>;
21+
size: ISettableObservable<ISize>;
2222

2323
/**
2424
* A callback to get the scoped context key service for a given container.

src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookEditor.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ export class PositronNotebookEditor extends EditorPane {
468468
<NotebookVisibilityProvider isVisible={this._isVisible}>
469469
<NotebookInstanceProvider instance={notebookInstance}>
470470
<EnvironentProvider environmentBundle={{
471-
sizeObservable: this._size,
471+
size: this._size,
472472
scopedContextKeyProviderCallback: container => scopedContextKeyService.createScoped(container),
473473
}}>
474474
<PositronNotebookComponent />

src/vs/workbench/contrib/positronNotebook/browser/notebookCells/CellEditorMonacoWidget.tsx

Lines changed: 27 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import './CellEditorMonacoWidget.css';
99
// React.
1010
import React from 'react';
1111

12-
import * as DOM from '../../../../../base/browser/dom.js';
1312
import { EditorExtensionsRegistry, IEditorContributionDescription } from '../../../../../editor/browser/editorExtensions.js';
1413
import { CodeEditorWidget } from '../../../../../editor/browser/widget/codeEditor/codeEditorWidget.js';
1514
import { Event } from '../../../../../base/common/event.js';
@@ -37,10 +36,6 @@ export function CellEditorMonacoWidget({ cell }: { cell: PositronNotebookCellGen
3736
/>;
3837
}
3938

40-
// Padding for the editor widget. The sizing is not perfect but this helps the editor not overflow
41-
// its container. In the future we should figure out how to make sure this is sized correctly.
42-
const EDITOR_INSET_PADDING_PX = 1;
43-
4439
/**
4540
* Create a cell editor widget for a cell.
4641
* @param cell Cell whose editor is to be created
@@ -51,107 +46,77 @@ export function useCellEditorWidget(cell: PositronNotebookCellGeneral) {
5146
const environment = useEnvironment();
5247
const instance = useNotebookInstance();
5348

54-
const sizeObservable = environment.sizeObservable;
55-
56-
// Grab the wrapping div for the editor. This is used for passing context key service
49+
// Create an element ref to contain the editor
5750
const editorPartRef = React.useRef<HTMLDivElement>(null);
58-
// Grab a ref to the div that will hold the editor. This is needed to pass an element to the
59-
// editor creation function.
60-
6151

6252
// Create the editor
6353
React.useEffect(() => {
6454
if (!editorPartRef.current) { return; }
6555

66-
const disposableStore = new DisposableStore();
67-
68-
// We need to use a native dom element here instead of a react ref one because the elements
69-
// created by react's refs are not _true_ dom elements and thus calls like `refEl instanceof
70-
// HTMLElement` will return false. This is a problem when we hand the elements into the
71-
// editor widget as it expects a true dom element.
72-
const nativeContainer = DOM.$('.positron-monaco-editor-container');
73-
editorPartRef.current.appendChild(nativeContainer);
56+
const disposables = new DisposableStore();
7457

7558
const language = cell.cellModel.language;
76-
const editorContextKeyService = environment.scopedContextKeyProviderCallback(editorPartRef.current);
77-
disposableStore.add(editorContextKeyService);
59+
const editorContextKeyService = disposables.add(environment.scopedContextKeyProviderCallback(editorPartRef.current));
7860
const editorInstaService = services.instantiationService.createChild(new ServiceCollection([IContextKeyService, editorContextKeyService]));
7961
const editorOptions = new CellEditorOptions(instance.getBaseCellEditorOptions(language), instance.notebookOptions, services.configurationService);
8062

81-
82-
const editor = editorInstaService.createInstance(CodeEditorWidget, nativeContainer, {
63+
const editor = disposables.add(editorInstaService.createInstance(CodeEditorWidget, editorPartRef.current, {
8364
...editorOptions.getDefaultValue(),
84-
// Turns off the margin of the editor. This should probably be placed in a settable
85-
// option somewhere eventually.
86-
glyphMargin: false,
8765
dimension: {
88-
width: 500,
89-
height: 200
66+
width: 0,
67+
height: 0,
9068
},
9169
}, {
9270
contributions: getNotebookEditorContributions()
93-
});
94-
disposableStore.add(editor);
71+
}));
9572
cell.attachEditor(editor);
9673

97-
editor.setValue(cell.getContent());
98-
99-
disposableStore.add(
100-
editor.onDidFocusEditorWidget(() => {
101-
instance.setEditingCell(cell);
102-
})
103-
);
74+
// Request model for cell and pass to editor.
75+
cell.getTextEditorModel().then(model => {
76+
editor.setModel(model);
77+
});
10478

105-
disposableStore.add(
106-
editor.onDidBlurEditorWidget(() => {
107-
})
108-
);
79+
disposables.add(editor.onDidFocusEditorWidget(() => {
80+
instance.setEditingCell(cell);
81+
}));
10982

83+
// disposables.add(editor.onDidBlurEditorWidget(() => {
84+
// }));
11085

11186
/**
11287
* Resize the editor widget to fill the width of its container and the height of its
11388
* content.
11489
* @param height Height to set. Defaults to checking content height.
11590
*/
11691
function resizeEditor(height: number = editor.getContentHeight()) {
92+
if (!editorPartRef.current) { return; }
11793
editor.layout({
11894
height,
119-
width: (editorPartRef.current?.offsetWidth ?? 500) - EDITOR_INSET_PADDING_PX * 2
95+
width: editorPartRef.current.offsetWidth,
12096
});
12197
}
12298

123-
// Request model for cell and pass to editor.
124-
cell.getTextEditorModel().then(model => {
125-
editor.setModel(model);
126-
resizeEditor();
127-
128-
editor.onDidContentSizeChange(e => {
129-
if (!(e.contentHeightChanged || e.contentWidthChanged)) { return; }
130-
resizeEditor(e.contentHeight);
131-
});
132-
});
133-
134-
// Keep the width up-to-date as the window resizes.
99+
// Resize the editor when its content size changes
100+
disposables.add(editor.onDidContentSizeChange(e => {
101+
if (!(e.contentHeightChanged || e.contentWidthChanged)) { return; }
102+
resizeEditor(e.contentHeight);
103+
}));
135104

136-
disposableStore.add(Event.fromObservable(sizeObservable)(() => {
105+
// Resize the editor as the window resizes.
106+
disposables.add(Event.fromObservable(environment.size)(() => {
137107
resizeEditor();
138108
}));
139109

140110
services.logService.info('Positron Notebook | useCellEditorWidget() | Setting up editor widget');
141111

142-
143112
return () => {
144113
services.logService.info('Positron Notebook | useCellEditorWidget() | Disposing editor widget');
145-
disposableStore.dispose();
146-
nativeContainer.remove();
114+
disposables.dispose();
147115
cell.detachEditor();
148116
};
149-
}, [cell, environment, instance, services.configurationService, services.instantiationService, services.logService, sizeObservable]);
150-
151-
117+
}, [cell, environment, instance, services.configurationService, services.instantiationService, services.logService]);
152118

153119
return { editorPartRef };
154-
155120
}
156121

157122

0 commit comments

Comments
 (0)